linux-kernel - Re: [PATCH 01/10] mm, pcp: avoid to drain PCP when process exit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87mswodkv5.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date:   Thu, 12 Oct 2023 21:35:26 +0800
From:   "Huang, Ying" <ying.huang@...el.com>
To:     Mel Gorman <mgorman@...hsingularity.net>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>,
        Arjan Van De Ven <arjan@...ux.intel.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        David Hildenbrand <david@...hat.com>,
        Johannes Weiner <jweiner@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Michal Hocko <mhocko@...e.com>,
        Pavel Tatashin <pasha.tatashin@...een.com>,
        Matthew Wilcox <willy@...radead.org>,
        "Christoph Lameter" <cl@...ux.com>
Subject: Re: [PATCH 01/10] mm, pcp: avoid to drain PCP when process exit

Mel Gorman <mgorman@...hsingularity.net> writes:

> On Wed, Oct 11, 2023 at 10:16:17AM -0700, Andrew Morton wrote:
>> On Wed, 11 Oct 2023 13:46:10 +0100 Mel Gorman <mgorman@...hsingularity.net> wrote:
>> 
>> > > --- a/include/linux/mmzone.h
>> > > +++ b/include/linux/mmzone.h
>> > > @@ -676,12 +676,15 @@ enum zone_watermarks {
>> > >  #define high_wmark_pages(z) (z->_watermark[WMARK_HIGH] + z->watermark_boost)
>> > >  #define wmark_pages(z, i) (z->_watermark[i] + z->watermark_boost)
>> > >  
>> > > +#define	PCPF_PREV_FREE_HIGH_ORDER	0x01
>> > > +
>> > 
>> > The meaning of the flag and its intent should have been documented.
>> 
>> I need to rebase mm-stable for other reasons.  So let's please
>> decide (soon) whether Mel's review comments can be addressed
>> via add-on patches or whether I should drop this version of this
>> series altogether, during that rebase.
>
> The cache slice calculation is the only change I think may deserve a
> respin as it may have a material impact on the performance figures if the
> "size_data" value changes by too much. Huang, what do you think and how
> long do you think it would take to update the performance figures?  As it
> may require multiple tests for each patch in the series, I would also be ok
> with a follow-on patch like "mm: page_alloc: Simply cache size estimation
> for PCP tuning" that documents the limitation of summing the unified caches
> and the impact, if any, on performance. It makes for a messy history *but*
> it would also record the reasons why summing hierarchies is not necessarily
> the best approach which also has value.

I am OK to respin the series.  It will take 3-4 days to update the
performance figures.

> I think patch 9 should be dropped as it has no impact on headline performance
> while adding a relatively tricky heuristic that updates within a fast
> path. Again, I'd like to give Huang a chance to respond and to evaluate
> if it materially impacts patch 10 -- I don't think it does but I didn't
> think very hard about it. Even if patch 9+10 had to be dropped, it would
> not take much from the overall value of the series.

I am OK to drop patch 9 at least for now.  In the future we may revisit
it when we found workloads that benefit more from it.  It's not too hard
to rebase patch 10.

> Comments and documentation alone are not grounds for pulling the series but
> I hope they do get addressed in follow-on patches. I think requiring them
> for accepting the series is unfair even if the only reason is I took too
> long to review.

Never mind, your review are very value!

--
Best Regards,
Huang, Ying