linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "David Hildenbrand (Red Hat)" <david@kernel.org>
To: John Hubbard <jhubbard@nvidia.com>,
	peng8420.li@gmail.com, linux-mm@kvack.org,
	akpm@linux-foundation.org
Cc: osalvador@suse.de, jgg@ziepe.ca, peterx@redhat.com,
	linux-kernel@vger.kernel.org, dan.j.williams@intel.com
Subject: Re: [PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.
Date: Fri, 14 Nov 2025 22:37:16 +0100	[thread overview]
Message-ID: <0e0fb943-1d42-4b4b-a4e4-32e95a0dbe6a@kernel.org> (raw)
In-Reply-To: <6869aee5-0093-489d-bac7-fa718bdcb362@nvidia.com>

On 14.11.25 21:24, John Hubbard wrote:
> Hi,
> 
> The diffs look good, but the commit log and your replies have many minor
> issues that add up. So you'll want to avoid them in the future, and with
> that in mind, I've listed all of them, along with suggested alternatives.
> 
> On 11/14/25 8:11 AM, peng8420.li@gmail.com wrote:
>> From: "peng8420.li" <peng8420.li@gmail.com>
>   
> This should be your real name, such as (my first guess)
> "Peng Li" <peng8420.li@gmail.com>.
> 
> Also, if you are sending from the same email address as your "From:",
> then the "From:" item shouldn't normally show up (it's harmless here,
> but could be cleaner without it). Check your .gitconfig send-email
> options and you can make it go away.
> 
> Also, when replying, your email client is apparently using HTML, please
> fix it to use text only!
> 
> Please read the patch submitter guidelines, too:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submit-checklist.rst
> 
>> Ever since commit 0f20bba1688b ("mm/gup: explicitly define and check
>> internal GUP flags, disallow FOLL_TOUCH") we marked FOLL_TOUCH as a GUP-internal flag.
> 
> Not "ever since", but simply:
> 
> commit 0f20bba1688b ("mm/gup: explicitly define and check internal GUP
> flags, disallow FOLL_TOUCH") marked FOLL_TOUCH as a GUP-internal flag.
> 
> Also, it's best to keep the line length within about 72 columns for
> commit messages.
> 
>>
>> Therefore, remove the "FOLL_TOUCH" test code from gup_test.c;
> 
> Trailing semicolon. Should be a period.
> 
>>
>> Otherwise, executing the test command "./gup_test -L -r 100 -z" will report the following warning log:
>> TAP version 13
>> 1..1
>> ENCHMARK: Time: [   39.363371] WARNING: CPU: 1 PID: 117 at mm/gup.c:2512 is_valid_gup_args+0x66/0x8c
>> get:2818 put:46 [   39.364043] Modules linked in:
>> us# , truncated [   39.364311] CPU: 1 UID: 0 PID: 117 Comm: gup_test Not tainted 6.18.0-rc5-00324-gd09eaf415c87 #29 NONE
>> (size: 0)#
>> [   39.364434] Hardware name: riscv-virtio,qemu (DT)
>> [   39.364546] epc : is_valid_gup_args+0x66/0x8c
>> [   39.364596]  ra : pin_user_pages+0x38/0x78
>> [   39.364630] epc : ffffffff802079e6 ra : ffffffff8020c214 sp : ff2000000041bd20
>> [   39.364650]  gp : ffffffff81a26068 tp : ff60000080b3b000 t0 : ff2000000041bdf8
>> [   39.364678]  t1 : 000000000000001e t2 : 0000000000000000 s0 : ff2000000041bd30
>> [   39.364697]  s1 : ff60000081300000 a0 : ff60000081300000 a1 : 0000000000000000
>> [   39.364714]  a2 : ff2000000041bd3c a3 : 0000000000080000 a4 : 0000000000000001
>> [   39.364731]  a5 : 0000000000010101 a6 : 0000000000000001 a7 : 0000000000000000
>> [   39.364747]  s2 : 00007fff7eeed000 s3 : 0000000000000001 s4 : 00007fff7eeee000
>> [   39.364761]  s5 : 00007fff7eeeb838 s6 : 0000000000000000 s7 : 00007fff7eeed000
>> [   39.364825]  s8 : ff60000081300000 s9 : 0000000000000000 s10: 0000000000000002
>> [   39.364842]  s11: 000000092869bfdc t3 : 2152ffffffffffc0 t4 : 00000000001fffff
>> [   39.364855]  t5 : ffffffffffffffff t6 : 0000000000000000
>> [   39.364867] status: 0000000200000120 badaddr: ffffffff802079e6 cause: 0000000000000003
>> [   39.364949] [<ffffffff802079e6>] is_valid_gup_args+0x66/0x8c
>> [   39.365036] [<ffffffff8020c214>] pin_user_pages+0x38/0x78
>> [   39.365049] [<ffffffff80278f78>] gup_test_ioctl+0x2b4/0xc08
>> [   39.365060] [<ffffffff80297da6>] __riscv_sys_ioctl+0xba/0xc4
>> [   39.365072] [<ffffffff80b93f96>] do_trap_ecall_u+0x296/0x370
>> [   39.365093] [<ffffffff80b9ef06>] handle_exception+0x146/0x152
>> [   39.365220] ---[ end trace 0000000000000000 ]---
>>
> 
> We don't need all of this output for such a simple case. Just the
> warning will suffice, and even that is optional for this patch.
> 
> 
>> Signed-off-by: peng8420.li <peng8420.li@gmail.com>
> 
> Same rules for the Signed-off-by: real name needed here.
> 
> Combining all of that, your commit message would look like this:
> 
> 
> From: "Peng Li" <peng8420.li@gmail.com>
> 
> commit 0f20bba1688b ("mm/gup: explicitly define and check internal GUP
> flags, disallow FOLL_TOUCH") marked FOLL_TOUCH as a GUP-internal flag.
> 
> This causes a warning to fire when running gup_test, for example:
> 
>      $ ./gup_test -L -r 100 -z
> 
> dmesg:
>      WARNING: CPU: 1 PID: 117 at mm/gup.c:2512 is_valid_gup_args+0x66/0x8c
> 
> Therefore, remove the "FOLL_TOUCH" test code from gup_test.c.
> 
> Signed-off-by: Peng Li <peng8420.li@gmail.com>
> 
> With the commit log fixed up, please feel free to add:
> 
> Reviewed-by: John Hubbard <jhubbard@nvidia.com>

Thanks John!

With that

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David


  reply	other threads:[~2025-11-14 21:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14 16:11 peng8420.li
2025-11-14 16:11 ` [PATCH v2 2/2] Fix the source file annotation information for the "FOLL_WRITE" macro copied peng8420.li
2025-11-14 20:26   ` John Hubbard
2025-11-14 21:41     ` David Hildenbrand (Red Hat)
2025-11-14 20:24 ` [PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c John Hubbard
2025-11-14 21:37   ` David Hildenbrand (Red Hat) [this message]
2025-11-14 21:39   ` David Hildenbrand (Red Hat)
2025-11-17  7:09     ` peng li
2025-11-17  7:05   ` peng li

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=0e0fb943-1d42-4b4b-a4e4-32e95a0dbe6a@kernel.org \
    --to=david@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=jgg@ziepe.ca \
    --cc=jhubbard@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=osalvador@suse.de \
    --cc=peng8420.li@gmail.com \
    --cc=peterx@redhat.com \
    /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