On Fri, Nov 24, 2023 at 02:53:52PM +0900, Damien Le Moal wrote:
> On 11/24/23 12:05, Wei Gao wrote:
> > The current logic "if (level >= IOPRIO_NR_LEVELS)" can not be reached since
> > level value get from IOPRIO_PRIO_LEVEL ONLY extract lower 3-bits of ioprio.
> > (IOPRIO_NR_LEVELS=8)
> >
> > So this trigger LTP test case ioprio_set03 failed, the test case expect
> > error when set IOPRIO_CLASS_BE prio 8, in current implementation level
> > value will be 0 and obviously can not return error.
> >
> > Fixes: eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
>
> No. Please see below.
>
> > Signed-off-by: Wei Gao <wegao@xxxxxxxx>
> > ---
> > block/ioprio.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/block/ioprio.c b/block/ioprio.c
> > index b5a942519a79..f83029208f2a 100644
> > --- a/block/ioprio.c
> > +++ b/block/ioprio.c
> > @@ -33,7 +33,7 @@
> > int ioprio_check_cap(int ioprio)
> > {
> > int class = IOPRIO_PRIO_CLASS(ioprio);
> > - int level = IOPRIO_PRIO_LEVEL(ioprio);
> > + int data = IOPRIO_PRIO_DATA(ioprio);
> >
> > switch (class) {
> > case IOPRIO_CLASS_RT:
> > @@ -49,13 +49,13 @@ int ioprio_check_cap(int ioprio)
> > fallthrough;
> > /* rt has prio field too */
> > case IOPRIO_CLASS_BE:
> > - if (level >= IOPRIO_NR_LEVELS)
> > + if (data >= IOPRIO_NR_LEVELS || data < 0)
>
> This is incorrect: data is the combination of level AND hints, so that value can
> be larger than or equal to 8 with the level still being valid. Hard NACK on this.
>
> The issue with LTP test case has been fixed in LTP and by changing the ioprio.h
> header file. See commit 01584c1e2337 ("scsi: block: Improve ioprio value
> validity checks") which introduces IOPRIO_BAD_VALUE() macro for that.
>
> And for ltp, the commits are:
> 6b7f448fe392 ("ioprio: Use IOPRIO_PRIO_NUM to check prio range")
> 7c84fa710f75 ("ioprio: use ioprio.h kernel header if it exists")
>
> So please update your setup, including your install of kernel user API header files.
>
Thanks a lot for your quick feedback and detail explaination, if i am guess correctly,
my test kernel include eca2040972b4 ("scsi: block: ioprio: Clean up interface definition") but
not include 01584c1e2337 ("scsi: block: Improve ioprio value validity checks") by coincidence.
> > return -EINVAL;
> > break;
> > case IOPRIO_CLASS_IDLE:
> > break;
> > case IOPRIO_CLASS_NONE:
> > - if (level)
> > + if (data)
> > return -EINVAL;
> > break;
> > case IOPRIO_CLASS_INVALID:
>
> --
> Damien Le Moal
> Western Digital Research
>
