linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Xishi Qiu <qiuxishi@huawei.com>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Michal Hocko <mhocko@kernel.org>,
	Mel Gorman <mgorman@techsingularity.net>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	LKML <linux-kernel@vger.kernel.org>,
	"Linux MM" <linux-mm@kvack.org>,
	Yisheng Xie <xieyisheng1@huawei.com>
Subject: Re: [RFC] mm: a question about high-order check in __zone_watermark_ok()
Date: Wed, 28 Sep 2016 15:54:46 +0800	[thread overview]
Message-ID: <57EB7746.7030108@huawei.com> (raw)
In-Reply-To: <20160928055203.GE22706@js1304-P5Q-DELUXE>

On 2016/9/28 13:52, Joonsoo Kim wrote:

> On Mon, Sep 26, 2016 at 01:02:31PM +0200, Michal Hocko wrote:
>> On Mon 26-09-16 18:17:50, Xishi Qiu wrote:
>>> On 2016/9/26 17:43, Michal Hocko wrote:
>>>
>>>> On Mon 26-09-16 17:16:54, Xishi Qiu wrote:
>>>>> On 2016/9/26 16:58, Michal Hocko wrote:
>>>>>
>>>>>> On Mon 26-09-16 16:47:57, Xishi Qiu wrote:
>>>>>>> commit 97a16fc82a7c5b0cfce95c05dfb9561e306ca1b1
>>>>>>> (mm, page_alloc: only enforce watermarks for order-0 allocations)
>>>>>>> rewrite the high-order check in __zone_watermark_ok(), but I think it
>>>>>>> quietly fix a bug. Please see the following.
>>>>>>>
>>>>>>> Before this patch, the high-order check is this:
>>>>>>> __zone_watermark_ok()
>>>>>>> 	...
>>>>>>> 	for (o = 0; o < order; o++) {
>>>>>>> 		/* At the next order, this order's pages become unavailable */
>>>>>>> 		free_pages -= z->free_area[o].nr_free << o;
>>>>>>>
>>>>>>> 		/* Require fewer higher order pages to be free */
>>>>>>> 		min >>= 1;
>>>>>>>
>>>>>>> 		if (free_pages <= min)
>>>>>>> 			return false;
>>>>>>> 	}
>>>>>>> 	...
>>>>>>>
>>>>>>> If we have cma memory, and we alloc a high-order movable page, then it's right.
>>>>>>>
>>>>>>> But if we alloc a high-order unmovable page(e.g. alloc kernel stack in dup_task_struct()),
>>>>>>> and there are a lot of high-order cma pages, but little high-order unmovable
>>>>>>> pages, the it is still return *true*, but we will alloc *failed* finally, because
>>>>>>> we cannot fallback from migrate_unmovable to migrate_cma, right?
>>>>>>
>>>>>> AFAIR CMA wmark check was always tricky and the above commit has made
>>>>>> the situation at least a bit more clear. Anyway IIRC 
>>>>>>
>>>>>> #ifdef CONFIG_CMA
>>>>>> 	/* If allocation can't use CMA areas don't use free CMA pages */
>>>>>> 	if (!(alloc_flags & ALLOC_CMA))
>>>>>> 		free_cma = zone_page_state(z, NR_FREE_CMA_PAGES);
>>>>>> #endif
>>>>>>
>>>>>> 	if (free_pages - free_cma <= min + z->lowmem_reserve[classzone_idx])
>>>>>> 		return false;
>>>>>>
>>>>>> should reduce the prioblem because a lot of CMA pages should just get us
>>>>>> below the wmark + reserve boundary.
>>>>>
>>>>> Hi Michal,
>>>>>
>>>>> If we have many high-order cma pages, and the left pages (unmovable/movable/reclaimable)
>>>>> are also enough, but they are fragment, then it will triger the problem.
>>>>> If we alloc a high-order unmovable page, water mark check return *true*, but we
>>>>> will alloc *failed*, right?
>>>>
>>>> As Vlastimil has written. There were known issues with the wmark checks
>>>> and high order requests.
>>>
>>> Shall we backport to stable?
>>
>> I dunno, it was a part of a larger series with high atomic reserves and
>> changes which sound a bit intrusive for the stable kernel. Considering
>> that CMA was known to be problematic and there are still some issues
>> left I do not think this is worth the trouble/risk.
> 
> CMA problem is known one. I mentioned it on my ZONE_CMA series v1 but
> removed due to Mel's high atomic reserve series.
> 
> That series is rather large and has some problems so I think that it
> is not suitable for stable tree.
> 
> Thanks.
> 

OK, I know, thank you very much.

Thanks,
Xishi Qiu

> .
> 

      reply	other threads:[~2016-09-28  7:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-26  8:47 [RFC] mm: a question about high-order check in __zone_watermark_ok() Xishi Qiu
2016-09-26  8:52 ` Vlastimil Babka
2016-09-26  8:58 ` Michal Hocko
2016-09-26  9:16   ` Xishi Qiu
2016-09-26  9:43     ` Michal Hocko
2016-09-26 10:17       ` Xishi Qiu
2016-09-26 11:02         ` Michal Hocko
2016-09-28  5:52           ` Joonsoo Kim
2016-09-28  7:54             ` Xishi Qiu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57EB7746.7030108@huawei.com \
    --to=qiuxishi@huawei.com \
    --cc=hannes@cmpxchg.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=vbabka@suse.cz \
    --cc=xieyisheng1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).