Re: [PATCH v5 5/5] mm: selftests: Add shmem into memory failure test

[Ackerley Tng]

Ackerley Tng <ackerleytng@xxxxxxxxxx> writes:

> Lisa Wang <wyihan@xxxxxxxxxx> writes:
>
>> On Mon, Aug 24, 2026 at 03:00:47PM +0200, David Hildenbrand (Arm) wrote:
>>> > +++ b/tools/testing/selftests/mm/memory-failure.c
>>> > @@ -20,6 +20,10 @@
>>> >
>>> >  #include "vm_util.h"
>>> >
>>> > +#ifndef fallthrough
>>> > +# define fallthrough __attribute__((__fallthrough__))
>>> > +#endif
>>>
>>> This doesn't belong into this test.
>>>
>>> rseq/rseq.c selftest already uses it.
>>>
>>> I think tools/testing/include/ already provides it.
>>>
>>> Maybe
>>>
>>> 	#include <linux/compiler.h>
>>>
>>> does the trick?
>>
>> Hi David,
>> Thank you for replying.
>> Would it be better to change it to /* FALLTHROUGH */
>> like the rest of the mm selftests?
>>
>> If we prefer to include <linux/compiler.h>, we can
>> certainly do that, but we would also need to update
>> the Makefile and fix a redefinition in pkey-helpers.h.
>>
>> --- a/tools/testing/selftests/mm/Makefile
>> +++ b/tools/testing/selftests/mm/Makefile
>> @@ -37,7 +37,7 @@ endif
>>  # LDLIBS.
>>  MAKEFLAGS += --no-builtin-rules
>>
>> -CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
>> +CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES) -I$(top_srcdir)/tools/include
>>  CFLAGS += -Wunreachable-code
>>  LDLIBS = -lrt -lpthread -lm
>>
>> --- a/tools/testing/selftests/mm/pkey-helpers.h
>> +++ b/tools/testing/selftests/mm/pkey-helpers.h
>> @@ -22,7 +22,7 @@
>>  typedef __u8   u8;
>>  typedef __u16  u16;
>>  typedef __u32  u32;
>> -typedef __u64  u64;
>> +// typedef __u64       u64;
>
> Removing this u64 typedef seems to be unrelated, the layers of includes
> probably has some weirdness that needs to be ironed out.
>
> If the weirdness is not too complex to iron out to be in time for 7.4, I
> think we can iron that out, if not perhaps using /* FALLTHROUGH */ is
> fine, to be aligned with the other mm selftests.
>

Lisa did some more investigation, and we discussed too. Here's my
understanding:

TOOLS_INCLUDES in the Makefile points to tools/include/uapi, which provides
underscored types like __u64 to avoid polluting userspace symbol names. That
makes sense since tools/include/uapi is generally copied from usr/include/
that is generated at build time, and usr/include/ is used by userspace
programs in general.

selftests are a special case of userspace programs, and since we want selftests
to look similar to the kernel, we want to use u64 instead of __u64. While
the kernel uses include/linux/types.h, the tools subsystem provides its own
userspace compatibility header at tools/include/linux/types.h defining u64.

Hence:

1. tools/include/uapi/linux/types.h provides __u64.
2. tools/include/linux/types.h provides u64.

Adding -I$(top_srcdir)/tools/include switches from 1 to 2, causing
definition conflicts in the mm selftests and more warnings.


Since adding the tools/include path to use
tools/include/linux/compiler.h breaks existing definitions, my vote is
to use the comment-style fallthrough like the other mm selftests and
clean up in a separate patch series.

>>
>>
>> Lisa