From: 김재원 <jaewon31.kim@samsung.com>
To: "Michal Hocko" <mhocko@kernel.org>, 김재원 <jaewon31.kim@samsung.com>
Cc: "vbabka@suse.cz" <vbabka@suse.cz>,
"bhe@redhat.com" <bhe@redhat.com>,
"mgorman@techsingularity.net" <mgorman@techsingularity.net>,
"minchan@kernel.org" <minchan@kernel.org>,
"mgorman@suse.de" <mgorman@suse.de>,
"hannes@cmpxchg.org" <hannes@cmpxchg.org>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"jaewon31.kim@gmail.com" <jaewon31.kim@gmail.com>,
이용택 <ytk.lee@samsung.com>, 김철민 <cmlaika.kim@samsung.com>
Subject: RE: [PATCH v4] page_alloc: consider highatomic reserve in watermark fast
Date: Mon, 22 Jun 2020 18:40:20 +0900 [thread overview]
Message-ID: <20200622094020epcms1p639cc33933fbb7a9d578adb16a6ea0734@epcms1p6> (raw)
In-Reply-To: <20200622091107.GC31426@dhcp22.suse.cz>
>On Sat 20-06-20 08:59:58, Jaewon Kim wrote:
>[...]
>> @@ -3502,19 +3525,12 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
>> const bool alloc_harder = (alloc_flags & (ALLOC_HARDER|ALLOC_OOM));
>>
>> /* free_pages may go negative - that's OK */
>> - free_pages -= (1 << order) - 1;
>> + free_pages -= __zone_watermark_unusable_free(z, order, alloc_flags);
>>
>> if (alloc_flags & ALLOC_HIGH)
>> min -= min / 2;
>>
>> - /*
>> - * If the caller does not have rights to ALLOC_HARDER then subtract
>> - * the high-atomic reserves. This will over-estimate the size of the
>> - * atomic reserve but it avoids a search.
>> - */
>> - if (likely(!alloc_harder)) {
>> - free_pages -= z->nr_reserved_highatomic;
>> - } else {
>> + if (unlikely(alloc_harder)) {
>> /*
>> * OOM victims can try even harder than normal ALLOC_HARDER
>> * users on the grounds that it's definitely going to be in
>[...]
>> @@ -3582,25 +3591,22 @@ static inline bool zone_watermark_fast(struct zone *z, unsigned int order,
>> unsigned long mark, int highest_zoneidx,
>> unsigned int alloc_flags)
>> {
>> - long free_pages = zone_page_state(z, NR_FREE_PAGES);
>> - long cma_pages = 0;
>> + long free_pages;
>> + long unusable_free;
>>
>> -#ifdef CONFIG_CMA
>> - /* If allocation can't use CMA areas don't use free CMA pages */
>> - if (!(alloc_flags & ALLOC_CMA))
>> - cma_pages = zone_page_state(z, NR_FREE_CMA_PAGES);
>> -#endif
>> + free_pages = zone_page_state(z, NR_FREE_PAGES);
>> + unusable_free = __zone_watermark_unusable_free(z, order, alloc_flags);
>>
>> /*
>> * Fast check for order-0 only. If this fails then the reserves
>> - * need to be calculated. There is a corner case where the check
>> - * passes but only the high-order atomic reserve are free. If
>> - * the caller is !atomic then it'll uselessly search the free
>> - * list. That corner case is then slower but it is harmless.
>> + * need to be calculated.
>> */
>> - if (!order && (free_pages - cma_pages) >
>> - mark + z->lowmem_reserve[highest_zoneidx])
>> - return true;
>> + if (!order) {
>> + long fast_free = free_pages - unusable_free;
>> +
>> + if (fast_free > mark + z->lowmem_reserve[highest_zoneidx])
>> + return true;
>> + }
>
>There is no user of unusable_free for order > 0. With you current code
>__zone_watermark_unusable_free would be called twice for high order
>allocations unless compiler tries to be clever..
Yes you're right.
Following code could be moved only for order-0.
unusable_free = __zone_watermark_unusable_free(z, order, alloc_flags);
Let me fix it at v5.
>
>But more importantly, I have hard time to follow why we need both
>zone_watermark_fast and zone_watermark_ok now. They should be
>essentially the same for anything but order == 0. For order 0 the
>only difference between the two is that zone_watermark_ok checks for
>ALLOC_HIGH resp ALLOC_HARDER, ALLOC_OOM. So what is exactly fast about
>the former and why do we need it these days?
>
I think the author, Mel, may ansewr. But I think the wmark_fast may
fast by 1) not checking more condition about wmark and 2) using inline
rather than function. According to description on commit 48ee5f3696f6,
it seems to bring about 4% improvement.
>>
>> return __zone_watermark_ok(z, order, mark, highest_zoneidx, alloc_flags,
>> free_pages);
>> --
>> 2.17.1
>>
>
>--
>Michal Hocko
>SUSE Labs
>
next prev parent reply other threads:[~2020-06-22 9:40 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20200619055816epcas1p184da90b01aff559fe3cd690ebcd921ca@epcas1p1.samsung.com>
2020-06-19 23:59 ` [PATCH v4] page_alloc: consider highatomic reserve in watermark fast Jaewon Kim
2020-06-19 12:42 ` Baoquan He
2020-06-22 8:55 ` Mel Gorman
2020-06-22 9:11 ` Michal Hocko
[not found] ` <CGME20200619055816epcas1p184da90b01aff559fe3cd690ebcd921ca@epcms1p6>
2020-06-22 9:40 ` 김재원 [this message]
2020-06-22 10:04 ` Mel Gorman
2020-06-22 14:23 ` Michal Hocko
2020-06-22 16:25 ` Mel Gorman
2020-06-23 7:11 ` Michal Hocko
2022-09-13 13:09 yong
2022-09-13 13:54 ` Greg KH
2022-09-14 0:46 ` yong w
2022-09-16 9:40 ` Greg KH
[not found] ` <CGME20220916094017epcas1p1deed4041f897d2bf0e0486554d79b3af@epcms1p4>
2022-09-18 1:41 ` Jaewon Kim
2022-09-19 13:21 ` yong w
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=20200622094020epcms1p639cc33933fbb7a9d578adb16a6ea0734@epcms1p6 \
--to=jaewon31.kim@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=bhe@redhat.com \
--cc=cmlaika.kim@samsung.com \
--cc=hannes@cmpxchg.org \
--cc=jaewon31.kim@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=mgorman@techsingularity.net \
--cc=mhocko@kernel.org \
--cc=minchan@kernel.org \
--cc=vbabka@suse.cz \
--cc=ytk.lee@samsung.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).