[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <01c1cdb2-6b1e-4cb2-8ad8-31448ddb897e@lucifer.local>
Date: Mon, 21 Apr 2025 12:21:42 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Ye Liu <ye.liu@...ux.dev>
Cc: akpm@...ux-foundation.org, nao.horiguchi@...il.com, linmiaohe@...wei.com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
Liam.Howlett@...cle.com, david@...hat.com, harry.yoo@...cle.com,
riel@...riel.com, vbabka@...e.cz, liuye@...inos.cn
Subject: Re: [PATCH v3] mm/rmap: rename page__anon_vma to anon_vma for
consistency
On Mon, Apr 21, 2025 at 05:11:22PM +0800, Ye Liu wrote:
>
> 在 2025/4/21 14:03, Lorenzo Stoakes 写道:
> > On Mon, Apr 21, 2025 at 09:58:23AM +0800, Ye Liu wrote:
> >> From: Ye Liu <liuye@...inos.cn>
> >>
> >> Renamed local variable page__anon_vma in page_address_in_vma() to
> >> anon_vma. The previous naming convention of using double underscores
> >> (__) is unnecessary and inconsistent with typical kernel style, which uses
> >> single underscores to denote local variables. Also updated comments to
> >> reflect the new variable name.
> >>
> >> Functionality unchanged.
> >>
> >> Signed-off-by: Ye Liu <liuye@...inos.cn>
> > Hi Ye,
> >
> > I see you're still getting used to the process :) this patch isn't correct,
> > you're sending a single patch at v3 against a series with 2 patches in it
> > [0], but you have sent it entirely separately - this is not correct.
> >
> > If you want to revise a series, you have a couple choices - you can write a
> > 'fix patch' -in reply to- the patch you are altering, which provides the
> > delta against that patch.
> >
> > This is the preferred way in mm, unless you have made such a big change
> > that this is infeasible or if multiple files change at once or if so much
> > has changed it would be a bit silly to do this.
> >
> > To do this, you use a client like mutt/neomutt/whatever you use (but one
> > that can do the linux-y In-Reply-To stuff correctly) to reply directly to
> > the mail, and then say something like 'hey here's a fix patch please apply,
> > and add the output of the git format-patch that contains the delta against
> > the file at the end after a single line with '----8<----' on it.
> >
> > See [1] for an example.
> >
> > Here, however, you've confused matters a bit by sending this as a v3 when
> > it's not really a v3 properly, so I suggest in _this case_ you just resend
> > the whole thing as a v3 with this correction in place.
> >
> > You should also propagate tags, I gave a reviewed-by tag here, so make sure
> > to include this and others given when you resend the v3 series.
> >
> > Thanks!
> >
> > [0]: https://lore.kernel.org/all/20250418095600.721989-1-ye.liu@linux.dev/
> > [1]: https://lore.kernel.org/linux-mm/13426c71-d069-4407-9340-b227ff8b8736@lucifer.local/
> >
> >> Changes in v3:
> >> - Rename variable from page_anon_vma to anon_vma.
> > Also _please_ copy/paste all changes for all versions each time, and add
> > lore links for each prior version.
> >
> > You can figure out the lore link as https://lore.kernel.org/all/<message id
> > from mail headers>/ - I always do this manually :) you can see I already
> > figured it out for the v2 in [0] above.
>
> Thank you very much for your detailed feedback and guidance! I sincerely
> appreciate you taking the time to explain the correct workflow and the
> options for revising the patch series.
>
> I’ve taken your advice and resent the entire series as v3 with the
> following updates:
>
> - Incorporated the variable rename fix directly into the series.
> - Propagated all relevant tags (including your Reviewed-by).
> - Added complete version history with lore links for v1 and v2.
>
> Your patience and clear instructions are incredibly helpful as I learn
> the community’s processes. If there’s anything else that needs
> adjustment, please let me know.
>
> Thanks,
> Ye Liu
You're welcome :)
The resend LGTM!
>
> >> ---
> >> mm/rmap.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/mm/rmap.c b/mm/rmap.c
> >> index 67bb273dfb80..447e5b57e44f 100644
> >> --- a/mm/rmap.c
> >> +++ b/mm/rmap.c
> >> @@ -789,13 +789,13 @@ unsigned long page_address_in_vma(const struct folio *folio,
> >> const struct page *page, const struct vm_area_struct *vma)
> >> {
> >> if (folio_test_anon(folio)) {
> >> - struct anon_vma *page__anon_vma = folio_anon_vma(folio);
> >> + struct anon_vma *anon_vma = folio_anon_vma(folio);
> >> /*
> >> * Note: swapoff's unuse_vma() is more efficient with this
> >> * check, and needs it to match anon_vma when KSM is active.
> >> */
> >> - if (!vma->anon_vma || !page__anon_vma ||
> >> - vma->anon_vma->root != page__anon_vma->root)
> >> + if (!vma->anon_vma || !anon_vma ||
> >> + vma->anon_vma->root != anon_vma->root)
> >> return -EFAULT;
> >> } else if (!vma->vm_file) {
> >> return -EFAULT;
> >> @@ -803,7 +803,7 @@ unsigned long page_address_in_vma(const struct folio *folio,
> >> return -EFAULT;
> >> }
> >>
> >> - /* KSM folios don't reach here because of the !page__anon_vma check */
> >> + /* KSM folios don't reach here because of the !anon_vma check */
> >> return vma_address(vma, page_pgoff(folio, page), 1);
> >> }
> >>
> >> --
> >> 2.25.1
> >>
Powered by blists - more mailing lists