[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1a52c747-3642-4f05-a662-c010240e9e4b@linux.dev>
Date: Mon, 21 Apr 2025 17:11:22 +0800
From: Ye Liu <ye.liu@...ux.dev>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
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
在 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
>> ---
>> 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