| 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
| ||
|
Message-ID: <87wm8jukkp.fsf@DESKTOP-5N7EMDA>
Date: Tue, 08 Jul 2025 16:56:54 +0800
From: "Huang, Ying" <ying.huang@...ux.alibaba.com>
To: "Zhijian Li (Fujitsu)" <lizhijian@...itsu.com>
Cc: "linux-mm@...ck.org" <linux-mm@...ck.org>, "akpm@...ux-foundation.org"
<akpm@...ux-foundation.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "Yasunori Gotou (Fujitsu)"
<y-goto@...itsu.com>, Ingo Molnar <mingo@...hat.com>, Peter Zijlstra
<peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>, Vincent
Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann
<dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>, Ben
Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, Valentin
Schneider <vschneid@...hat.com>, "lkp@...el.com" <lkp@...el.com>
Subject: Re: [PATCH RFC v2] mm: memory-tiering: Fix PGPROMOTE_CANDIDATE
accounting
"Zhijian Li (Fujitsu)" <lizhijian@...itsu.com> writes:
> On 08/07/2025 10:47, Huang, Ying wrote:
>> "Zhijian Li (Fujitsu)" <lizhijian@...itsu.com> writes:
>>
>>> On 08/07/2025 09:14, Huang, Ying wrote:
>>>> "Zhijian Li (Fujitsu)" <lizhijian@...itsu.com> writes:
>>>>
>>>>> Hi,
>>>>>
>>>>>
>>>>> On 25/06/2025 10:13, Li Zhijian wrote:
>>>>>> V2:
>>>>>> Fix compiling error # Reported by LKP
>>>>>>
>>>>>> As Ying suggested, we need to assess whether this change causes regression.
>>>>>> However, considering the stringent conditions this patch involves,
>>>>>> properly evaluating it may be challenging, as the outcomes depend on your
>>>>>> perspective. Much like in a zero-sum game, if someone benefits, another
>>>>>> might lose.
>>>>>>
>>>>>> If there are subsequent results, I will update them here.
>>>>>
>>>>> I ran memhog + pmbench to evaluate the impact of the patch(3 runs [1] for each kernel).
>>>>>
>>>>> The results show an approximate 4% performance increase in pmbench after applying this patch.
>>>>>
>>>>> Average pmbench-access max-promotion-rate
>>>>> Before: 7956805 pages/sec 168301 pages/sec
>>>>> After: 8313666 pages/sec (+4.4%) 207149 pages/sec
>>>>
>>>> It's hard for me to understand why performance increases because of
>>>> higher promotion rate, while the expected behavior is more promotion
>>>> rate limiting.
>>>
>>> Good question.
>>>
>>> Above max-promotion-rate means the maximum rate during the WHOLE pmbench period which
>>> can not indicate the total promoted pages.
>>>
>>> Allow me to present each sample [0] recorded per second during the pmbench duration, as exemplified below:
>>>
>>>
>>> | AFTER |VS | BEFORE |
>>> ------------+-------------------------+++++------------------------|
>>> | Timestamp | pgprom/s | pgdem/s | | pgprom/s | pgdem/s |
>>> |-----------|-------------|-----------|---|------------|-----------|
>>> | 1 | 122977 | 0 | | 123051 | 0 |
>>> | 2 | 50171 | 0 | | 50159 | 0 |
>>> | 3 | 18 | 0 | | 28 | 0 |
>>> | 4 | 16647 | 0 | | 0 | 0 |
>>> | 5 | 207149.5 | 0 | | 78895 | 0 |
>>> | 6 | 193411 | 161521 | | 168301 | 8702 |
>>> | 7 | 52464 | 53989 | | 42294 | 39108 |
>>> | 8 | 5133 | 2627 | | 0 | 0 |
>>> | 9 | 24 | 8 | | 3875 | 6213 |
>>> | 10 | 0 | 0 | | 45513 | 43260 |
>>> | 11 | 0 | 0 | | 36600 | 44982 |
>>> | 12 | 0 | 0 | | 21091 | 11631 |
>>> | 13 | 0 | 0 | | 12276 | 10719 |
>>> | 14 | 0 | 0 | | 149699 | 149400 |
>>> | 15 | 0 | 0 | | 4026 | 4933 |
>>> | 16 | 0 | 0 | | 3780 | 0 |
>>> | 17 | 0 | 0 | | 2 | 0 |
>>> | 18 | 0 | 0 | | 0 | 0 |
>>> | 19 | 0 | 0 | | 0 | 0 |
>>> | 20 | 0 | 0 | | 0 | 0 |
>>> | 21 | 0 | 0 | | 62 | 0 |
>>> | 22 | 0 | 0 | | 2016 | 0 |
>>> | 23 | 0 | 0 | | 0 | 0 |
>>> | 24 | 0 | 0 | | 62 | 0 |
>>> | 25 | 8308 | 0 | | 1 | 0 |
>>> | 26 | 220 | 0 | | 0 | 0 |
>>> | 27 | 0 | 0 | | 1995.05 | 0 |
>>> | 28 | 0 | 0 | | 1 | 0 |
>>> | 29 | 5791 | 0 | | 0 | 0 |
>>> | 30 | 0 | 0 | | 62 | 0 |
>>> ------------+-------------------------+++++------------------------|
>>> | total | 662313.5 | 218145 | | 743789.05 | 318948 |
>>> | max | 207149.5 | 161521 | | 168301 | 149400 |
>>> ------------+-------------------------+++++------------------------|
>>> | pmbench | 8416250 |VS | 8079500 |
>>>
>>>
>>> As far as I can tell, the higher pmbench scores applied-patch may be attributed to
>>> a reduction in the total number of promoted pages in the entire pmbench execution period.
>>> (Similar circumstances were observed in the results of other tests conducted)
>>>
>>>
>>>
>>> [0]
>>> before:
>>> https://github.com/zhijianli88/misc/blob/main/20250627/promotion-evaluation/without-patch/pmbench-1750988862.log
>>> https://github.com/zhijianli88/misc/blob/main/20250627/promotion-evaluation/without-patch/sar-1750988862.log
>>> after:
>>> https://github.com/zhijianli88/misc/blob/main/20250627/promotion-evaluation/with-patch/pmbench-1750988291.log
>>> https://github.com/zhijianli88/misc/blob/main/20250627/promotion-evaluation/with-patch/sar-1750988291.log
>>>
>>
>> Check the usage of PGPROMOTE_CANDIDATE again. It is used not only by
>> rate limiting, but also promotion threshold adjustment, please take a
>> look at numa_promotion_adjust_threshold(). Which may have larger
>> influence on performance.
>>
>> After checking the threshold adjustment code, I think the changes in
>> this patch may confuse threshold adjustment.
>
>
> Indeed, I misunderstood the comment in the previous code:
> /* workload changed, reset hot threshold */.
>
> Originally, this logic only reset the threshold for the current interval.
> For the next cycle (60 seconds by default), the threshold is
> re-evaluated based on the historical PGPROMOTE_CANDIDATE counts.
> Therefore, the current change may affect threshold adjustment in subsequent cycles.
>
>
> Do you think there's still a case to push for this patch?
>
> For example, by collecting more data with longer pmbench runs (over two threshold cycles),
> or explicitly compensating nbp_rl_nr_cand and nbp_th_nr_cand to maintain existing
> behavior for both the rate limit and threshold logic? something like:
>
> if (pgdat_free_space_enough(pgdat)) {
> /* workload changed, reset hot threshold */
> pgdat->nbp_threshold = 0;
>
> mod_node_page_state(pgdat, PGPROMOTE_CANDIDATE, nr);
> // compensation for rate limit and threshold
> pgdat->nbp_rl_nr_cand += nr;
> pgdat->nbp_th_nr_cand += nr;
>
> return true;
> }
I don't think that it's necessary to make the algorithm harder to be
understood.
If you think that the original stat really makes people confusing, I
guess that we can add a new stat (say PGPROMOTE_CANDIDATE_OTHER).
---
Best Regards,
Huang, Ying
Powered by blists - more mailing lists