locking/futex/selftests: Remove duplicate ABI defines

[tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by tip-bot2 for Muhammad Usama Anjum 2 years, 3 months ago

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     d351a9e56cc90a9ff694550e4b3bcaf51a391525
Gitweb:        https://git.kernel.org/tip/d351a9e56cc90a9ff694550e4b3bcaf51a391525
Author:        Muhammad Usama Anjum <usama.anjum@collabora.com>
AuthorDate:    Fri, 06 Oct 2023 14:55:37 +05:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 06 Oct 2023 12:29:45 +02:00

locking/futex/selftests: Remove duplicate ABI defines

Kselftests are kernel tests that are built with kernel headers
from the same source version. The kernel headers, which includes
current ABI definitions, are already being included correctly
in the futex selftest Makefile with the help of KHDR_INCLUDE,
no need to define them again.

Remove duplicate ABI definitions, which is effectively dead code.

No functional changes intended.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20231006095539.1601385-1-usama.anjum@collabora.com
---
 tools/testing/selftests/futex/include/futextest.h | 22 +--------------
 1 file changed, 22 deletions(-)

diff --git a/tools/testing/selftests/futex/include/futextest.h b/tools/testing/selftests/futex/include/futextest.h
index ddbcfc9..59f66af 100644
--- a/tools/testing/selftests/futex/include/futextest.h
+++ b/tools/testing/selftests/futex/include/futextest.h
@@ -25,28 +25,6 @@
 typedef volatile u_int32_t futex_t;
 #define FUTEX_INITIALIZER 0
 
-/* Define the newer op codes if the system header file is not up to date. */
-#ifndef FUTEX_WAIT_BITSET
-#define FUTEX_WAIT_BITSET		9
-#endif
-#ifndef FUTEX_WAKE_BITSET
-#define FUTEX_WAKE_BITSET		10
-#endif
-#ifndef FUTEX_WAIT_REQUEUE_PI
-#define FUTEX_WAIT_REQUEUE_PI		11
-#endif
-#ifndef FUTEX_CMP_REQUEUE_PI
-#define FUTEX_CMP_REQUEUE_PI		12
-#endif
-#ifndef FUTEX_WAIT_REQUEUE_PI_PRIVATE
-#define FUTEX_WAIT_REQUEUE_PI_PRIVATE	(FUTEX_WAIT_REQUEUE_PI | \
-					 FUTEX_PRIVATE_FLAG)
-#endif
-#ifndef FUTEX_REQUEUE_PI_PRIVATE
-#define FUTEX_CMP_REQUEUE_PI_PRIVATE	(FUTEX_CMP_REQUEUE_PI | \
-					 FUTEX_PRIVATE_FLAG)
-#endif
-
 /**
  * futex() - SYS_futex syscall wrapper
  * @uaddr:	address of first futex

Re: [tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by Peter Zijlstra 2 years, 3 months ago

On Fri, Oct 06, 2023 at 10:32:20AM -0000, tip-bot2 for Muhammad Usama Anjum wrote:
> The following commit has been merged into the locking/core branch of tip:
> 
> Commit-ID:     d351a9e56cc90a9ff694550e4b3bcaf51a391525
> Gitweb:        https://git.kernel.org/tip/d351a9e56cc90a9ff694550e4b3bcaf51a391525
> Author:        Muhammad Usama Anjum <usama.anjum@collabora.com>
> AuthorDate:    Fri, 06 Oct 2023 14:55:37 +05:00
> Committer:     Ingo Molnar <mingo@kernel.org>
> CommitterDate: Fri, 06 Oct 2023 12:29:45 +02:00
> 
> locking/futex/selftests: Remove duplicate ABI defines
> 
> Kselftests are kernel tests that are built with kernel headers
> from the same source version. The kernel headers, which includes
> current ABI definitions, are already being included correctly
> in the futex selftest Makefile with the help of KHDR_INCLUDE,
> no need to define them again.
> 
> Remove duplicate ABI definitions, which is effectively dead code.
> 
> No functional changes intended.

so.. as it happens I recently built these things as stand-alone, and
then you ver much end up using the system headers.

Also see 20230922205449.808782861@infradead.org where I add more of
this.

Specifically, if one does:

cd tools/testing/selftests/futex/functional; make

You don't get kernel headers and stuff does not build.

So no, I don't think this is superfluous.

> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Link: https://lore.kernel.org/r/20231006095539.1601385-1-usama.anjum@collabora.com
> ---
>  tools/testing/selftests/futex/include/futextest.h | 22 +--------------
>  1 file changed, 22 deletions(-)
> 
> diff --git a/tools/testing/selftests/futex/include/futextest.h b/tools/testing/selftests/futex/include/futextest.h
> index ddbcfc9..59f66af 100644
> --- a/tools/testing/selftests/futex/include/futextest.h
> +++ b/tools/testing/selftests/futex/include/futextest.h
> @@ -25,28 +25,6 @@
>  typedef volatile u_int32_t futex_t;
>  #define FUTEX_INITIALIZER 0
>  
> -/* Define the newer op codes if the system header file is not up to date. */
> -#ifndef FUTEX_WAIT_BITSET
> -#define FUTEX_WAIT_BITSET		9
> -#endif
> -#ifndef FUTEX_WAKE_BITSET
> -#define FUTEX_WAKE_BITSET		10
> -#endif
> -#ifndef FUTEX_WAIT_REQUEUE_PI
> -#define FUTEX_WAIT_REQUEUE_PI		11
> -#endif
> -#ifndef FUTEX_CMP_REQUEUE_PI
> -#define FUTEX_CMP_REQUEUE_PI		12
> -#endif
> -#ifndef FUTEX_WAIT_REQUEUE_PI_PRIVATE
> -#define FUTEX_WAIT_REQUEUE_PI_PRIVATE	(FUTEX_WAIT_REQUEUE_PI | \
> -					 FUTEX_PRIVATE_FLAG)
> -#endif
> -#ifndef FUTEX_REQUEUE_PI_PRIVATE
> -#define FUTEX_CMP_REQUEUE_PI_PRIVATE	(FUTEX_CMP_REQUEUE_PI | \
> -					 FUTEX_PRIVATE_FLAG)
> -#endif
> -
>  /**
>   * futex() - SYS_futex syscall wrapper
>   * @uaddr:	address of first futex

Re: [tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by Ingo Molnar 2 years, 3 months ago

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Fri, Oct 06, 2023 at 10:32:20AM -0000, tip-bot2 for Muhammad Usama Anjum wrote:
> > The following commit has been merged into the locking/core branch of tip:
> > 
> > Commit-ID:     d351a9e56cc90a9ff694550e4b3bcaf51a391525
> > Gitweb:        https://git.kernel.org/tip/d351a9e56cc90a9ff694550e4b3bcaf51a391525
> > Author:        Muhammad Usama Anjum <usama.anjum@collabora.com>
> > AuthorDate:    Fri, 06 Oct 2023 14:55:37 +05:00
> > Committer:     Ingo Molnar <mingo@kernel.org>
> > CommitterDate: Fri, 06 Oct 2023 12:29:45 +02:00
> > 
> > locking/futex/selftests: Remove duplicate ABI defines
> > 
> > Kselftests are kernel tests that are built with kernel headers
> > from the same source version. The kernel headers, which includes
> > current ABI definitions, are already being included correctly
> > in the futex selftest Makefile with the help of KHDR_INCLUDE,
> > no need to define them again.
> > 
> > Remove duplicate ABI definitions, which is effectively dead code.
> > 
> > No functional changes intended.
> 
> so.. as it happens I recently built these things as stand-alone, and
> then you ver much end up using the system headers.
> 
> Also see 20230922205449.808782861@infradead.org where I add more of
> this.
> 
> Specifically, if one does:
> 
> cd tools/testing/selftests/futex/functional; make
> 
> You don't get kernel headers and stuff does not build.

Hm, I did this after applying the patch, and it does work,
but maybe I missed that those definitions were picked up
from system headers...

So how about we make sure current kernel headers are applied
correctly in a 'standalone' build? There's no reason they
shouldn't be.

Thanks,

	Ingo

Re: [tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by Peter Zijlstra 2 years, 3 months ago

On Fri, Oct 06, 2023 at 12:57:45PM +0200, Ingo Molnar wrote:

> Hm, I did this after applying the patch, and it does work,
> but maybe I missed that those definitions were picked up
> from system headers...

Use an older distro :-) Or apply the FUTEX2 patches and remove the extra
definitions I did there.

Re: [tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by Ingo Molnar 2 years, 3 months ago

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Fri, Oct 06, 2023 at 10:32:20AM -0000, tip-bot2 for Muhammad Usama Anjum wrote:
> > > The following commit has been merged into the locking/core branch of tip:
> > > 
> > > Commit-ID:     d351a9e56cc90a9ff694550e4b3bcaf51a391525
> > > Gitweb:        https://git.kernel.org/tip/d351a9e56cc90a9ff694550e4b3bcaf51a391525
> > > Author:        Muhammad Usama Anjum <usama.anjum@collabora.com>
> > > AuthorDate:    Fri, 06 Oct 2023 14:55:37 +05:00
> > > Committer:     Ingo Molnar <mingo@kernel.org>
> > > CommitterDate: Fri, 06 Oct 2023 12:29:45 +02:00
> > > 
> > > locking/futex/selftests: Remove duplicate ABI defines
> > > 
> > > Kselftests are kernel tests that are built with kernel headers
> > > from the same source version. The kernel headers, which includes
> > > current ABI definitions, are already being included correctly
> > > in the futex selftest Makefile with the help of KHDR_INCLUDE,
> > > no need to define them again.
> > > 
> > > Remove duplicate ABI definitions, which is effectively dead code.
> > > 
> > > No functional changes intended.
> > 
> > so.. as it happens I recently built these things as stand-alone, and
> > then you ver much end up using the system headers.
> > 
> > Also see 20230922205449.808782861@infradead.org where I add more of
> > this.
> > 
> > Specifically, if one does:
> > 
> > cd tools/testing/selftests/futex/functional; make
> > 
> > You don't get kernel headers and stuff does not build.
> 
> Hm, I did this after applying the patch, and it does work,
> but maybe I missed that those definitions were picked up
> from system headers...
> 
> So how about we make sure current kernel headers are applied
> correctly in a 'standalone' build? There's no reason they
> shouldn't be.

Anyway, I've removed this patch from tip:locking/core until
this is cleared up, as your usecase is obviously a valid one ...

Thanks,

	Ingo

Re: [tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by Muhammad Usama Anjum 2 years, 3 months ago

On 10/6/23 4:04 PM, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
>>
>> * Peter Zijlstra <peterz@infradead.org> wrote:
>>
>>> On Fri, Oct 06, 2023 at 10:32:20AM -0000, tip-bot2 for Muhammad Usama Anjum wrote:
>>>> The following commit has been merged into the locking/core branch of tip:
>>>>
>>>> Commit-ID:     d351a9e56cc90a9ff694550e4b3bcaf51a391525
>>>> Gitweb:        https://git.kernel.org/tip/d351a9e56cc90a9ff694550e4b3bcaf51a391525
>>>> Author:        Muhammad Usama Anjum <usama.anjum@collabora.com>
>>>> AuthorDate:    Fri, 06 Oct 2023 14:55:37 +05:00
>>>> Committer:     Ingo Molnar <mingo@kernel.org>
>>>> CommitterDate: Fri, 06 Oct 2023 12:29:45 +02:00
>>>>
>>>> locking/futex/selftests: Remove duplicate ABI defines
>>>>
>>>> Kselftests are kernel tests that are built with kernel headers
>>>> from the same source version. The kernel headers, which includes
>>>> current ABI definitions, are already being included correctly
>>>> in the futex selftest Makefile with the help of KHDR_INCLUDE,
>>>> no need to define them again.
>>>>
>>>> Remove duplicate ABI definitions, which is effectively dead code.
>>>>
>>>> No functional changes intended.
>>>
>>> so.. as it happens I recently built these things as stand-alone, and
>>> then you ver much end up using the system headers.
>>>
>>> Also see 20230922205449.808782861@infradead.org where I add more of
>>> this.
>>>
>>> Specifically, if one does:
>>>
>>> cd tools/testing/selftests/futex/functional; make
>>>
>>> You don't get kernel headers and stuff does not build.
>>
>> Hm, I did this after applying the patch, and it does work,
>> but maybe I missed that those definitions were picked up
>> from system headers...
>>
>> So how about we make sure current kernel headers are applied
>> correctly in a 'standalone' build? There's no reason they
>> shouldn't be.
> 
> Anyway, I've removed this patch from tip:locking/core until
> this is cleared up, as your usecase is obviously a valid one ...

These days a error should appear if the kernel headers aren't found at
build time of kselftests. After building headers, kselftests should be build.

➜  functional (06bc8fe4bfc4b) ✗ pwd
/linux_mainline/tools/testing/selftests/futex/functional
➜  functional (06bc8fe4bfc4b) ✗ make

-e error: missing kernel header files.
Please run this and try again:

    cd /linux_mainline/tools/testing/selftests/../../..
    make headers

make: *** [../../lib.mk:81: kernel_header_files] Error 1
➜  functional (06bc8fe4bfc4b) ✗ (cd $mainline && make headers) > /dev/null
➜  functional (06bc8fe4bfc4b) ✗ make
gcc  -g -O2 -Wall -D_GNU_SOURCE -pthread -I../include -I../../
futex_wait_timeout.c -lpthread -lrt -o
/linux_mainline/tools/testing/selftests/futex/functional/futex_wait_timeout

Before and after applying this patch the behavior is same. I'm doing
testing on next-20231005.

> 
> Thanks,
> 
> 	Ingo

-- 
BR,
Muhammad Usama Anjum

Re: [tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by Peter Zijlstra 2 years, 3 months ago

On Fri, Oct 06, 2023 at 04:05:31PM +0500, Muhammad Usama Anjum wrote:

> These days a error should appear if the kernel headers aren't found at
> build time of kselftests. After building headers, kselftests should be build.
> 
> ➜  functional (06bc8fe4bfc4b) ✗ pwd
> /linux_mainline/tools/testing/selftests/futex/functional
> ➜  functional (06bc8fe4bfc4b) ✗ make
> 
> -e error: missing kernel header files.

Obviously I don't see that. And I would consider this a regression.
Since it means I can't build tests anymore.

Re: [tip: locking/core] locking/futex/selftests: Remove duplicate ABI defines

Posted by Peter Zijlstra 2 years, 3 months ago

On Fri, Oct 06, 2023 at 04:05:31PM +0500, Muhammad Usama Anjum wrote:

>     cd /linux_mainline/tools/testing/selftests/../../..

That's very odd way of writing:

 cd /linux_mainline/

>     make headers

This does a build without O=, I can't do that. 

As in, one must not mix O= and non O= builds, and since I have a ton of
O= output dirs on, I simply can't do this.

> make: *** [../../lib.mk:81: kernel_header_files] Error 1
> ➜  functional (06bc8fe4bfc4b) ✗ (cd $mainline && make headers) > /dev/null
> ➜  functional (06bc8fe4bfc4b) ✗ make
> gcc  -g -O2 -Wall -D_GNU_SOURCE -pthread -I../include -I../../
> futex_wait_timeout.c -lpthread -lrt -o
> /linux_mainline/tools/testing/selftests/futex/functional/futex_wait_timeout

root@noisy:/usr/src/linux-2.6# cd tools/testing/selftests/futex/functional/
root@noisy:/usr/src/linux-2.6/tools/testing/selftests/futex/functional# make
gcc  -g -O2 -Wall -D_GNU_SOURCE -pthread -I../include -I../../       futex_wait_timeout.c -lpthread -lrt -o /mnt/hirez/usr/src/linux-2.6/tools/testing/selftests/futex/functional/futex_wait_timeout
In file included from futex_wait_timeout.c:20:
../include/futex2test.h:18:47: warning: ‘struct futex_waitv’ declared inside parameter list will not be visible outside of this definition or declaration
18 | static inline int futex_waitv(volatile struct futex_waitv *waiters, unsigned long nr_waiters,


Because noisy runs a really old userspace (buster)..