[PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.

[PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.

[peng8420.li]

From: "peng8420.li" <peng8420.li@gmail.com>

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.

Therefore, remove the "FOLL_TOUCH" test code from gup_test.c;

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 ]---

Signed-off-by: peng8420.li <peng8420.li@gmail.com>
---
 tools/testing/selftests/mm/gup_test.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
index 8900b840c17a..75f7134d529d 100644
--- a/tools/testing/selftests/mm/gup_test.c
+++ b/tools/testing/selftests/mm/gup_test.c
@@ -19,7 +19,6 @@
 
 /* Just the flags we need, copied from mm.h: */
 #define FOLL_WRITE	0x01	/* check pte is writable */
-#define FOLL_TOUCH	0x02	/* mark page accessed */
 
 #define GUP_TEST_FILE "/sys/kernel/debug/gup_test"
 
@@ -93,7 +92,7 @@ int main(int argc, char **argv)
 {
 	struct gup_test gup = { 0 };
 	int filed, i, opt, nr_pages = 1, thp = -1, write = 1, nthreads = 1, ret;
-	int flags = MAP_PRIVATE, touch = 0;
+	int flags = MAP_PRIVATE;
 	char *file = "/dev/zero";
 	pthread_t *tid;
 	char *p;
@@ -170,10 +169,6 @@ int main(int argc, char **argv)
 		case 'H':
 			flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
 			break;
-		case 'z':
-			/* fault pages in gup, do not fault in userland */
-			touch = 1;
-			break;
 		default:
 			ksft_exit_fail_msg("Wrong argument\n");
 		}
@@ -244,18 +239,9 @@ int main(int argc, char **argv)
 	else if (thp == 0)
 		madvise(p, size, MADV_NOHUGEPAGE);
 
-	/*
-	 * FOLL_TOUCH, in gup_test, is used as an either/or case: either
-	 * fault pages in from the kernel via FOLL_TOUCH, or fault them
-	 * in here, from user space. This allows comparison of performance
-	 * between those two cases.
-	 */
-	if (touch) {
-		gup.gup_flags |= FOLL_TOUCH;
-	} else {
-		for (; (unsigned long)p < gup.addr + size; p += psize())
-			p[0] = 0;
-	}
+	/* Fault them in here, from user space. */
+	for (; (unsigned long)p < gup.addr + size; p += psize())
+		p[0] = 0;
 
 	tid = malloc(sizeof(pthread_t) * nthreads);
 	assert(tid);
-- 
2.43.0

[PATCH v2 2/2] Fix the source file annotation information for the "FOLL_WRITE" macro copied.

[peng8420.li]

From: "peng8420.li" <peng8420.li@gmail.com>

The 'FOLL_RITE' of the copied source is located in mm_types. h of mm, not mm. h, so fix it.

Suggested-by: "David Hildenbrand (Red Hat)" <david@kernel.org>
Signed-off-by: peng8420.li <peng8420.li@gmail.com>
---
 tools/testing/selftests/mm/gup_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
index 75f7134d529d..40c1538a17b4 100644
--- a/tools/testing/selftests/mm/gup_test.c
+++ b/tools/testing/selftests/mm/gup_test.c
@@ -17,7 +17,7 @@
 
 #define MB (1UL << 20)
 
-/* Just the flags we need, copied from mm.h: */
+/* Just the flags we need, copied from the kernel internals. */
 #define FOLL_WRITE	0x01	/* check pte is writable */
 
 #define GUP_TEST_FILE "/sys/kernel/debug/gup_test"
-- 
2.43.0

Re: [PATCH v2 2/2] Fix the source file annotation information for the "FOLL_WRITE" macro copied.

[John Hubbard]

On 11/14/25 8:11 AM, peng8420.li@gmail.com wrote:
> From: "peng8420.li" <peng8420.li@gmail.com>

Please see my reply to patch 1/1, about the real name, and the
From: item too.

> 
> The 'FOLL_RITE' of the copied source is located in mm_types. h of mm, not mm. h, so fix it.

"FOLL_WRITE"

Also, please wrap to 72 columns.

> 
> Suggested-by: "David Hildenbrand (Red Hat)" <david@kernel.org>
> Signed-off-by: peng8420.li <peng8420.li@gmail.com>
> ---
>  tools/testing/selftests/mm/gup_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
> index 75f7134d529d..40c1538a17b4 100644
> --- a/tools/testing/selftests/mm/gup_test.c
> +++ b/tools/testing/selftests/mm/gup_test.c
> @@ -17,7 +17,7 @@
>  
>  #define MB (1UL << 20)
>  
> -/* Just the flags we need, copied from mm.h: */
> +/* Just the flags we need, copied from the kernel internals. */
>  #define FOLL_WRITE	0x01	/* check pte is writable */
>  
>  #define GUP_TEST_FILE "/sys/kernel/debug/gup_test"

With the commit log fixed up, please feel free to add:

Reviewed-by: John Hubbard <jhubbard@nvidia.com>

thanks,
-- 
John Hubbard

Re: [PATCH v2 2/2] Fix the source file annotation information for the "FOLL_WRITE" macro copied.

[David Hildenbrand (Red Hat)]

On 14.11.25 21:26, John Hubbard wrote:
> On 11/14/25 8:11 AM, peng8420.li@gmail.com wrote:
>> From: "peng8420.li" <peng8420.li@gmail.com>
> 
> Please see my reply to patch 1/1, about the real name, and the
> From: item too.
> 
>>
>> The 'FOLL_RITE' of the copied source is located in mm_types. h of mm, not mm. h, so fix it.
> 
> "FOLL_WRITE"
> 
> Also, please wrap to 72 columns.
> 
>>
>> Suggested-by: "David Hildenbrand (Red Hat)" <david@kernel.org>
>> Signed-off-by: peng8420.li <peng8420.li@gmail.com>
>> ---
>>   tools/testing/selftests/mm/gup_test.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
>> index 75f7134d529d..40c1538a17b4 100644
>> --- a/tools/testing/selftests/mm/gup_test.c
>> +++ b/tools/testing/selftests/mm/gup_test.c
>> @@ -17,7 +17,7 @@
>>   
>>   #define MB (1UL << 20)
>>   
>> -/* Just the flags we need, copied from mm.h: */
>> +/* Just the flags we need, copied from the kernel internals. */
>>   #define FOLL_WRITE	0x01	/* check pte is writable */
>>   
>>   #define GUP_TEST_FILE "/sys/kernel/debug/gup_test"
> 
> With the commit log fixed up, please feel free to add:
> 
> Reviewed-by: John Hubbard <jhubbard@nvidia.com>

And here the subject should likely be:

"selftests/mm: gup_test: fix comment regarding origin of FOLL_WRITE"

It's usually a good idea to take a look at "git log -s $FILE" to see
how previous commits came up with a subject.


-- 
Cheers

David

Re: [PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.

[John Hubbard]

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 Hubbard

> ---
>  tools/testing/selftests/mm/gup_test.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
> index 8900b840c17a..75f7134d529d 100644
> --- a/tools/testing/selftests/mm/gup_test.c
> +++ b/tools/testing/selftests/mm/gup_test.c
> @@ -19,7 +19,6 @@
>  
>  /* Just the flags we need, copied from mm.h: */
>  #define FOLL_WRITE	0x01	/* check pte is writable */
> -#define FOLL_TOUCH	0x02	/* mark page accessed */
>  
>  #define GUP_TEST_FILE "/sys/kernel/debug/gup_test"
>  
> @@ -93,7 +92,7 @@ int main(int argc, char **argv)
>  {
>  	struct gup_test gup = { 0 };
>  	int filed, i, opt, nr_pages = 1, thp = -1, write = 1, nthreads = 1, ret;
> -	int flags = MAP_PRIVATE, touch = 0;
> +	int flags = MAP_PRIVATE;
>  	char *file = "/dev/zero";
>  	pthread_t *tid;
>  	char *p;
> @@ -170,10 +169,6 @@ int main(int argc, char **argv)
>  		case 'H':
>  			flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
>  			break;
> -		case 'z':
> -			/* fault pages in gup, do not fault in userland */
> -			touch = 1;
> -			break;
>  		default:
>  			ksft_exit_fail_msg("Wrong argument\n");
>  		}
> @@ -244,18 +239,9 @@ int main(int argc, char **argv)
>  	else if (thp == 0)
>  		madvise(p, size, MADV_NOHUGEPAGE);
>  
> -	/*
> -	 * FOLL_TOUCH, in gup_test, is used as an either/or case: either
> -	 * fault pages in from the kernel via FOLL_TOUCH, or fault them
> -	 * in here, from user space. This allows comparison of performance
> -	 * between those two cases.
> -	 */
> -	if (touch) {
> -		gup.gup_flags |= FOLL_TOUCH;
> -	} else {
> -		for (; (unsigned long)p < gup.addr + size; p += psize())
> -			p[0] = 0;
> -	}
> +	/* Fault them in here, from user space. */
> +	for (; (unsigned long)p < gup.addr + size; p += psize())
> +		p[0] = 0;
>  
>  	tid = malloc(sizeof(pthread_t) * nthreads);
>  	assert(tid);

Re: [PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.

[David Hildenbrand (Red Hat)]

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

Re: [PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.

[David Hildenbrand (Red Hat)]

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>

Oh, one thing, subject should be something like

"selftests/mm: gup_test: stop testing FOLL_TOUCH"

-- 
Cheers

David

Re: [PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.

[peng li]

David Hildenbrand (Red Hat) <david@kernel.org> 于2025年11月15日周六 05:39写道：
>
> 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>
>
> Oh, one thing, subject should be something like
>
> "selftests/mm: gup_test: stop testing FOLL_TOUCH"
>

Got it :)
I will revise and release the next version based on your and John
Hubbard's suggestions.
Thank you very much.

> --
> Cheers
>
> David

Re: [PATCH v2 1/2] Remove the "FOLL_TOUCH" test code from gup_test.c.

[peng li]

John Hubbard <jhubbard@nvidia.com> 于2025年11月15日周六 04:25写道：
>
> 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 Hubbard
>
> > ---
> >  tools/testing/selftests/mm/gup_test.c | 22 ++++------------------
> >  1 file changed, 4 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
> > index 8900b840c17a..75f7134d529d 100644
> > --- a/tools/testing/selftests/mm/gup_test.c
> > +++ b/tools/testing/selftests/mm/gup_test.c
> > @@ -19,7 +19,6 @@
> >
> >  /* Just the flags we need, copied from mm.h: */
> >  #define FOLL_WRITE   0x01    /* check pte is writable */
> > -#define FOLL_TOUCH   0x02    /* mark page accessed */
> >
> >  #define GUP_TEST_FILE "/sys/kernel/debug/gup_test"
> >
> > @@ -93,7 +92,7 @@ int main(int argc, char **argv)
> >  {
> >       struct gup_test gup = { 0 };
> >       int filed, i, opt, nr_pages = 1, thp = -1, write = 1, nthreads = 1, ret;
> > -     int flags = MAP_PRIVATE, touch = 0;
> > +     int flags = MAP_PRIVATE;
> >       char *file = "/dev/zero";
> >       pthread_t *tid;
> >       char *p;
> > @@ -170,10 +169,6 @@ int main(int argc, char **argv)
> >               case 'H':
> >                       flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
> >                       break;
> > -             case 'z':
> > -                     /* fault pages in gup, do not fault in userland */
> > -                     touch = 1;
> > -                     break;
> >               default:
> >                       ksft_exit_fail_msg("Wrong argument\n");
> >               }
> > @@ -244,18 +239,9 @@ int main(int argc, char **argv)
> >       else if (thp == 0)
> >               madvise(p, size, MADV_NOHUGEPAGE);
> >
> > -     /*
> > -      * FOLL_TOUCH, in gup_test, is used as an either/or case: either
> > -      * fault pages in from the kernel via FOLL_TOUCH, or fault them
> > -      * in here, from user space. This allows comparison of performance
> > -      * between those two cases.
> > -      */
> > -     if (touch) {
> > -             gup.gup_flags |= FOLL_TOUCH;
> > -     } else {
> > -             for (; (unsigned long)p < gup.addr + size; p += psize())
> > -                     p[0] = 0;
> > -     }
> > +     /* Fault them in here, from user space. */
> > +     for (; (unsigned long)p < gup.addr + size; p += psize())
> > +             p[0] = 0;
> >
> >       tid = malloc(sizeof(pthread_t) * nthreads);
> >       assert(tid);
>
>

Hi John Hubbard,

Your review suggestions were very helpful.
Please understand that I'm new here. Thank you so much!



Thanks & Cheers!