linux-kernel - Re: [PATCH] selftests/mm: use __auto

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH4c4jJmCP8w8mbeTpOhNvWCgb9U1A+ssny3nSw9Qg5tEnqVcw@mail.gmail.com>
Date: Tue, 26 Aug 2025 21:56:26 +0530
From: Pranav Tyagi <pranav.tyagi03@...il.com>
To: Joshua Hahn <joshua.hahnjy@...il.com>
Cc: akpm@...ux-foundation.org, peterx@...hat.com, shuah@...nel.org, 
	linux-mm@...ck.org, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH] selftests/mm: use __auto_type in swap() macro

On Wed, Aug 6, 2025 at 10:18 PM Joshua Hahn <joshua.hahnjy@...il.com> wrote:
>
> On Wed, 30 Jul 2025 19:53:01 +0530 Pranav Tyagi <pranav.tyagi03@...il.com> wrote:
>
> Hi Pranav,
>
> Thank you for this patch! Sorry for the late response, thank you for bumping
> the patch. I just have a few comments about the commit message.
>
> > Replace typeof() with __auto_type in the swap() macro in uffd-stress.c.
> > __auto_type was introduced in GCC 4.9 and reduces the compile time for
> > all compilers. No functional changes intended.
>
> From what I understand, the compiler optimizations from typeof() --> __auto_type
> applies when the argument is a variably modified type. It seems like this
> internal definition of swap() is only used within selftests/mm/uffd-stress.c,
> and in particular is only used twice, in these two lines:
>
> /* prepare next bounce */
> swap(area_src, area_dst);
>
> swap(area_src_alias, area_dst_alias);
>
> Where area_src, area_dst, area_src_alias, area_dst_alias are all char * which
> the compiler knows the size of at compile time. Given this, I wonder if there
> are any compile time speedups.
>
> But this is just my understanding, based on a quick look at the code. Please
> feel free to correct me, if I am incorrectly understanding your changes or if
> my understanding of __auto_type is incorrect : -)
>
> With that said, I think the main benefit that we get from using __auto_type
> has more to do with readability. Since __auto_type can only be used to
> declare local variables (as we are doing here), maybe we can rephrase the
> commit to be about improving readability, and not compile time?
>
> Again, please let me know if I am overlooking something.
>
> One other thought -- while this internal definition of swap() is confined to
> selftests/mm/uffd-stress.c, there is another identical definition in
> include/linux/minmax.h that may be used more widely across not only mm
> stresstests, but across subsystems as well. Without having taken a deeper
> look into this, perhaps this version of swap is indeed called on some data
> structures with variable type, and we might be able to see some tangible
> compile time improvements?
>
> In any case, this change looks good to me, but maybe a new commit message
> that can more closely describe the effects would be better : -) Once you add
> those changes, please feel free to add my review tag for the mm selftest change:
>
> Reviewed-by: Joshua Hahn <joshua.hahnjy@...il.com>
>
> > Signed-off-by: Pranav Tyagi <pranav.tyagi03@...il.com>
> > ---
> >  tools/testing/selftests/mm/uffd-stress.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
> > index 40af7f67c407..c0f64df5085c 100644
> > --- a/tools/testing/selftests/mm/uffd-stress.c
> > +++ b/tools/testing/selftests/mm/uffd-stress.c
> > @@ -51,7 +51,7 @@ static char *zeropage;
> >  pthread_attr_t attr;
> >
> >  #define swap(a, b) \
> > -     do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
> > +     do { __auto_type __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
> >
> >  const char *examples =
> >       "# Run anonymous memory test on 100MiB region with 99999 bounces:\n"
> > --
> > 2.49.0
>
> Sent using hkml (https://github.com/sjp38/hackermail)

Hi Joshua,

Thanks for the detailed review and I sincerely apologize for the delayed
response.

You’re correct — __auto_type mainly provides compiler optimizations when the
argument is a variably modified type and in this case the compiler already
knows the size of the arguments at compile time. The motivation here is more
about readability and consistency, as __auto_type is already used in several
places within selftests. This patch has since been picked up into the
mm-unstable branch.

I also looked at the swap() definition in include/linux/minmax.h, which is
indeed more widely used across subsystems. That version may offer tangible
compile-time improvements in cases involving variable argument types and I
plan to follow up with a patch there.

Thanks again for the thorough review.

Regards
Pranav Tyagi