2023-08-05 08:13:31

by Muhammad Usama Anjum

[permalink] [raw]
Subject: [PATCH 6/6] selftests: futex: remove duplicate unneeded defines

Kselftests are kernel tests and must be build with kernel headers from
same source version. These duplicate defines should automatically
picked up from kernel headers. Use KHDR_INCLUDES to add kernel header
files.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
.../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 ddbcfc9b7bac4..59f66af3a6d10 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
--
2.39.2



2023-10-03 08:46:54

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 6/6] selftests: futex: remove duplicate unneeded defines


* Muhammad Usama Anjum <[email protected]> wrote:

> Kselftests are kernel tests and must be build with kernel headers from
> same source version. These duplicate defines should automatically
> picked up from kernel headers. Use KHDR_INCLUDES to add kernel header
> files.
>
> Signed-off-by: Muhammad Usama Anjum <[email protected]>
> ---
> .../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 ddbcfc9b7bac4..59f66af3a6d10 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

AFAICT I cannot really pick this up into the locking tree as-is, as this patch
relies on the KHDR_INCLUDES change in patch #1, so that all self-tests get the
kernel headers included, correct?

Thanks,

Ingo

2023-10-05 14:36:42

by Muhammad Usama Anjum

[permalink] [raw]
Subject: Re: [PATCH 6/6] selftests: futex: remove duplicate unneeded defines

On 10/3/23 1:46 PM, Ingo Molnar wrote:
>
> * Muhammad Usama Anjum <[email protected]> wrote:
>
>> Kselftests are kernel tests and must be build with kernel headers from
>> same source version. These duplicate defines should automatically
>> picked up from kernel headers. Use KHDR_INCLUDES to add kernel header
>> files.
>>
>> Signed-off-by: Muhammad Usama Anjum <[email protected]>
>> ---
>> .../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 ddbcfc9b7bac4..59f66af3a6d10 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
>
> AFAICT I cannot really pick this up into the locking tree as-is, as this patch
> relies on the KHDR_INCLUDES change in patch #1, so that all self-tests get the
> kernel headers included, correct?
No this patch is self contained and doesn't depend on the patch #1.
KHDR_INCLUDES was included several releases back in kselftest's Makefile
and in kselftests of futex. Correct headers are being included already. In
this patch, I'm removing just the un-needed dead code. Other patches were
already picked up by other maintainers.

>
> Thanks,
>
> Ingo

--
BR,
Muhammad Usama Anjum

2023-10-05 20:25:21

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 6/6] selftests: futex: remove duplicate unneeded defines


* Muhammad Usama Anjum <[email protected]> wrote:

> On 10/3/23 1:46 PM, Ingo Molnar wrote:
> >
> > * Muhammad Usama Anjum <[email protected]> wrote:
> >
> >> Kselftests are kernel tests and must be build with kernel headers from
> >> same source version. These duplicate defines should automatically
> >> picked up from kernel headers. Use KHDR_INCLUDES to add kernel header
> >> files.
> >>
> >> Signed-off-by: Muhammad Usama Anjum <[email protected]>
> >> ---
> >> .../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 ddbcfc9b7bac4..59f66af3a6d10 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
> >
> > AFAICT I cannot really pick this up into the locking tree as-is, as this patch
> > relies on the KHDR_INCLUDES change in patch #1, so that all self-tests get the
> > kernel headers included, correct?
> No this patch is self contained and doesn't depend on the patch #1.
> KHDR_INCLUDES was included several releases back in kselftest's Makefile
> and in kselftests of futex. Correct headers are being included already. In
> this patch, I'm removing just the un-needed dead code. Other patches were
> already picked up by other maintainers.

So the changelog does not match that characterization, it talks about
KHDR_INCLUDES in the present tense, and patch #1 adds the KHDR_INCLUDES,
which further suggested a dependency to me:

> >> Kselftests are kernel tests and must be build with kernel headers from
> >> same source version. These duplicate defines should automatically
> >> picked up from kernel headers. Use KHDR_INCLUDES to add kernel header
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >> files.
^^^^^^

Could you please re-send it with the changelog updated that makes it clear
that this patch works fine standalone against current mainline kernels?

Thanks,

Ingo

2023-10-06 08:28:06

by Muhammad Usama Anjum

[permalink] [raw]
Subject: Re: [PATCH 6/6] selftests: futex: remove duplicate unneeded defines

On 10/6/23 1:25 AM, Ingo Molnar wrote:
>
> * Muhammad Usama Anjum <[email protected]> wrote:
>
>> On 10/3/23 1:46 PM, Ingo Molnar wrote:
>>>
>>> * Muhammad Usama Anjum <[email protected]> wrote:
>>>
>>>> Kselftests are kernel tests and must be build with kernel headers from
>>>> same source version. These duplicate defines should automatically
>>>> picked up from kernel headers. Use KHDR_INCLUDES to add kernel header
>>>> files.
>>>>
>>>> Signed-off-by: Muhammad Usama Anjum <[email protected]>
>>>> ---
>>>> .../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 ddbcfc9b7bac4..59f66af3a6d10 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
>>>
>>> AFAICT I cannot really pick this up into the locking tree as-is, as this patch
>>> relies on the KHDR_INCLUDES change in patch #1, so that all self-tests get the
>>> kernel headers included, correct?
>> No this patch is self contained and doesn't depend on the patch #1.
>> KHDR_INCLUDES was included several releases back in kselftest's Makefile
>> and in kselftests of futex. Correct headers are being included already. In
>> this patch, I'm removing just the un-needed dead code. Other patches were
>> already picked up by other maintainers.
>
> So the changelog does not match that characterization, it talks about
> KHDR_INCLUDES in the present tense, and patch #1 adds the KHDR_INCLUDES,
> which further suggested a dependency to me:
Sorry, so the working of the change-log isn't clear enough. I'll send the
patch separately and with better wording.

>
>>>> Kselftests are kernel tests and must be build with kernel headers from
>>>> same source version. These duplicate defines should automatically
>>>> picked up from kernel headers. Use KHDR_INCLUDES to add kernel header
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> files.
> ^^^^^^
>
> Could you please re-send it with the changelog updated that makes it clear
> that this patch works fine standalone against current mainline kernels?
>
> Thanks,
>
> Ingo

--
BR,
Muhammad Usama Anjum

2023-10-06 08:39:56

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 6/6] selftests: futex: remove duplicate unneeded defines


* Muhammad Usama Anjum <[email protected]> wrote:

> Sorry, so the working of the change-log isn't clear enough. I'll send the
> patch separately and with better wording.

Thank you!

Ingo