[LTP] [PATCH 0/2] Improve ioprio tests

[LTP] [PATCH 0/2] Improve ioprio tests

[Damien Le Moal]

The ioprio syscall tests rely on ltp internal definitions of the
IOPRIO_XXX() macro defining priority classes and levels. With changes
to the ioprio API to support command duration limits, these internal
definitions are incomplete, causing test case ioprio_set03 to fail.

Avoid this issue by having the iprio tests use the kernel header file
definitions if that header exists. This enables additional checks in
the header file [1] which restores the expected results with test
ioprio_set03.

[1] https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/

Note: a review of this patch on the kernel block mailing list would be
very appreciated.

Damien Le Moal (2):
  ioprio: use ioprio.h kernel header if it exists
  ioprio: Use IOPRIO_PRIO_NUM to check prio range

 configure.ac                                  |  1 +
 testcases/kernel/syscalls/ioprio/ioprio.h     | 33 ++++++++++++++-----
 .../kernel/syscalls/ioprio/ioprio_set03.c     |  2 +-
 3 files changed, 26 insertions(+), 10 deletions(-)

-- 
2.40.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/2] ioprio: use ioprio.h kernel header if it exists

[Damien Le Moal]

For the ioprio system call test cases, avoid blindly defining the
IOPRIO_XXX macro internally and instead use the kernel user API header
file if it exists. Given that the definitions in this header file have
changed over time, make sure to test for the existence of the macro
IOPRIO_PRIO_LEVEL macro and define it if it does not exist. Similarly,
use IOPRIO_NR_LEVELS to define IOPRIO_PRIO_NUM if that macro exists.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 configure.ac                              |  1 +
 testcases/kernel/syscalls/ioprio/ioprio.h | 29 +++++++++++++++++------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 548288310..e4aa2cadf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -56,6 +56,7 @@ AC_CHECK_HEADERS_ONCE([ \
     linux/if_ether.h \
     linux/if_packet.h \
     linux/io_uring.h \
+    linux/ioprio.h \
     linux/keyctl.h \
     linux/mempolicy.h \
     linux/module.h \
diff --git a/testcases/kernel/syscalls/ioprio/ioprio.h b/testcases/kernel/syscalls/ioprio/ioprio.h
index c74380475..6ca134a54 100644
--- a/testcases/kernel/syscalls/ioprio/ioprio.h
+++ b/testcases/kernel/syscalls/ioprio/ioprio.h
@@ -6,6 +6,12 @@
 #ifndef LTP_IOPRIO_H
 #define LTP_IOPRIO_H
 
+#ifdef HAVE_LINUX_IOPRIO_H
+
+# include <linux/ioprio.h>
+
+#else
+
 enum {
 	IOPRIO_CLASS_NONE = 0,
 	IOPRIO_CLASS_RT,
@@ -19,15 +25,24 @@ enum {
 	IOPRIO_WHO_USER,
 };
 
-/* The I/O scheduler classes have 8 priorities 0..7 except for the IDLE class */
-#define IOPRIO_PRIO_NUM		8
+# define IOPRIO_CLASS_SHIFT	(13)
+# define IOPRIO_PRIO_MASK	((1UL << IOPRIO_CLASS_SHIFT) - 1)
+
+# define IOPRIO_PRIO_CLASS(data)	((data) >> IOPRIO_CLASS_SHIFT)
+# define IOPRIO_PRIO_VALUE(class, data)	(((class) << IOPRIO_CLASS_SHIFT) | data)
 
-#define IOPRIO_CLASS_SHIFT	(13)
-#define IOPRIO_PRIO_MASK	((1UL << IOPRIO_CLASS_SHIFT) - 1)
+#endif
+
+/* The RT and BE I/O priority classes have 8 priority levels 0..7 */
+#ifdef IOPRIO_NR_LEVELS
+# define IOPRIO_PRIO_NUM		IOPRIO_NR_LEVELS
+#else
+# define IOPRIO_PRIO_NUM		8
+#endif
 
-#define IOPRIO_PRIO_CLASS(data)	((data) >> IOPRIO_CLASS_SHIFT)
-#define IOPRIO_PRIO_LEVEL(data)	((data) & IOPRIO_PRIO_MASK)
-#define IOPRIO_PRIO_VALUE(class, data)	(((class) << IOPRIO_CLASS_SHIFT) | data)
+#ifndef IOPRIO_PRIO_LEVEL
+# define IOPRIO_PRIO_LEVEL(data)	((data) & IOPRIO_PRIO_MASK)
+#endif
 
 static const char * const to_class_str[] = {
 	[IOPRIO_CLASS_NONE] = "NONE",
-- 
2.40.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/2] ioprio: Use IOPRIO_PRIO_NUM to check prio range

[Damien Le Moal]

Use the macro IOPRIO_PRIO_NUM in prio_in_range() to check the upper
bound of the valid range for priority levels. Similarly, in the test
case ioprio_set03, use this macro to check for failures when the user
attempts using a priority level out of range.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 testcases/kernel/syscalls/ioprio/ioprio.h       | 4 ++--
 testcases/kernel/syscalls/ioprio/ioprio_set03.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/ioprio/ioprio.h b/testcases/kernel/syscalls/ioprio/ioprio.h
index 6ca134a54..c2115bf20 100644
--- a/testcases/kernel/syscalls/ioprio/ioprio.h
+++ b/testcases/kernel/syscalls/ioprio/ioprio.h
@@ -61,10 +61,10 @@ static inline int sys_ioprio_set(int which, int who, int ioprio)
 	return tst_syscall(__NR_ioprio_set, which, who, ioprio);
 }
 
-/* Priority range from 0 (highest) to 7 (lowest) */
+/* Priority range from 0 (highest) to IOPRIO_PRIO_NUM (lowest) */
 static inline int prio_in_range(int prio)
 {
-	if ((prio < 0) || (prio > 7))
+	if ((prio < 0) || (prio >= IOPRIO_PRIO_NUM))
 		return 0;
 	return 1;
 }
diff --git a/testcases/kernel/syscalls/ioprio/ioprio_set03.c b/testcases/kernel/syscalls/ioprio/ioprio_set03.c
index b2c962a6f..d6b44df85 100644
--- a/testcases/kernel/syscalls/ioprio/ioprio_set03.c
+++ b/testcases/kernel/syscalls/ioprio/ioprio_set03.c
@@ -27,7 +27,7 @@ static void run(void)
 	sys_ioprio_set(IOPRIO_WHO_PROCESS, 0,
 		       IOPRIO_PRIO_VALUE(class, 4));
 	TEST(sys_ioprio_set(IOPRIO_WHO_PROCESS, 0,
-			    IOPRIO_PRIO_VALUE(class, 8)));
+			    IOPRIO_PRIO_VALUE(class, IOPRIO_PRIO_NUM)));
 	if (TST_RET == -1) {
 		ioprio_check_setting(class, 4, 1);
 		if (errno == EINVAL)
-- 
2.40.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] ioprio: use ioprio.h kernel header if it exists

[Linus Walleij]

On Mon, Jun 5, 2023 at 6:41 AM Damien Le Moal <dlemoal@kernel.org> wrote:

> For the ioprio system call test cases, avoid blindly defining the
> IOPRIO_XXX macro internally and instead use the kernel user API header
> file if it exists. Given that the definitions in this header file have
> changed over time, make sure to test for the existence of the macro
> IOPRIO_PRIO_LEVEL macro and define it if it does not exist. Similarly,
> use IOPRIO_NR_LEVELS to define IOPRIO_PRIO_NUM if that macro exists.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioprio: Use IOPRIO_PRIO_NUM to check prio range

[Linus Walleij]

On Mon, Jun 5, 2023 at 6:41 AM Damien Le Moal <dlemoal@kernel.org> wrote:

> Use the macro IOPRIO_PRIO_NUM in prio_in_range() to check the upper
> bound of the valid range for priority levels. Similarly, in the test
> case ioprio_set03, use this macro to check for failures when the user
> attempts using a priority level out of range.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioprio: Use IOPRIO_PRIO_NUM to check prio range

[Damien Le Moal]

On 6/7/23 15:44, Linus Walleij wrote:
> On Mon, Jun 5, 2023 at 6:41 AM Damien Le Moal <dlemoal@kernel.org> wrote:
> 
>> Use the macro IOPRIO_PRIO_NUM in prio_in_range() to check the upper
>> bound of the valid range for priority levels. Similarly, in the test
>> case ioprio_set03, use this macro to check for failures when the user
>> attempts using a priority level out of range.
>>
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks Linus !

May I also ask for your review of the kernel ioprio API patch ?
It is here:

https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/

> 
> Yours,
> Linus Walleij

-- 
Damien Le Moal
Western Digital Research


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioprio: Use IOPRIO_PRIO_NUM to check prio range

[Linus Walleij]

On Thu, Jun 8, 2023 at 2:55 AM Damien Le Moal <dlemoal@kernel.org> wrote:

> Thanks Linus !
>
> May I also ask for your review of the kernel ioprio API patch ?
> It is here:
>
> https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/

Sadly I don't have this patch in my inbox, because it appears I fell
off the block mailing list and wasn't on To or Cc. If you resend
then include me on CC and I'll have a look!

Yours,
Linus Walleij

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] Improve ioprio tests

[Petr Vorel]

Hi Damien,

> The ioprio syscall tests rely on ltp internal definitions of the
> IOPRIO_XXX() macro defining priority classes and levels. With changes
> to the ioprio API to support command duration limits, these internal
> definitions are incomplete, causing test case ioprio_set03 to fail.

> Avoid this issue by having the iprio tests use the kernel header file
> definitions if that header exists. This enables additional checks in
> the header file [1] which restores the expected results with test
> ioprio_set03.

> [1] https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/

> Note: a review of this patch on the kernel block mailing list would be
> very appreciated.

I haven't found this patchset on https://lore.kernel.org/linux-block/,
did you send it there?

Kind regards,
Petr

> Damien Le Moal (2):
>   ioprio: use ioprio.h kernel header if it exists
>   ioprio: Use IOPRIO_PRIO_NUM to check prio range

>  configure.ac                                  |  1 +
>  testcases/kernel/syscalls/ioprio/ioprio.h     | 33 ++++++++++++++-----
>  .../kernel/syscalls/ioprio/ioprio_set03.c     |  2 +-
>  3 files changed, 26 insertions(+), 10 deletions(-)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] Improve ioprio tests

[Niklas Cassel via ltp]

On Tue, Jun 20, 2023 at 11:13:16AM +0200, Petr Vorel wrote:
> Hi Damien,
> 
> > The ioprio syscall tests rely on ltp internal definitions of the
> > IOPRIO_XXX() macro defining priority classes and levels. With changes
> > to the ioprio API to support command duration limits, these internal
> > definitions are incomplete, causing test case ioprio_set03 to fail.
> 
> > Avoid this issue by having the iprio tests use the kernel header file
> > definitions if that header exists. This enables additional checks in
> > the header file [1] which restores the expected results with test
> > ioprio_set03.
> 
> > [1] https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/
> 
> > Note: a review of this patch on the kernel block mailing list would be
> > very appreciated.
> 
> I haven't found this patchset on https://lore.kernel.org/linux-block/,
> did you send it there?

Hello Petr,

The patch series for LTP can be found here:
https://lore.kernel.org/ltp/20230608005315.3703446-1-dlemoal@kernel.org/T/#t

I just checked LTP master, and it hasn't been applied by the
LTP maintainers yet.


Kind regards,
Niklas

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] Improve ioprio tests

[Niklas Cassel via ltp]

On Tue, Jun 20, 2023 at 11:16:49AM +0200, Niklas Cassel wrote:
> On Tue, Jun 20, 2023 at 11:13:16AM +0200, Petr Vorel wrote:
> > Hi Damien,
> > 
> > > The ioprio syscall tests rely on ltp internal definitions of the
> > > IOPRIO_XXX() macro defining priority classes and levels. With changes
> > > to the ioprio API to support command duration limits, these internal
> > > definitions are incomplete, causing test case ioprio_set03 to fail.
> > 
> > > Avoid this issue by having the iprio tests use the kernel header file
> > > definitions if that header exists. This enables additional checks in
> > > the header file [1] which restores the expected results with test
> > > ioprio_set03.
> > 
> > > [1] https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/
> > 
> > > Note: a review of this patch on the kernel block mailing list would be
> > > very appreciated.
> > 
> > I haven't found this patchset on https://lore.kernel.org/linux-block/,
> > did you send it there?
> 
> Hello Petr,
> 
> The patch series for LTP can be found here:
> https://lore.kernel.org/ltp/20230608005315.3703446-1-dlemoal@kernel.org/T/#t
> 
> I just checked LTP master, and it hasn't been applied by the
> LTP maintainers yet.
> 
> 
> Kind regards,
> Niklas

Oh, and in case you were referring to the prerequisite kernel patch,
it is already in linux-next:

https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=staging&id=01584c1e233740519d0e11aa20daa323d26bf598

So right now we are just waiting for the LTP patch series to get picked up.


Kind regards,
Niklas

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] Improve ioprio tests

[Petr Vorel]

Hi all,

> On Tue, Jun 20, 2023 at 11:13:16AM +0200, Petr Vorel wrote:
> > Hi Damien,

> > > The ioprio syscall tests rely on ltp internal definitions of the
> > > IOPRIO_XXX() macro defining priority classes and levels. With changes
> > > to the ioprio API to support command duration limits, these internal
> > > definitions are incomplete, causing test case ioprio_set03 to fail.

> > > Avoid this issue by having the iprio tests use the kernel header file
> > > definitions if that header exists. This enables additional checks in
> > > the header file [1] which restores the expected results with test
> > > ioprio_set03.

> > > [1] https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/

> > > Note: a review of this patch on the kernel block mailing list would be
> > > very appreciated.

I'm sorry, I overlooked Linus' review [2], because it's not visible in
patchwork [3].

@Damien, @Linus, @Niklas FYI you need to be subscribed to post to LTP ML (unlike
vger), otherwise the message is held (and we don't check for held messages very often).
@Niklas I had to push your message due this.

> > I haven't found this patchset on https://lore.kernel.org/linux-block/,
> > did you send it there?

> Hello Petr,

> The patch series for LTP can be found here:
> https://lore.kernel.org/ltp/20230608005315.3703446-1-dlemoal@kernel.org/T/#t

Yes, it's in our LTP mailing list, thus on LTP on lore. But I expected that
linux-block ML would be in Cc (that's why Linus Walleij didn't get it in his
mailbox [4] => @Damien, do not hesitate to Cc relevant kernel (or other mailing
lists), if you expect kernel maintainers to do the review.

> I just checked LTP master, and it hasn't been applied by the
> LTP maintainers yet.

Yes, but do not worry, I'll merge it soon (likely today).

Thank to all for your time.

Kind regards,
Petr

[2] https://lore.kernel.org/ltp/CACRpkdaBinsAofvQgLZ5u8ScR0+yWPnQCf6E7CPtn598PN0eoQ@mail.gmail.com/
[3] https://patchwork.ozlabs.org/project/ltp/patch/20230605044131.798383-2-dlemoal@kernel.org/
[4] https://lore.kernel.org/ltp/CACRpkdaD3xM=Z0xTzHa+yH-wZ+LyhrSb3ZzGzkGdnboCaHw_+A@mail.gmail.com/

> Kind regards,
> Niklas

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] Improve ioprio tests

[Petr Vorel]

> On Tue, Jun 20, 2023 at 11:16:49AM +0200, Niklas Cassel wrote:
> > On Tue, Jun 20, 2023 at 11:13:16AM +0200, Petr Vorel wrote:
> > > Hi Damien,

> > > > The ioprio syscall tests rely on ltp internal definitions of the
> > > > IOPRIO_XXX() macro defining priority classes and levels. With changes
> > > > to the ioprio API to support command duration limits, these internal
> > > > definitions are incomplete, causing test case ioprio_set03 to fail.

> > > > Avoid this issue by having the iprio tests use the kernel header file
> > > > definitions if that header exists. This enables additional checks in
> > > > the header file [1] which restores the expected results with test
> > > > ioprio_set03.

> > > > [1] https://lore.kernel.org/linux-block/20230530061307.525644-1-dlemoal@kernel.org/

> > > > Note: a review of this patch on the kernel block mailing list would be
> > > > very appreciated.

> > > I haven't found this patchset on https://lore.kernel.org/linux-block/,
> > > did you send it there?

> > Hello Petr,

> > The patch series for LTP can be found here:
> > https://lore.kernel.org/ltp/20230608005315.3703446-1-dlemoal@kernel.org/T/#t

> > I just checked LTP master, and it hasn't been applied by the
> > LTP maintainers yet.


> > Kind regards,
> > Niklas

> Oh, and in case you were referring to the prerequisite kernel patch,
> it is already in linux-next:

> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=staging&id=01584c1e233740519d0e11aa20daa323d26bf598

Good to know, thank you (referring to kernel commits is always considered
useful).

No, I really searched if LTP patchset was also Cc'ed to linux-block.
But as the maintainers were Cc'ed I now understand why linux-block was omitted.
I'm sorry for the confusion.

Kind regards,
Petr

> So right now we are just waiting for the LTP patch series to get picked up.


> Kind regards,
> Niklas

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] Improve ioprio tests

[Petr Vorel]

Hi all,

patchset merged with added missing #include "config.h".

I'll do the additional cleanup work and Cc you.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] Improve ioprio tests

[Niklas Cassel via ltp]

On Tue, Jun 20, 2023 at 12:54:22PM +0200, Petr Vorel wrote:
> Hi all,
> 
> patchset merged with added missing #include "config.h".
> 
> I'll do the additional cleanup work and Cc you.

I was just about to send a v3 with #include "config.h",
now I don't need to :)

Thank you for your help Petr!


Kind regards,
Niklas

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp