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

[Huang, Ying]

Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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