linux-kernel - Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of

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] [day] [month] [year] [list]
Message-ID: <20250603030530.365142-1-wangchuanguo@inspur.com>
Date: Tue, 3 Jun 2025 11:05:30 +0800
From: wangchuanguo <wangchuanguo@...pur.com>
To: <wangchuanguo@...pur.com>, SeongJae Park <sj@...nel.org>
CC: "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"hannes@...xchg.org" <hannes@...xchg.org>, "david@...hat.com"
	<david@...hat.com>, "mhocko@...nel.org" <mhocko@...nel.org>,
	"zhengqi.arch@...edance.com" <zhengqi.arch@...edance.com>,
	"shakeel.butt@...ux.dev" <shakeel.butt@...ux.dev>,
	"lorenzo.stoakes@...cle.com" <lorenzo.stoakes@...cle.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "damon@...ts.linux.dev"
	<damon@...ts.linux.dev>, Honggyu Kim <honggyu.kim@...com>
Subject: Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

On Fri, 30 May 2025 12:40:16 -0700 SeongJae Park <sj@...nel.org> wrote:

> Hi Simon,
> 
> 
> Thank you for continuing this important discussion.
> 
> Before starting, though, seems your mail client is not setting 'In-Reply-To'
> field of your mails.  For people who uses 'In-Reply-To' field based threads
> displaying tools, ths thread could be difficult to read the whole contents.
> Please consider using tools that set the field correctly if possible.
> 
> You could get more information about available mailing tools from
> https://docs.kernel.org/process/email-clients.html
> 
> Btw, I use hkml
> (https://docs.kernel.org/process/email-clients.html#hackermail-tui) ;)
> 
> On Fri, 30 May 2025 08:04:42 +0000 Simon Wang (王传国) <wangchuanguo@...pur.com> wrote:
> 
> [...]
> > Your concern is that adding the bool use_nodes_of_tier variable and introducing 
> > an additional parameter to multiple functions would cause ABI changes, correct?
> 
> You are correct.
> 
> > 
> > I propose avoiding the creation of the 'use_nodes_of_tier' sysfs
> > file. Instead, we can modify the __damon_pa_migrate_folio_list() function to
> > change the allowed_mask from NODE_MASK_NONE to the full node mask of the
> > entire tier where the target_nid resides.  This approach would be similar to
> > the implementation in commit 320080272892 ('mm/demotion: demote pages
> > according to allocation fallback order').
> 
> Then, this causes a behavior change, which we should not allow if it can be
> considered a regression.  In other words, we could do this if it is a clear
> improvement.
> 
> So, let's think about if your proposed change is an improvement.  As the commit
> 320080272892 is nicely explaining, I think that it is an improved behavior for
> demotion.  Actually it seems good behavior for promotion, too.  But, the
> behavior we are discussing here is not for the demotion but general migration
> (specifically, DAMOS_MIGRATE_{HOT,COLD}).
> 
> In my opinion, DAMOS_MIGRATE_{HOT,COLD} behavior should be somewhat similar to
> that of move_pages() syscall, to make its behavior easy to expect.  So I think
> having commit 320080272892's behavior improvement to DAMOS_MIGRATE_{HOT,COLD}
> is not a right thing to do.
> 
> And this asks me a question.  Is current DAMOS_MIGRATE_{HOT,COLD} behavior
> similar to move_pages() syscall?  Not really, since do_move_pages_to_node(),
> which is called from move_pages() syscall and calls migrate_pages() is setting
> mtc->nmask as NULL, while DAMOS_MIGRATE_{HOT,COLD} set it as NODE_MASK_NONE.
> Also, do_move_pages_to_node() uses alloc_migration_target() while
> DAMOS_MIGRATE_{HOT,COLD} uses alloc_migrate_folio().
> 
> I overlooked this different behavior while reviewing this code, sorry.  And I
> don't think this difference is what we need to keep, unless there are good
> rasons that well documented.  Thank you for let us find this, Simon.
> 
> So I suggest to set mtc->nmask as NULL, and use alloc_migration_target() from
> __damon_pa_migrate_folio_list(), same to move_pages() system call.  To use
> alloc_migrate_folio() from __damon_pa_migrate_folio_list(), we renamed it from
> alloc_demote_folio(), and made it none-static.  If we use
> alloc_migration_target() from __damon_pa_migrate_folio_list(), there is no
> reason to keep the changes.  Let's revert those too.
> 
> Cc-ing Honggyu, who originally implemented the current behavior of
> __damon_pa_migrate().  Honggyu, could you please let us know if the above
> suggested changes are not ok for you?
> 
> If Honggyu has no problem at the suggested change, Simon, would you mind doing
> that?  I can also make the patches.  I don't really care who do that.  I just
> think someone should do that.  This shouldn't be urgent real issue, in my
> opinion, though.
> 
> > 
> > I'd like to confirm two modification points with you:
> > 1.Regarding alloc_migrate_folio():
> > Restoring the original nodemask and gfp_mask in this function is the correct approach, correct?
> 
> I think that's correct, but let's discuss about the patch on the patch's
> thread.
> 
> > 2.Regarding DAMON's migration logic:
> > The target scope should be expanded from a single specified node to the entire memory tier
> >  (where the target node resides), correct?
> 
> I don't think so, as abovely explained.
> 
> > Can we confirm these two points are agreed upon?
> 
> I believe hope this is answered above.
> 
> 
> Thanks,
> SJ
> 
> [...]

Sent using hkml (https://github.com/sjp38/hackermail)