Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <86984d9b-c955-4b06-9097-2a757b1bacfe@vivo.com>
Date:   Wed, 13 Dec 2023 09:58:26 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V4 1/2] mm: add defines for min/max swappiness
To:     Dan Schatzberg <schatzberg.dan@gmail.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Yosry Ahmed <yosryahmed@google.com>, Huan Yang <link@vivo.com>
Cc:     linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
        linux-mm@kvack.org, Tejun Heo <tj@kernel.org>,
        Zefan Li <lizefan.x@bytedance.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Michal Hocko <mhocko@kernel.org>,
        Shakeel Butt <shakeelb@google.com>,
        Muchun Song <muchun.song@linux.dev>,
        Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>,
        Matthew Wilcox <willy@infradead.org>,
        Chris Li <chrisl@kernel.org>,
        Kefeng Wang <wangkefeng.wang@huawei.com>,
        Yue Zhao <findns94@gmail.com>, Hugh Dickins <hughd@google.com>
References: <20231213013807.897742-1-schatzberg.dan@gmail.com>
 <20231213013807.897742-2-schatzberg.dan@gmail.com>
From:   Huan Yang <11133793@vivo.com>
In-Reply-To: <20231213013807.897742-2-schatzberg.dan@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?b1hDR0svSHNBWnlTRXFxYk41eXgzR2hRSUV4WG01R043Q28ya0lQaFV0eDlT?=
 =?utf-8?B?UmVOWWk5QW5BU3FEK1o5cVJOWGJGcDJBcWVKcXJZQnJVOGRhekErVkdDc1RV?=
 =?utf-8?B?OHZGNzVhdHR1TmJsK3EzUSs5OFhLTHBtMllPK2tncVRVN1Z5bEptTDBscUxC?=
 =?utf-8?B?K1lvQkdEQ3Y0Y2N2OEFCWFhTM0I4NVFGMTJIOHgrR3VRN2RMVmE0dVZiTFVY?=
 =?utf-8?B?b1p4c3U4Ym45b1dCeW8venFvQTJuaXdLVDVEV0M1a2VxSi8yQVlhZVQ3aVYx?=
 =?utf-8?B?dG4vR05Ma1BsSFF6NjNZNkZCSExwYmE5T054NWdMSGRGcVJPMXN3Vkpldk9Q?=
 =?utf-8?B?Y0pCRzFIQlM3Z3htQTZSUlRKTmlhZGprUnF2bXVjTGM5Rk1yOUR2eW95aXlr?=
 =?utf-8?B?bWlqWXRaa2p2aDE4S0Y4T2hKOXdVMllMR1NDenJBOHhNNG51eEJ6eXFUNGVV?=
 =?utf-8?B?VVBEazhHY2dEV1h1SmdCT3lUSyt2Uy9RSDFsMm5BSFY4My9ENXRPS1lxS1A1?=
 =?utf-8?B?NXFiRzZrTDhseGNqc0RyNWpidjg2RndWWHI1ZnRic01nUW1JeDJ4RStnTUtM?=
 =?utf-8?B?T1hVbG4vMFZLYWxYQ0txWW5JRzF3Z3hhSm05RExTRlVBdWE2L0RHUFB1bkl5?=
 =?utf-8?B?eEVrZzVvTmRCK1hLVncrc0RnT05oZjM2TGREL2t4UG5Lc3NtMHo4MkNDSlhu?=
 =?utf-8?B?eFV5Y1RaVmlaRGU2a3E5N1RlWThTblRQRFVveWFpSkx0YzdaRmNmeFd1SDR6?=
 =?utf-8?B?SWhOTzBDZFNWR25IV3l1SFcwVzZqaTZ0Mk5HekJaalFCdkxoVXVNSVE4R2Fz?=
 =?utf-8?B?dlpyWDk5SThhcU13Qlh4ZW5JOXdEbnlXWk1hc2FBVjhJQm9lNlQvZXA4U1Q5?=
 =?utf-8?B?SERwNTRnc21Eb2tKczBaYjBhSXFwNkVFcmJNU29pZFRIUm11QzFROXFkbzA0?=
 =?utf-8?B?VUZ5M1ZOUGxaV1VtVEtaQ1huOFhMbEdBdzF0Z1pjVDdWWXF3NXFaRlNiOCtr?=
 =?utf-8?B?ZHlKaFdNYldwbFVCeEtqQlhSRWJtellmalhuRk5sWHlTaW1zbUo4cSt0Vjc4?=
 =?utf-8?B?c2MwNkpzZHNJZVk5dlRPNms0QWFFMFNHbCtBbWJ5dTBBeUxrYWRndDB1Y01J?=
 =?utf-8?B?S3JYaGlVeDZqbmVNaGpYSmFGQWliZGdTUmtPN0RraTBXc2gzdDV3VUsvUmE4?=
 =?utf-8?B?NHBpcHlWWElHd29CVEEvQTFRalpJR09RZ0ZNTjFxaUpvTml0bDA3eEVJWGJy?=
 =?utf-8?B?ejliUzh1WmhjZy9ZalBLb25NaXAxM2VpbG9yVmRJNC8zd0dPcGlpd0s5ZGxE?=
 =?utf-8?B?OUovRGRzVHlsU0NqMWpsOW12U2srNkdzZ3FlZ2dKRU9NUjdZbndCTGkzZm5X?=
 =?utf-8?B?TmEwRTQyOHNmRXJ6enZCandsSm96TzBJOGdSOXl0K1BuZ25GZ2sxV0QwbXVm?=
 =?utf-8?B?a2tBVFNGUmJGU3h4RkI1bUQ4L3pmNldFOXloZGRTZmxWZmZuZFZScll1RGU0?=
 =?utf-8?B?YVU0THZmUFBoSHU4UTVWanpnblhzbUs2bjNteHZPUHRoSGQ1U09DeUtDZ2Ju?=
 =?utf-8?B?N0Q2RGRmN3hGYWEzbm1iM09oUW5EUEJodjRiTGlEd09DdVl5YmlSdEJ6UVdX?=
 =?utf-8?B?bm9yTHBKMm9yaks0UGcrR09FazNVekNHNWdRcmdSNThqNTRtUkZxQ0RBcFFl?=
 =?utf-8?B?S3dhSldFdUE3MjlJcGZVT3hhZnhJU2JLWWkxeExaY1YzbmJtbDk3NW9qZHE1?=
 =?utf-8?B?akFrS2JhcFBPT0RYZTNWaEJzdHNPNFNaSXljVEJWWll6NjVyaG9Jemd0SnZn?=
 =?utf-8?B?dEhCbGFmZmVoaDNRZFBxK29iend2UFBPd212WTBIb3hESzdNVk9Cd1dWTEtY?=
 =?utf-8?B?L1pNRGhZOXVTS0QxbTVjNjBBaWJWRkZtdXJhU0Q1aWRkbUVaQXd0V2cvai9X?=
 =?utf-8?B?VGZ6Vy9tQjFabmN5RTM2enhaQmk3QmxrMGYyalRRa1BZeHlLWTF2TktvNisz?=
 =?utf-8?B?SFp3NWErQXVzRXBTSUZZYmE2ZWc4RDhJbmlVSGlmYlh5aE1ySmlDQlpvY1d4?=
 =?utf-8?B?VlJvTktHRG5adVFqNnFiYWxnYjEvQk1LYVZsL3FoL09VMHJMNlJ6bk1RbEEz?=
 =?utf-8?Q?bbS02joSerUIp6lP9UQ6Jwq9B?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 85b85bfb-1f23-41d0-7bd4-08dbfb7f0472
X-MS-Exchange-CrossTenant-AuthSource: PUZPR06MB5676.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 01:58:36.1383
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: O5gZHgk7FUxWBvOcMHKIWwplh59J7T4zZy5/jvoP+BP+ajLIPqZuIPQfo5cFmkhCzsTK92xH8TdQxjUt774woQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: JH0PR06MB6655
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Tue, 12 Dec 2023 17:58:52 -0800 (PST)


在 2023/12/13 9:38, Dan Schatzberg 写道:
> [????????? schatzberg.dan@gmail.com ????????? https://aka.ms/LearnAboutSenderIdentification,????????????]
>
> We use the constants 0 and 200 in a few places in the mm code when
> referring to the min and max swappiness. This patch adds MIN_SWAPPINESS
> and MAX_SWAPPINESS #defines to improve clarity. There are no functional
> changes.
>
> Signed-off-by: Dan Schatzberg <schatzberg.dan@gmail.com>
> ---
>   include/linux/swap.h |  2 ++
>   mm/memcontrol.c      |  2 +-
>   mm/vmscan.c          | 10 +++++-----
>   3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index f6dd6575b905..e2ab76c25b4a 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -407,6 +407,8 @@ extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
>
>   #define MEMCG_RECLAIM_MAY_SWAP (1 << 1)
>   #define MEMCG_RECLAIM_PROACTIVE (1 << 2)
> +#define MIN_SWAPPINESS 0
> +#define MAX_SWAPPINESS 200

Do MAX_SWAPPINESS apply for all swapppiness? If so, maybe better change 
swappiness sysctl define:
```
sysctl.c:

{
         .procname    = "swappiness",
         .data        = &vm_swappiness,
         .maxlen        = sizeof(vm_swappiness),
         .mode        = 0644,
         .proc_handler    = proc_dointvec_minmax,
         .extra1        = SYSCTL_ZERO,
         .extra2        = SYSCTL_TWO_HUNDRED,
     },

```

Here hard code swappiness in [0, 200], and now add a new define.

And many other code hard reference 200 into max value of swappiness, like:

```
memcontrol.c:
static int mem_cgroup_swappiness_write(struct cgroup_subsys_state *css,
                        struct cftype *cft, u64 val)
{
     struct mem_cgroup *memcg = mem_cgroup_from_css(css);

     if (val > 200)
         return -EINVAL;

     if (!mem_cgroup_is_root(memcg))
         memcg->swappiness = val;
     else
         vm_swappiness = val;

     return 0;
}
vmscan.c:

/*
  * From 0 .. 200.  Higher means more swappy.
  */
int vm_swappiness = 60;

```


>   extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
>                                                    unsigned long nr_pages,
>                                                    gfp_t gfp_mask,
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1c1061df9cd1..9748a6b88bb8 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4337,7 +4337,7 @@ static int mem_cgroup_swappiness_write(struct cgroup_subsys_state *css,
>   {
>          struct mem_cgroup *memcg = mem_cgroup_from_css(css);
>
> -       if (val > 200)
> +       if (val > MAX_SWAPPINESS)
>                  return -EINVAL;
>
>          if (!mem_cgroup_is_root(memcg))
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 506f8220c5fe..2aa671fe938b 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2403,7 +2403,7 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
>          ap = swappiness * (total_cost + 1);
>          ap /= anon_cost + 1;
>
> -       fp = (200 - swappiness) * (total_cost + 1);
> +       fp = (MAX_SWAPPINESS - swappiness) * (total_cost + 1);
>          fp /= file_cost + 1;
>
>          fraction[0] = ap;
> @@ -4400,7 +4400,7 @@ static int get_type_to_scan(struct lruvec *lruvec, int swappiness, int *tier_idx
>   {
>          int type, tier;
>          struct ctrl_pos sp, pv;
> -       int gain[ANON_AND_FILE] = { swappiness, 200 - swappiness };
> +       int gain[ANON_AND_FILE] = { swappiness, MAX_SWAPPINESS - swappiness };
>
>          /*
>           * Compare the first tier of anon with that of file to determine which
> @@ -4444,7 +4444,7 @@ static int isolate_folios(struct lruvec *lruvec, struct scan_control *sc, int sw
>                  type = LRU_GEN_ANON;
>          else if (swappiness == 1)
>                  type = LRU_GEN_FILE;
> -       else if (swappiness == 200)
> +       else if (swappiness == MAX_SWAPPINESS)
>                  type = LRU_GEN_ANON;
>          else
>                  type = get_type_to_scan(lruvec, swappiness, &tier);
> @@ -5368,9 +5368,9 @@ static int run_cmd(char cmd, int memcg_id, int nid, unsigned long seq,
>
>          lruvec = get_lruvec(memcg, nid);
>
> -       if (swappiness < 0)
> +       if (swappiness < MIN_SWAPPINESS)
>                  swappiness = get_swappiness(lruvec, sc);
> -       else if (swappiness > 200)
> +       else if (swappiness > MAX_SWAPPINESS)
>                  goto done;
>
>          switch (cmd) {
> --
> 2.34.1
>