Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1161082 > unrolled thread

Re: [RFC PATCH 10/12] mm: add the buddy system interface

Started byKamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
First post2015-06-09 09:20 +0200
Last post2015-06-16 02:40 +0200
Articles 3 — 1 participant

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFC PATCH 10/12] mm: add the buddy system interface Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> - 2015-06-09 09:20 +0200
    Re: [RFC PATCH 10/12] mm: add the buddy system interface Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> - 2015-06-10 05:10 +0200
      Re: [RFC PATCH 10/12] mm: add the buddy system interface Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> - 2015-06-16 02:40 +0200

#1161082 — Re: [RFC PATCH 10/12] mm: add the buddy system interface

FromKamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date2015-06-09 09:20 +0200
SubjectRe: [RFC PATCH 10/12] mm: add the buddy system interface
Message-ID<pzlZg-7RG-13@gated-at.bofh.it>
On 2015/06/04 22:04, Xishi Qiu wrote:
> Add the buddy system interface for address range mirroring feature.
> Allocate mirrored pages in MIGRATE_MIRROR list. If there is no mirrored pages
> left, use other types pages.
>
> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> ---
>   mm/page_alloc.c | 40 +++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d4d2066..0fb55288 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -599,6 +599,26 @@ static inline bool is_mirror_pfn(unsigned long pfn)
>
>   	return false;
>   }
> +
> +static inline bool change_to_mirror(gfp_t gfp_flags, int high_zoneidx)
> +{
> +	/*
> +	 * Do not alloc mirrored memory below 4G, because 0-4G is
> +	 * all mirrored by default, and the list is always empty.
> +	 */
> +	if (high_zoneidx < ZONE_NORMAL)
> +		return false;
> +
> +	/* Alloc mirrored memory for only kernel */
> +	if (gfp_flags & __GFP_MIRROR)
> +		return true;

GFP_KERNEL itself should imply mirror, I think.

> +
> +	/* Alloc mirrored memory for both user and kernel */
> +	if (sysctl_mirrorable)
> +		return true;

Reading this, I think this sysctl is not good. The user cannot know what is mirrored
because memory may not be mirrored until the sysctl is set.

Thanks,
-Kame


> +
> +	return false;
> +}
>   #endif
>
>   /*
> @@ -1796,7 +1816,10 @@ struct page *buffered_rmqueue(struct zone *preferred_zone,
>   			WARN_ON_ONCE(order > 1);
>   		}
>   		spin_lock_irqsave(&zone->lock, flags);
> -		page = __rmqueue(zone, order, migratetype);
> +		if (is_migrate_mirror(migratetype))
> +			page = __rmqueue_smallest(zone, order, migratetype);
> +		else
> +			page = __rmqueue(zone, order, migratetype);
>   		spin_unlock(&zone->lock);
>   		if (!page)
>   			goto failed;
> @@ -2928,6 +2951,11 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
>   	if (IS_ENABLED(CONFIG_CMA) && ac.migratetype == MIGRATE_MOVABLE)
>   		alloc_flags |= ALLOC_CMA;
>
> +#ifdef CONFIG_MEMORY_MIRROR
> +	if (change_to_mirror(gfp_mask, ac.high_zoneidx))
> +		ac.migratetype = MIGRATE_MIRROR;
> +#endif
> +
>   retry_cpuset:
>   	cpuset_mems_cookie = read_mems_allowed_begin();
>
> @@ -2943,9 +2971,19 @@ retry_cpuset:
>
>   	/* First allocation attempt */
>   	alloc_mask = gfp_mask|__GFP_HARDWALL;
> +retry:
>   	page = get_page_from_freelist(alloc_mask, order, alloc_flags, &ac);
>   	if (unlikely(!page)) {
>   		/*
> +		 * If there is no mirrored memory, we will alloc other
> +		 * types memory.
> +		 */
> +		if (is_migrate_mirror(ac.migratetype)) {
> +			ac.migratetype = gfpflags_to_migratetype(gfp_mask);
> +			goto retry;
> +		}
> +
> +		/*
>   		 * Runtime PM, block IO and its error handling path
>   		 * can deadlock because I/O on the device might not
>   		 * complete.
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1161924

FromKamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date2015-06-10 05:10 +0200
Message-ID<pzEyR-1FE-5@gated-at.bofh.it>
In reply to#1161082
On 2015/06/09 19:04, Xishi Qiu wrote:
> On 2015/6/9 15:12, Kamezawa Hiroyuki wrote:
>
>> On 2015/06/04 22:04, Xishi Qiu wrote:
>>> Add the buddy system interface for address range mirroring feature.
>>> Allocate mirrored pages in MIGRATE_MIRROR list. If there is no mirrored pages
>>> left, use other types pages.
>>>
>>> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
>>> ---
>>>    mm/page_alloc.c | 40 +++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 39 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index d4d2066..0fb55288 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -599,6 +599,26 @@ static inline bool is_mirror_pfn(unsigned long pfn)
>>>
>>>        return false;
>>>    }
>>> +
>>> +static inline bool change_to_mirror(gfp_t gfp_flags, int high_zoneidx)
>>> +{
>>> +    /*
>>> +     * Do not alloc mirrored memory below 4G, because 0-4G is
>>> +     * all mirrored by default, and the list is always empty.
>>> +     */
>>> +    if (high_zoneidx < ZONE_NORMAL)
>>> +        return false;
>>> +
>>> +    /* Alloc mirrored memory for only kernel */
>>> +    if (gfp_flags & __GFP_MIRROR)
>>> +        return true;
>>
>> GFP_KERNEL itself should imply mirror, I think.
>>
>
> Hi Kame,
>
> How about like this: #define GFP_KERNEL (__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_MIRROR) ?
>

Hm.... it cannot cover GFP_ATOMIC at el.

I guess, mirrored memory should be allocated if !__GFP_HIGHMEM or !__GFP_MOVABLE

thanks,
-Kame

Thanks,
-Kame



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1165623

FromKamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date2015-06-16 02:40 +0200
Message-ID<pBN4Z-2YA-3@gated-at.bofh.it>
In reply to#1161924
On 2015/06/16 2:20, Luck, Tony wrote:
> On Mon, Jun 15, 2015 at 05:47:27PM +0900, Kamezawa Hiroyuki wrote:
>> So, there are 3 ideas.
>>
>>   (1) kernel only from MIRROR / user only from MOVABLE (Tony)
>>   (2) kernel only from MIRROR / user from MOVABLE + MIRROR(ASAP)  (AKPM suggested)
>>       This makes use of the fact MOVABLE memory is reclaimable but Tony pointed out
>>       the memory reclaim can be critical for GFP_ATOMIC.
>>   (3) kernel only from MIRROR / user from MOVABLE, special user from MIRROR (Xishi)
>>
>> 2 Implementation ideas.
>>    - creating ZONE
>>    - creating new alloation attribute
>>
>> I don't convince whether we need some new structure in mm. Isn't it good to use
>> ZONE_MOVABLE for not-mirrored memory ?
>> Then, disable fallback from ZONE_MOVABLE -> ZONE_NORMAL for (1) and (3)
>
> We might need to rename it ... right now the memory hotplug
> people use ZONE_MOVABLE to indicate regions of physical memory
> that can be removed from the system.  I'm wondering whether
> people will want systems that have both removable and mirrored
> areas?  Then we have four attribute combinations:
>
> mirror=no  removable=no  - prefer to use for user, could use for kernel if we run out of mirror
> mirror=no  removable=yes - can only be used for user (kernel allocation makes it not-removable)
> mirror=yes removable=no  - use for kernel, possibly for special users if we define some interface
> mirror=yes removable=yes - must not use for kernel ... would have to give to user ... seems like a bad idea to configure a system this way
>

Thank you for clarification. I see "mirror=no, removable=no" case may require a new name.

IMHO, the value of Address-Based-Memory-Mirror is that users can protect their system's
important functions without using full-memory mirror. So, I feel thinking
"mirror=no, removable=no" just makes our discussion/implemenation complex without real
user value.

Shouldn't we start with just thiking 2 cases of
  mirror=no  removable=yes
  mirror=yes removable=no
?

And then, if the naming is problem, alias name can be added.

Thanks,
-Kame






--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web