[PATCH] selftests: user: remove user suite

[PATCH] selftests: user: remove user suite

[Muhammad Usama Anjum]

The user test suite has only one test, test_user_copy which loads
test_user_copy module for testing. But test_user_copy module has already
been converted to kunit (see fixes). Hence remove the entire suite.

Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/Makefile               |  1 -
 tools/testing/selftests/user/Makefile          |  9 ---------
 tools/testing/selftests/user/config            |  1 -
 tools/testing/selftests/user/test_user_copy.sh | 18 ------------------
 4 files changed, 29 deletions(-)
 delete mode 100644 tools/testing/selftests/user/Makefile
 delete mode 100644 tools/testing/selftests/user/config
 delete mode 100755 tools/testing/selftests/user/test_user_copy.sh

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index bc8fe9e8f7f20..af2429431b6b2 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -107,7 +107,6 @@ TARGETS += tmpfs
 TARGETS += tpm2
 TARGETS += tty
 TARGETS += uevent
-TARGETS += user
 TARGETS += user_events
 TARGETS += vDSO
 TARGETS += mm
diff --git a/tools/testing/selftests/user/Makefile b/tools/testing/selftests/user/Makefile
deleted file mode 100644
index 640a40f9b72bc..0000000000000
--- a/tools/testing/selftests/user/Makefile
+++ /dev/null
@@ -1,9 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-# Makefile for user memory selftests
-
-# No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
-all:
-
-TEST_PROGS := test_user_copy.sh
-
-include ../lib.mk
diff --git a/tools/testing/selftests/user/config b/tools/testing/selftests/user/config
deleted file mode 100644
index 784ed8416324d..0000000000000
--- a/tools/testing/selftests/user/config
+++ /dev/null
@@ -1 +0,0 @@
-CONFIG_TEST_USER_COPY=m
diff --git a/tools/testing/selftests/user/test_user_copy.sh b/tools/testing/selftests/user/test_user_copy.sh
deleted file mode 100755
index f9b31a57439b7..0000000000000
--- a/tools/testing/selftests/user/test_user_copy.sh
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-# Runs copy_to/from_user infrastructure using test_user_copy kernel module
-
-# Kselftest framework requirement - SKIP code is 4.
-ksft_skip=4
-
-if ! /sbin/modprobe -q -n test_user_copy; then
-	echo "user: module test_user_copy is not found [SKIP]"
-	exit $ksft_skip
-fi
-if /sbin/modprobe -q test_user_copy; then
-	/sbin/modprobe -q -r test_user_copy
-	echo "user_copy: ok"
-else
-	echo "user_copy: [FAIL]"
-	exit 1
-fi
-- 
2.39.2

Re: [PATCH] selftests: user: remove user suite

[Shuah Khan]

On 7/25/24 05:08, Muhammad Usama Anjum wrote:
> The user test suite has only one test, test_user_copy which loads
> test_user_copy module for testing. But test_user_copy module has already
> been converted to kunit (see fixes). Hence remove the entire suite.
> 
> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")

Remove fixes tag - this isn't a fix and we don't want this propagating
to stable releases without kunit test for this.

> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---

Thanks,
-- Shuah

Re: [PATCH] selftests: user: remove user suite

[Muhammad Usama Anjum]

On 7/25/24 7:44 PM, Shuah Khan wrote:
> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>> The user test suite has only one test, test_user_copy which loads
>> test_user_copy module for testing. But test_user_copy module has already
>> been converted to kunit (see fixes). Hence remove the entire suite.
>>
>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
> 
> Remove fixes tag - this isn't a fix and we don't want this propagating
> to stable releases without kunit test for this.
The user test suite has been failing since cf6219ee889f as the test module
wasn't found. So this is fixing the failure of kselftest. It just causes
noise and may mask other failures. If you still think that fixes isn't
needed, I can send a new version by removing the fixes tag.

> 
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
> 
> Thanks,
> -- Shuah
> 
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH] selftests: user: remove user suite

[Shuah Khan]

On 7/26/24 02:16, Muhammad Usama Anjum wrote:
> On 7/25/24 7:44 PM, Shuah Khan wrote:
>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>> The user test suite has only one test, test_user_copy which loads
>>> test_user_copy module for testing. But test_user_copy module has already
>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>>
>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>>
>> Remove fixes tag - this isn't a fix and we don't want this propagating
>> to stable releases without kunit test for this.
> The user test suite has been failing since cf6219ee889f as the test module
> wasn't found. So this is fixing the failure of kselftest. It just causes
> noise and may mask other failures. If you still think that fixes isn't
> needed, I can send a new version by removing the fixes tag.
> 

In which case this information should have been part of the change log to
make it clear this is a fix. Fixes tag should also mention the releases
this is applicable to so this patch doesn't make it to stables releases
without cf6219ee889f - so this test still runs.

If you are adding Fixes tag it should mention the releases this is
applicable to. Can you get me that information?

thanks,
-- Shuah

Re: [PATCH] selftests: user: remove user suite

[Muhammad Usama Anjum]

On 7/26/24 10:19 PM, Shuah Khan wrote:
> On 7/26/24 02:16, Muhammad Usama Anjum wrote:
>> On 7/25/24 7:44 PM, Shuah Khan wrote:
>>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>>> The user test suite has only one test, test_user_copy which loads
>>>> test_user_copy module for testing. But test_user_copy module has already
>>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>>>
>>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>>>
>>> Remove fixes tag - this isn't a fix and we don't want this propagating
>>> to stable releases without kunit test for this.
>> The user test suite has been failing since cf6219ee889f as the test module
>> wasn't found. So this is fixing the failure of kselftest. It just causes
>> noise and may mask other failures. If you still think that fixes isn't
>> needed, I can send a new version by removing the fixes tag.
>>
> 
> In which case this information should have been part of the change log to
> make it clear this is a fix.
Sorry, I should have mentioned explicitly. Please let me know if I should
send another version for this and the other patch for acceptance [1]?

> Fixes tag should also mention the releases
> this is applicable to so this patch doesn't make it to stables releases
> without cf6219ee889f - so this test still runs.
Release tag is unnecessary as the fix can be applied easily to fixes
commit. From stable-kernel-rules:

  Note, such tagging is unnecessary if the stable team can derive the
  appropriate versions from Fixes: tags.

> 
> If you are adding Fixes tag it should mention the releases this is
> applicable to. Can you get me that information?
> 
> thanks,
> -- Shuah
> 
> 

[1]
https://lore.kernel.org/all/20240725121212.808206-1-usama.anjum@collabora.com

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH] selftests: user: remove user suite

[Shuah Khan]

On 7/25/24 08:44, Shuah Khan wrote:
> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>> The user test suite has only one test, test_user_copy which loads
>> test_user_copy module for testing. But test_user_copy module has already
>> been converted to kunit (see fixes). Hence remove the entire suite.
>>
>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
> 
> Remove fixes tag - this isn't a fix and we don't want this propagating
> to stable releases without kunit test for this.
> 
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
> 
> Thanks,
> -- Shuah
> 

As mentioned in other threads on this conversion to kunit and removal
of kselfttest - NACK on this patch.

Please don't send me any more of these conversion and removal patches.

thanks,
-- Shuah

Re: [PATCH] selftests: user: remove user suite

[Kees Cook]

On July 30, 2024 3:36:11 PM PDT, Shuah Khan <skhan@linuxfoundation.org> wrote:
>On 7/25/24 08:44, Shuah Khan wrote:
>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>> The user test suite has only one test, test_user_copy which loads
>>> test_user_copy module for testing. But test_user_copy module has already
>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>> 
>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>> 
>> Remove fixes tag - this isn't a fix and we don't want this propagating
>> to stable releases without kunit test for this.
>> 
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>> 
>> Thanks,
>> -- Shuah
>> 
>
>As mentioned in other threads on this conversion to kunit and removal
>of kselfttest - NACK on this patch.
>
>Please don't send me any more of these conversion and removal patches.
>

I think there is a misunderstanding about these particular patches (for string and usercopy selftests). Those were already converted, as desired, by the maintainer (me) to KUnit. These associated patches are cleaning up the dangling kselftest part of them, and should land (with the Fixes tag, which is aimed at the commit that did the conversion).

-Kees


-- 
Kees Cook

Re: [PATCH] selftests: user: remove user suite

[Muhammad Usama Anjum]

On 7/31/24 7:33 AM, Kees Cook wrote:
> 
> 
> On July 30, 2024 3:36:11 PM PDT, Shuah Khan <skhan@linuxfoundation.org> wrote:
>> On 7/25/24 08:44, Shuah Khan wrote:
>>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>>> The user test suite has only one test, test_user_copy which loads
>>>> test_user_copy module for testing. But test_user_copy module has already
>>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>>>
>>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>>>
>>> Remove fixes tag - this isn't a fix and we don't want this propagating
>>> to stable releases without kunit test for this.
>>>
>>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>>> ---
>>>
>>> Thanks,
>>> -- Shuah
>>>
>>
>> As mentioned in other threads on this conversion to kunit and removal
>> of kselfttest - NACK on this patch.
>>
>> Please don't send me any more of these conversion and removal patches.
>>
> 
> I think there is a misunderstanding about these particular patches (for string and usercopy selftests). Those were already converted, as desired, by the maintainer (me) to KUnit. These associated patches are cleaning up the dangling kselftest part of them, and should land (with the Fixes tag, which is aimed at the commit that did the conversion).
Yes, this is misunderstanding and these patches should be taken to remove
dead tests in kselftests as their corresponding test modules have been
moved already. So these patches are fixes.

> 
> -Kees
> 
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH] selftests: user: remove user suite

[Shuah Khan]

On 7/30/24 23:13, Muhammad Usama Anjum wrote:
> On 7/31/24 7:33 AM, Kees Cook wrote:
>>
>>
>> On July 30, 2024 3:36:11 PM PDT, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>> On 7/25/24 08:44, Shuah Khan wrote:
>>>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>>>> The user test suite has only one test, test_user_copy which loads
>>>>> test_user_copy module for testing. But test_user_copy module has already
>>>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>>>>
>>>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>>>>
>>>> Remove fixes tag - this isn't a fix and we don't want this propagating
>>>> to stable releases without kunit test for this.
>>>>
>>>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>>>> ---
>>>>
>>>> Thanks,
>>>> -- Shuah
>>>>
>>>
>>> As mentioned in other threads on this conversion to kunit and removal
>>> of kselfttest - NACK on this patch.
>>>
>>> Please don't send me any more of these conversion and removal patches.
>>>
>>
>> I think there is a misunderstanding about these particular patches (for string and usercopy selftests). Those were already converted, as desired, by the maintainer (me) to KUnit. These associated patches are cleaning up the dangling kselftest part of them, and should land (with the Fixes tag, which is aimed at the commit that did the conversion).
> Yes, this is misunderstanding and these patches should be taken to remove
> dead tests in kselftests as their corresponding test modules have been
> moved already. So these patches are fixes.
> 

Thank you both for the clarification

Applied linux-kselftest for Linux 6.12-rc1

thanks,
-- Shuah