On Wed, 30 Jul 2025 19:53:01 +0530 Pranav Tyagi pranav.tyagi03@gmail.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@gmail.com
Signed-off-by: Pranav Tyagi pranav.tyagi03@gmail.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)