Fix some more minor issues in ublk selftests.
The first patch is from https://lore.kernel.org/linux-block/20250423-ublk_selftests-v1-0-7d060e260e7... with a modification requested by Jens. The others are new.
Signed-off-by: Uday Shankar ushankar@purestorage.com --- Uday Shankar (3): selftests: ublk: kublk: build with -Werror iff CONFIG_WERROR=y selftests: ublk: make test_generic_06 silent on success selftests: ublk: kublk: fix include path
tools/testing/selftests/ublk/Makefile | 4 +++- tools/testing/selftests/ublk/kublk.h | 1 - tools/testing/selftests/ublk/test_generic_06.sh | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) --- base-commit: 53ec1abce79c986dc59e59d0c60d00088bcdf32a change-id: 20250428-ublk_selftests-983240d3a325
Best regards,
Compiler warnings can catch bugs at compile time. They can also produce annoying false positives. Due to this duality, the kernel provides CONFIG_WERROR so that the developer can choose whether or not they want compiler warnings to fail the build. Use this same config options to control whether or not warnings in building kublk fail its build.
Signed-off-by: Uday Shankar ushankar@purestorage.com --- tools/testing/selftests/ublk/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile index ec4624a283bce2ebeed80509be6573c1b7a3623d..86474cfe8d03b2df3f8c9bc1a5902701a0f72f58 100644 --- a/tools/testing/selftests/ublk/Makefile +++ b/tools/testing/selftests/ublk/Makefile @@ -1,6 +1,8 @@ # SPDX-License-Identifier: GPL-2.0
-CFLAGS += -O3 -Wl,-no-as-needed -Wall -I $(top_srcdir) +CONFIG = $(top_srcdir)/include/config/auto.conf +WERROR = $(if $(shell grep CONFIG_WERROR=y ${CONFIG}),-Werror,) +CFLAGS += -O3 -Wl,-no-as-needed -Wall ${WERROR} -I $(top_srcdir) LDLIBS += -lpthread -lm -luring
TEST_PROGS := test_generic_01.sh
On Mon, Apr 28, 2025 at 05:10:20PM -0600, Uday Shankar wrote:
Compiler warnings can catch bugs at compile time. They can also produce annoying false positives. Due to this duality, the kernel provides CONFIG_WERROR so that the developer can choose whether or not they want compiler warnings to fail the build. Use this same config options to control whether or not warnings in building kublk fail its build.
Signed-off-by: Uday Shankar ushankar@purestorage.com
tools/testing/selftests/ublk/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile index ec4624a283bce2ebeed80509be6573c1b7a3623d..86474cfe8d03b2df3f8c9bc1a5902701a0f72f58 100644 --- a/tools/testing/selftests/ublk/Makefile +++ b/tools/testing/selftests/ublk/Makefile @@ -1,6 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -O3 -Wl,-no-as-needed -Wall -I $(top_srcdir) +CONFIG = $(top_srcdir)/include/config/auto.conf +WERROR = $(if $(shell grep CONFIG_WERROR=y ${CONFIG}),-Werror,) +CFLAGS += -O3 -Wl,-no-as-needed -Wall ${WERROR} -I $(top_srcdir) LDLIBS += -lpthread -lm -luring
I think it isn't good to reuse kernel CONFIG_WERROR for test code.
But it can be done in the following way by passing 'WERROR=1' to make command line:
+ifneq ($(WERROR),0) + CFLAGS += -Werror +endif
Thanks, Ming
On Tue, Apr 29, 2025 at 09:11:05AM +0800, Ming Lei wrote:
On Mon, Apr 28, 2025 at 05:10:20PM -0600, Uday Shankar wrote:
Compiler warnings can catch bugs at compile time. They can also produce annoying false positives. Due to this duality, the kernel provides CONFIG_WERROR so that the developer can choose whether or not they want compiler warnings to fail the build. Use this same config options to control whether or not warnings in building kublk fail its build.
Signed-off-by: Uday Shankar ushankar@purestorage.com
tools/testing/selftests/ublk/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile index ec4624a283bce2ebeed80509be6573c1b7a3623d..86474cfe8d03b2df3f8c9bc1a5902701a0f72f58 100644 --- a/tools/testing/selftests/ublk/Makefile +++ b/tools/testing/selftests/ublk/Makefile @@ -1,6 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -O3 -Wl,-no-as-needed -Wall -I $(top_srcdir) +CONFIG = $(top_srcdir)/include/config/auto.conf +WERROR = $(if $(shell grep CONFIG_WERROR=y ${CONFIG}),-Werror,) +CFLAGS += -O3 -Wl,-no-as-needed -Wall ${WERROR} -I $(top_srcdir) LDLIBS += -lpthread -lm -luring
I think it isn't good to reuse kernel CONFIG_WERROR for test code.
But it can be done in the following way by passing 'WERROR=1' to make command line:
+ifneq ($(WERROR),0)
CFLAGS += -Werror+endif
I've taken this approach in [1]. It actually passes -Werror by default, but it gives the developer a way to disable it with
make WERROR=0 TARGETS=ublk kselftest
[1] https://lore.kernel.org/linux-block/20250429-ublk_selftests-v2-1-e970b6d9e4f...
Convention dictates that tests should not log anything on success. Make test_generic_06 follow this convention.
Signed-off-by: Uday Shankar ushankar@purestorage.com --- tools/testing/selftests/ublk/test_generic_06.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ublk/test_generic_06.sh b/tools/testing/selftests/ublk/test_generic_06.sh index b67230c42c847c71b0bbe82ad9de1a737ea3cb75..fd42062b7b76b0b3dfae95a39aba6ae28facc185 100755 --- a/tools/testing/selftests/ublk/test_generic_06.sh +++ b/tools/testing/selftests/ublk/test_generic_06.sh @@ -17,7 +17,7 @@ STARTTIME=${SECONDS} dd if=/dev/urandom of=/dev/ublkb${dev_id} oflag=direct bs=4k count=1 status=none > /dev/null 2>&1 & dd_pid=$!
-__ublk_kill_daemon ${dev_id} "DEAD" +__ublk_kill_daemon ${dev_id} "DEAD" >/dev/null
wait $dd_pid dd_exitcode=$?
On Mon, Apr 28, 2025 at 05:10:21PM -0600, Uday Shankar wrote:
Convention dictates that tests should not log anything on success. Make test_generic_06 follow this convention.
Signed-off-by: Uday Shankar ushankar@purestorage.com
Reviewed-by: Ming Lei ming.lei@redhat.com
Thanks, Ming
Building kublk currently fails (with a "could not find linux/ublk_cmd.h" error message) if kernel headers are not installed in a system-global location (i.e. somewhere in the compiler's default include search path). This failure is unnecessary, as make kselftest installs kernel headers in the build tree - kublk's build just isn't looking for them properly. There is an include path in kublk's CFLAGS which is probably intended to find the kernel headers installed in the build tree; fix it so that it can actually find them.
This introduces some macro redefinition issues between glibc-provided headers and kernel headers; fix those by eliminating one include in kublk.
Signed-off-by: Uday Shankar ushankar@purestorage.com --- tools/testing/selftests/ublk/Makefile | 2 +- tools/testing/selftests/ublk/kublk.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile index 86474cfe8d03b2df3f8c9bc1a5902701a0f72f58..feca641635d706a856898f8e22a630f5f47361b6 100644 --- a/tools/testing/selftests/ublk/Makefile +++ b/tools/testing/selftests/ublk/Makefile @@ -2,7 +2,7 @@
CONFIG = $(top_srcdir)/include/config/auto.conf WERROR = $(if $(shell grep CONFIG_WERROR=y ${CONFIG}),-Werror,) -CFLAGS += -O3 -Wl,-no-as-needed -Wall ${WERROR} -I $(top_srcdir) +CFLAGS += -O3 -Wl,-no-as-needed -Wall ${WERROR} -I $(top_srcdir)/usr/include LDLIBS += -lpthread -lm -luring
TEST_PROGS := test_generic_01.sh diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h index 918db5cd633fc1041e1e0805142f00e7e4f28bf7..4b943e57a890e5f14fa11dd19d67c4c8684c4417 100644 --- a/tools/testing/selftests/ublk/kublk.h +++ b/tools/testing/selftests/ublk/kublk.h @@ -19,7 +19,6 @@ #include <sys/inotify.h> #include <sys/wait.h> #include <sys/eventfd.h> -#include <sys/uio.h> #include <sys/ipc.h> #include <sys/shm.h> #include <linux/io_uring.h>
On Mon, Apr 28, 2025 at 4:11 PM Uday Shankar ushankar@purestorage.com wrote:
Building kublk currently fails (with a "could not find linux/ublk_cmd.h" error message) if kernel headers are not installed in a system-global location (i.e. somewhere in the compiler's default include search path). This failure is unnecessary, as make kselftest installs kernel headers in the build tree - kublk's build just isn't looking for them properly. There is an include path in kublk's CFLAGS which is probably intended to find the kernel headers installed in the build tree; fix it so that it can actually find them.
This introduces some macro redefinition issues between glibc-provided headers and kernel headers; fix those by eliminating one include in kublk.
I'm curious what symbol was redefined. struct iovec? Anyways,
Reviewed-by: Caleb Sander Mateos csander@purestorage.com
Signed-off-by: Uday Shankar ushankar@purestorage.com
tools/testing/selftests/ublk/Makefile | 2 +- tools/testing/selftests/ublk/kublk.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile index 86474cfe8d03b2df3f8c9bc1a5902701a0f72f58..feca641635d706a856898f8e22a630f5f47361b6 100644 --- a/tools/testing/selftests/ublk/Makefile +++ b/tools/testing/selftests/ublk/Makefile @@ -2,7 +2,7 @@
CONFIG = $(top_srcdir)/include/config/auto.conf WERROR = $(if $(shell grep CONFIG_WERROR=y ${CONFIG}),-Werror,) -CFLAGS += -O3 -Wl,-no-as-needed -Wall ${WERROR} -I $(top_srcdir) +CFLAGS += -O3 -Wl,-no-as-needed -Wall ${WERROR} -I $(top_srcdir)/usr/include LDLIBS += -lpthread -lm -luring
TEST_PROGS := test_generic_01.sh diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h index 918db5cd633fc1041e1e0805142f00e7e4f28bf7..4b943e57a890e5f14fa11dd19d67c4c8684c4417 100644 --- a/tools/testing/selftests/ublk/kublk.h +++ b/tools/testing/selftests/ublk/kublk.h @@ -19,7 +19,6 @@ #include <sys/inotify.h> #include <sys/wait.h> #include <sys/eventfd.h> -#include <sys/uio.h> #include <sys/ipc.h> #include <sys/shm.h> #include <linux/io_uring.h>
-- 2.34.1
On Mon, Apr 28, 2025 at 06:14:18PM -0700, Caleb Sander Mateos wrote:
On Mon, Apr 28, 2025 at 4:11 PM Uday Shankar ushankar@purestorage.com wrote:
Building kublk currently fails (with a "could not find linux/ublk_cmd.h" error message) if kernel headers are not installed in a system-global location (i.e. somewhere in the compiler's default include search path). This failure is unnecessary, as make kselftest installs kernel headers in the build tree - kublk's build just isn't looking for them properly. There is an include path in kublk's CFLAGS which is probably intended to find the kernel headers installed in the build tree; fix it so that it can actually find them.
This introduces some macro redefinition issues between glibc-provided headers and kernel headers; fix those by eliminating one include in kublk.
I'm curious what symbol was redefined. struct iovec? Anyways,
The RWF_* flags. Here's an example error:
In file included from /root/linux/tools/testing/selftests/../../../usr/include/linux/io_uring.h:11, from kublk.h:25, from kublk.c:6: /root/linux/tools/testing/selftests/../../../usr/include/linux/fs.h:318: error: "RWF_HIPRI" redefined [-Werror] 318 | #define RWF_HIPRI ((__kernel_rwf_t)0x00000001) | In file included from /usr/include/sys/uio.h:179, from kublk.h:22, from kublk.c:6: /usr/include/bits/uio-ext.h:45: note: this is the location of the previous definition 45 | #define RWF_HIPRI 0x00000001 /* High priority request. */ |
On Mon, Apr 28, 2025 at 05:10:22PM -0600, Uday Shankar wrote:
Building kublk currently fails (with a "could not find linux/ublk_cmd.h" error message) if kernel headers are not installed in a system-global location (i.e. somewhere in the compiler's default include search path). This failure is unnecessary, as make kselftest installs kernel headers in the build tree - kublk's build just isn't looking for them properly. There is an include path in kublk's CFLAGS which is probably intended to find the kernel headers installed in the build tree; fix it so that it can actually find them.
This introduces some macro redefinition issues between glibc-provided headers and kernel headers; fix those by eliminating one include in kublk.
Signed-off-by: Uday Shankar ushankar@purestorage.com
kernel selftest is supposed to run under the kernel tree without installing headers system wide, nice!
Reviewed-by: Ming Lei ming.lei@redhat.com
Thanks, Ming
Hi Jens,
Can you take a look at Ming's comment on the first patch and merge the set if things look good? I can rebase/repost it as needed.
Thanks, Uday
On Mon, May 05, 2025 at 02:31:40PM -0600, Uday Shankar wrote:
Hi Jens,
Can you take a look at Ming's comment on the first patch and merge the set if things look good? I can rebase/repost it as needed.
Bleh, sorry, I meant to send this as a reply to v2:
https://lore.kernel.org/linux-block/20250429-ublk_selftests-v2-0-e970b6d9e4f...
On 5/5/25 2:34 PM, Uday Shankar wrote:
On Mon, May 05, 2025 at 02:31:40PM -0600, Uday Shankar wrote:
Hi Jens,
Can you take a look at Ming's comment on the first patch and merge the set if things look good? I can rebase/repost it as needed.
Bleh, sorry, I meant to send this as a reply to v2:
https://lore.kernel.org/linux-block/20250429-ublk_selftests-v2-0-e970b6d9e4f...
Let's give Ming a chance to review v2, then I can get it queued up.
On Mon, May 05, 2025 at 04:44:19PM -0600, Jens Axboe wrote:
On 5/5/25 2:34 PM, Uday Shankar wrote:
On Mon, May 05, 2025 at 02:31:40PM -0600, Uday Shankar wrote:
Hi Jens,
Can you take a look at Ming's comment on the first patch and merge the set if things look good? I can rebase/repost it as needed.
Bleh, sorry, I meant to send this as a reply to v2:
https://lore.kernel.org/linux-block/20250429-ublk_selftests-v2-0-e970b6d9e4f...
Let's give Ming a chance to review v2, then I can get it queued up.
It looks like he has already reviewed all the patches in the set.
On 5/5/25 4:53 PM, Uday Shankar wrote:
On Mon, May 05, 2025 at 04:44:19PM -0600, Jens Axboe wrote:
On 5/5/25 2:34 PM, Uday Shankar wrote:
On Mon, May 05, 2025 at 02:31:40PM -0600, Uday Shankar wrote:
Hi Jens,
Can you take a look at Ming's comment on the first patch and merge the set if things look good? I can rebase/repost it as needed.
Bleh, sorry, I meant to send this as a reply to v2:
https://lore.kernel.org/linux-block/20250429-ublk_selftests-v2-0-e970b6d9e4f...
Let's give Ming a chance to review v2, then I can get it queued up.
It looks like he has already reviewed all the patches in the set.
You're right, was looking at the other v2 that you posted today. Now queued up!
linux-kselftest-mirror@lists.linaro.org
-
Caleb Sander Mateos -
Jens Axboe -
Ming Lei -
Uday Shankar