From: SeongJae Park <sj@kernel.org>
To: Honggyu Kim <honggyu.kim@sk.com>
Cc: SeongJae Park <sj@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, damon@lists.linux.dev,
linux-kernel@vger.kernel.org, kernel_team@skhynix.com
Subject: Re: [RFC PATCH v4 1/5] mm: make alloc_demote_folio externally invokable for migration
Date: Wed, 15 May 2024 09:10:40 -0700 [thread overview]
Message-ID: <20240515161040.69242-1-sj@kernel.org> (raw)
In-Reply-To: <20240513120358.1503-1-honggyu.kim@sk.com>
On Mon, 13 May 2024 21:03:56 +0900 Honggyu Kim <honggyu.kim@sk.com> wrote:
> On Sun, 12 May 2024 10:54:43 -0700 SeongJae Park <sj@kernel.org> wrote:
> > From: Honggyu Kim <honggyu.kim@sk.com>
> >
> > The alloc_demote_folio can be used out of vmscan.c so it'd be better to
> > remove static keyword from it.
> >
> > This function can also be used for both demotion and promotion so it'd
> > be better to rename it from alloc_demote_folio to alloc_migrate_folio.
>
> This description doesn't match with the changes below.
You're right. Thank you for checking this.
>
> > Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
> > Reviewed-by: SeongJae Park <sj@kernel.org>
> > Signed-off-by: SeongJae Park <sj@kernel.org>
> > ---
> > mm/internal.h | 1 +
> > mm/vmscan.c | 3 +--
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index b2c75b12014e..b3ca996a4efc 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -1052,6 +1052,7 @@ extern unsigned long __must_check vm_mmap_pgoff(struct file *, unsigned long,
> > unsigned long, unsigned long);
> >
> > extern void set_pageblock_order(void);
> > +struct folio *alloc_demote_folio(struct folio *src, unsigned long private);
>
> I still prefer to rename it to alloc_migrate_folio.
And I still have no objection at renaming. I didn't make the change here not
because I want the change removed, but just because I wrote this RFC v4 in hope
of seeing how minimum this patchset can be while addressing my comments.
Please feel free to add the change back on the next revision of this patch if
you write. Maybe keeping this patchset without the renaming change, and
discussing the renaming with a separate patch is also an option in my opinion.
>
> > unsigned long reclaim_pages(struct list_head *folio_list);
> > unsigned int reclaim_clean_pages_from_list(struct zone *zone,
> > struct list_head *folio_list);
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 6981a71c8ef0..27269bc2bcc5 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -933,8 +933,7 @@ static void folio_check_dirty_writeback(struct folio *folio,
> > mapping->a_ops->is_dirty_writeback(folio, dirty, writeback);
> > }
> >
> > -static struct folio *alloc_demote_folio(struct folio *src,
> > - unsigned long private)
> > +struct folio *alloc_demote_folio(struct folio *src, unsigned long private)
>
> Here as well.
Ditto.
>
> > {
> > struct folio *dst;
> > nodemask_t *allowed_mask;
> > --
> > 2.39.2
> >
>
> Thanks,
> Honggyu
Thanks,
SJ
next prev parent reply other threads:[~2024-05-15 16:10 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-12 17:54 [RFC PATCH v4 0/5] DAMON based tiered memory management for CXL memory SeongJae Park
2024-05-12 17:54 ` [RFC PATCH v4 1/5] mm: make alloc_demote_folio externally invokable for migration SeongJae Park
2024-05-13 12:03 ` Honggyu Kim
2024-05-15 16:10 ` SeongJae Park [this message]
2024-05-12 17:54 ` [RFC PATCH v4 2/5] mm/damon/sysfs-schemes: add target_nid on sysfs-schemes SeongJae Park
2024-05-12 17:54 ` [RFC PATCH v4 3/5] mm/migrate: add MR_DAMON to migrate_reason SeongJae Park
2024-05-12 17:54 ` [RFC PATCH v4 4/5] mm/damon/paddr: introduce DAMOS_MIGRATE_COLD action for demotion SeongJae Park
2024-05-12 17:54 ` [RFC PATCH v4 5/5] mm/damon/paddr: introduce DAMOS_MIGRATE_HOT action for promotion SeongJae Park
2024-05-13 11:59 ` [RFC PATCH v4 0/5] DAMON based tiered memory management for CXL memory Honggyu Kim
2024-05-15 16:18 ` SeongJae Park
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240515161040.69242-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=damon@lists.linux.dev \
--cc=honggyu.kim@sk.com \
--cc=kernel_team@skhynix.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).