2022-11-09 07:29:29

by Chao Xu

Subject: [PATCH] mm/vmscan: simplify the nr assignment logic for pages to scan

By default the assignment logic of anonymouns or file inactive
pages and active pages to scan using the same duplicated code
snippet. To simplify the logic, merge the same part.

Signed-off-by: Chao Xu <[email protected]>
---
mm/vmscan.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 04d8b88e5216..df3c0cbe381f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5932,14 +5932,11 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
* scan target and the percentage scanning already complete
*/
lru = (lru == LRU_FILE) ? LRU_BASE : LRU_FILE;
- nr_scanned = targets[lru] - nr[lru];
- nr[lru] = targets[lru] * (100 - percentage) / 100;
- nr[lru] -= min(nr[lru], nr_scanned);
-
- lru += LRU_ACTIVE;
- nr_scanned = targets[lru] - nr[lru];
- nr[lru] = targets[lru] * (100 - percentage) / 100;
- nr[lru] -= min(nr[lru], nr_scanned);
+ for ( ; lru <= lru + LRU_ACTIVE; lru++) {
+ nr_scanned = targets[lru] - nr[lru];
+ nr[lru] = targets[lru] * (100 - percentage) / 100;
+ nr[lru] -= min(nr[lru], nr_scanned);
+ }

scan_adjusted = true;
}
--
2.34.1

2022-11-10 01:44:37

by Andrew Morton

Subject: Re: [PATCH] mm/vmscan: simplify the nr assignment logic for pages to scan

On Wed, 9 Nov 2022 15:04:16 +0800 Chao Xu <[email protected]> wrote:

> By default the assignment logic of anonymouns or file inactive
> pages and active pages to scan using the same duplicated code
> snippet. To simplify the logic, merge the same part.
>
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -5932,14 +5932,11 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
> * scan target and the percentage scanning already complete
> */
> lru = (lru == LRU_FILE) ? LRU_BASE : LRU_FILE;
> - nr_scanned = targets[lru] - nr[lru];
> - nr[lru] = targets[lru] * (100 - percentage) / 100;
> - nr[lru] -= min(nr[lru], nr_scanned);
> -
> - lru += LRU_ACTIVE;
> - nr_scanned = targets[lru] - nr[lru];
> - nr[lru] = targets[lru] * (100 - percentage) / 100;
> - nr[lru] -= min(nr[lru], nr_scanned);
> + for ( ; lru <= lru + LRU_ACTIVE; lru++) {

The "lru++" implicitly assumes that LRU_ACTIVE=1. That happens to be
the case, but a more accurate translation of the existing code would
use "lru += LRU_ACTIVE" here, yes?

> + nr_scanned = targets[lru] - nr[lru];
> + nr[lru] = targets[lru] * (100 - percentage) / 100;
> + nr[lru] -= min(nr[lru], nr_scanned);
> + }
>
> scan_adjusted = true;
> }

2022-11-10 03:08:41

by Chao Xu

Subject: Re: [PATCH] mm/vmscan: simplify the nr assignment logic for pages to scan

在 2022/11/10 9:19, Andrew Morton 写道:
> On Wed, 9 Nov 2022 15:04:16 +0800 Chao Xu <[email protected]> wrote:
>
>> By default the assignment logic of anonymouns or file inactive
>> pages and active pages to scan using the same duplicated code
>> snippet. To simplify the logic, merge the same part.
>>
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -5932,14 +5932,11 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
>> * scan target and the percentage scanning already complete
>> */
>> lru = (lru == LRU_FILE) ? LRU_BASE : LRU_FILE;
>> - nr_scanned = targets[lru] - nr[lru];
>> - nr[lru] = targets[lru] * (100 - percentage) / 100;
>> - nr[lru] -= min(nr[lru], nr_scanned);
>> -
>> - lru += LRU_ACTIVE;
>> - nr_scanned = targets[lru] - nr[lru];
>> - nr[lru] = targets[lru] * (100 - percentage) / 100;
>> - nr[lru] -= min(nr[lru], nr_scanned);
>> + for ( ; lru <= lru + LRU_ACTIVE; lru++) {
> The "lru++" implicitly assumes that LRU_ACTIVE=1. That happens to be
> the case, but a more accurate translation of the existing code would
> use "lru += LRU_ACTIVE" here, yes?
By default the value of LRU_ACTIVE is 1，but if someone change it one day, I
use "lru++" maybe facing some exceptions, which is not robust. So I
agree with
that "lru += LRU_ACTIVE" is appropriate instead of "lru++". I will send
a new patch.
>> + nr_scanned = targets[lru] - nr[lru];
>> + nr[lru] = targets[lru] * (100 - percentage) / 100;
>> + nr[lru] -= min(nr[lru], nr_scanned);
>> + }
>>
>> scan_adjusted = true;
>> }