* [PATCH RESEND] block: improve ioprio value validity checks
@ 2023-06-08 9:55 Damien Le Moal
2023-06-08 10:54 ` Niklas Cassel
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Damien Le Moal @ 2023-06-08 9:55 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi, Jens Axboe, linux-block
Cc: Linus Walleij, Niklas Cassel
The introduction of the macro IOPRIO_PRIO_LEVEL() in commit
eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
results in an iopriority level to always be masked using the macro
IOPRIO_LEVEL_MASK, and thus to the kernel always seeing an acceptable
value for an I/O priority level when checked in ioprio_check_cap().
Before this patch, this function would return an error for some (but not
all) invalid values for a level valid range of [0..7].
Restore and improve the detection of invalid priority levels by
introducing the inline function ioprio_value() to check an ioprio class,
level and hint value before combining these fields into a single value
to be used with ioprio_set() or AIOs. If an invalid value for the class,
level or hint of an ioprio is detected, ioprio_value() returns an ioprio
using the class IOPRIO_CLASS_INVALID, indicating an invalid value and
causing ioprio_check_cap() to return -EINVAL.
Fixes: 6c913257226a ("scsi: block: Introduce ioprio hints")
Fixes: eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
block/ioprio.c | 1 +
include/uapi/linux/ioprio.h | 47 +++++++++++++++++++++++--------------
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/block/ioprio.c b/block/ioprio.c
index f0d9e818abc5..b5a942519a79 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -58,6 +58,7 @@ int ioprio_check_cap(int ioprio)
if (level)
return -EINVAL;
break;
+ case IOPRIO_CLASS_INVALID:
default:
return -EINVAL;
}
diff --git a/include/uapi/linux/ioprio.h b/include/uapi/linux/ioprio.h
index 607c7617b9d2..7310449c0178 100644
--- a/include/uapi/linux/ioprio.h
+++ b/include/uapi/linux/ioprio.h
@@ -6,15 +6,13 @@
* Gives us 8 prio classes with 13-bits of data for each class
*/
#define IOPRIO_CLASS_SHIFT 13
-#define IOPRIO_CLASS_MASK 0x07
+#define IOPRIO_NR_CLASSES 8
+#define IOPRIO_CLASS_MASK (IOPRIO_NR_CLASSES - 1)
#define IOPRIO_PRIO_MASK ((1UL << IOPRIO_CLASS_SHIFT) - 1)
#define IOPRIO_PRIO_CLASS(ioprio) \
(((ioprio) >> IOPRIO_CLASS_SHIFT) & IOPRIO_CLASS_MASK)
#define IOPRIO_PRIO_DATA(ioprio) ((ioprio) & IOPRIO_PRIO_MASK)
-#define IOPRIO_PRIO_VALUE(class, data) \
- ((((class) & IOPRIO_CLASS_MASK) << IOPRIO_CLASS_SHIFT) | \
- ((data) & IOPRIO_PRIO_MASK))
/*
* These are the io priority classes as implemented by the BFQ and mq-deadline
@@ -25,10 +23,13 @@
* served when no one else is using the disk.
*/
enum {
- IOPRIO_CLASS_NONE,
- IOPRIO_CLASS_RT,
- IOPRIO_CLASS_BE,
- IOPRIO_CLASS_IDLE,
+ IOPRIO_CLASS_NONE = 0,
+ IOPRIO_CLASS_RT = 1,
+ IOPRIO_CLASS_BE = 2,
+ IOPRIO_CLASS_IDLE = 3,
+
+ /* Special class to indicate an invalid ioprio value */
+ IOPRIO_CLASS_INVALID = 7,
};
/*
@@ -73,15 +74,6 @@ enum {
#define IOPRIO_PRIO_HINT(ioprio) \
(((ioprio) >> IOPRIO_HINT_SHIFT) & IOPRIO_HINT_MASK)
-/*
- * Alternate macro for IOPRIO_PRIO_VALUE() to define an IO priority with
- * a class, level and hint.
- */
-#define IOPRIO_PRIO_VALUE_HINT(class, level, hint) \
- ((((class) & IOPRIO_CLASS_MASK) << IOPRIO_CLASS_SHIFT) | \
- (((hint) & IOPRIO_HINT_MASK) << IOPRIO_HINT_SHIFT) | \
- ((level) & IOPRIO_LEVEL_MASK))
-
/*
* IO hints.
*/
@@ -107,4 +99,25 @@ enum {
IOPRIO_HINT_DEV_DURATION_LIMIT_7 = 7,
};
+#define IOPRIO_BAD_VALUE(val, max) ((val) < 0 || (val) >= (max))
+
+/*
+ * Return an I/O priority value based on a class, a level and a hint.
+ */
+static __always_inline __u16 ioprio_value(int class, int level, int hint)
+{
+ if (IOPRIO_BAD_VALUE(class, IOPRIO_NR_CLASSES) ||
+ IOPRIO_BAD_VALUE(level, IOPRIO_NR_LEVELS) ||
+ IOPRIO_BAD_VALUE(hint, IOPRIO_NR_HINTS))
+ return IOPRIO_CLASS_INVALID << IOPRIO_CLASS_SHIFT;
+
+ return (class << IOPRIO_CLASS_SHIFT) |
+ (hint << IOPRIO_HINT_SHIFT) | level;
+}
+
+#define IOPRIO_PRIO_VALUE(class, level) \
+ ioprio_value(class, level, IOPRIO_HINT_NONE)
+#define IOPRIO_PRIO_VALUE_HINT(class, level, hint) \
+ ioprio_value(class, level, hint)
+
#endif /* _UAPI_LINUX_IOPRIO_H */
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] block: improve ioprio value validity checks
2023-06-08 9:55 [PATCH RESEND] block: improve ioprio value validity checks Damien Le Moal
@ 2023-06-08 10:54 ` Niklas Cassel
2023-06-09 7:46 ` Linus Walleij
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2023-06-08 10:54 UTC (permalink / raw)
To: Damien Le Moal
Cc: Martin K . Petersen, linux-scsi, Jens Axboe, linux-block, Linus Walleij
On Thu, Jun 08, 2023 at 06:55:56PM +0900, Damien Le Moal wrote:
> The introduction of the macro IOPRIO_PRIO_LEVEL() in commit
> eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
> results in an iopriority level to always be masked using the macro
> IOPRIO_LEVEL_MASK, and thus to the kernel always seeing an acceptable
> value for an I/O priority level when checked in ioprio_check_cap().
> Before this patch, this function would return an error for some (but not
> all) invalid values for a level valid range of [0..7].
>
> Restore and improve the detection of invalid priority levels by
> introducing the inline function ioprio_value() to check an ioprio class,
> level and hint value before combining these fields into a single value
> to be used with ioprio_set() or AIOs. If an invalid value for the class,
> level or hint of an ioprio is detected, ioprio_value() returns an ioprio
> using the class IOPRIO_CLASS_INVALID, indicating an invalid value and
> causing ioprio_check_cap() to return -EINVAL.
>
> Fixes: 6c913257226a ("scsi: block: Introduce ioprio hints")
> Fixes: eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> block/ioprio.c | 1 +
> include/uapi/linux/ioprio.h | 47 +++++++++++++++++++++++--------------
> 2 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/block/ioprio.c b/block/ioprio.c
> index f0d9e818abc5..b5a942519a79 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -58,6 +58,7 @@ int ioprio_check_cap(int ioprio)
> if (level)
> return -EINVAL;
> break;
> + case IOPRIO_CLASS_INVALID:
> default:
> return -EINVAL;
> }
> diff --git a/include/uapi/linux/ioprio.h b/include/uapi/linux/ioprio.h
> index 607c7617b9d2..7310449c0178 100644
> --- a/include/uapi/linux/ioprio.h
> +++ b/include/uapi/linux/ioprio.h
> @@ -6,15 +6,13 @@
> * Gives us 8 prio classes with 13-bits of data for each class
> */
> #define IOPRIO_CLASS_SHIFT 13
> -#define IOPRIO_CLASS_MASK 0x07
> +#define IOPRIO_NR_CLASSES 8
> +#define IOPRIO_CLASS_MASK (IOPRIO_NR_CLASSES - 1)
> #define IOPRIO_PRIO_MASK ((1UL << IOPRIO_CLASS_SHIFT) - 1)
>
> #define IOPRIO_PRIO_CLASS(ioprio) \
> (((ioprio) >> IOPRIO_CLASS_SHIFT) & IOPRIO_CLASS_MASK)
> #define IOPRIO_PRIO_DATA(ioprio) ((ioprio) & IOPRIO_PRIO_MASK)
> -#define IOPRIO_PRIO_VALUE(class, data) \
> - ((((class) & IOPRIO_CLASS_MASK) << IOPRIO_CLASS_SHIFT) | \
> - ((data) & IOPRIO_PRIO_MASK))
>
> /*
> * These are the io priority classes as implemented by the BFQ and mq-deadline
> @@ -25,10 +23,13 @@
> * served when no one else is using the disk.
> */
> enum {
> - IOPRIO_CLASS_NONE,
> - IOPRIO_CLASS_RT,
> - IOPRIO_CLASS_BE,
> - IOPRIO_CLASS_IDLE,
> + IOPRIO_CLASS_NONE = 0,
> + IOPRIO_CLASS_RT = 1,
> + IOPRIO_CLASS_BE = 2,
> + IOPRIO_CLASS_IDLE = 3,
> +
> + /* Special class to indicate an invalid ioprio value */
> + IOPRIO_CLASS_INVALID = 7,
> };
>
> /*
> @@ -73,15 +74,6 @@ enum {
> #define IOPRIO_PRIO_HINT(ioprio) \
> (((ioprio) >> IOPRIO_HINT_SHIFT) & IOPRIO_HINT_MASK)
>
> -/*
> - * Alternate macro for IOPRIO_PRIO_VALUE() to define an IO priority with
> - * a class, level and hint.
> - */
> -#define IOPRIO_PRIO_VALUE_HINT(class, level, hint) \
> - ((((class) & IOPRIO_CLASS_MASK) << IOPRIO_CLASS_SHIFT) | \
> - (((hint) & IOPRIO_HINT_MASK) << IOPRIO_HINT_SHIFT) | \
> - ((level) & IOPRIO_LEVEL_MASK))
> -
> /*
> * IO hints.
> */
> @@ -107,4 +99,25 @@ enum {
> IOPRIO_HINT_DEV_DURATION_LIMIT_7 = 7,
> };
>
> +#define IOPRIO_BAD_VALUE(val, max) ((val) < 0 || (val) >= (max))
> +
> +/*
> + * Return an I/O priority value based on a class, a level and a hint.
> + */
> +static __always_inline __u16 ioprio_value(int class, int level, int hint)
> +{
> + if (IOPRIO_BAD_VALUE(class, IOPRIO_NR_CLASSES) ||
> + IOPRIO_BAD_VALUE(level, IOPRIO_NR_LEVELS) ||
> + IOPRIO_BAD_VALUE(hint, IOPRIO_NR_HINTS))
> + return IOPRIO_CLASS_INVALID << IOPRIO_CLASS_SHIFT;
> +
> + return (class << IOPRIO_CLASS_SHIFT) |
> + (hint << IOPRIO_HINT_SHIFT) | level;
> +}
> +
> +#define IOPRIO_PRIO_VALUE(class, level) \
> + ioprio_value(class, level, IOPRIO_HINT_NONE)
> +#define IOPRIO_PRIO_VALUE_HINT(class, level, hint) \
> + ioprio_value(class, level, hint)
> +
> #endif /* _UAPI_LINUX_IOPRIO_H */
> --
> 2.40.1
>
I already gave my R-b tag on the original message,
but since my tag is not here, I'm giving it again:
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] block: improve ioprio value validity checks
2023-06-08 9:55 [PATCH RESEND] block: improve ioprio value validity checks Damien Le Moal
2023-06-08 10:54 ` Niklas Cassel
@ 2023-06-09 7:46 ` Linus Walleij
2023-06-12 2:20 ` Damien Le Moal
2023-06-15 1:27 ` Martin K. Petersen
2023-06-22 1:26 ` Martin K. Petersen
3 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2023-06-09 7:46 UTC (permalink / raw)
To: Damien Le Moal
Cc: Martin K . Petersen, linux-scsi, Jens Axboe, linux-block, Niklas Cassel
On Thu, Jun 8, 2023 at 11:55 AM Damien Le Moal <dlemoal@kernel.org> wrote:
> The introduction of the macro IOPRIO_PRIO_LEVEL() in commit
> eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
> results in an iopriority level to always be masked using the macro
> IOPRIO_LEVEL_MASK, and thus to the kernel always seeing an acceptable
> value for an I/O priority level when checked in ioprio_check_cap().
> Before this patch, this function would return an error for some (but not
> all) invalid values for a level valid range of [0..7].
>
> Restore and improve the detection of invalid priority levels by
> introducing the inline function ioprio_value() to check an ioprio class,
> level and hint value before combining these fields into a single value
> to be used with ioprio_set() or AIOs. If an invalid value for the class,
> level or hint of an ioprio is detected, ioprio_value() returns an ioprio
> using the class IOPRIO_CLASS_INVALID, indicating an invalid value and
> causing ioprio_check_cap() to return -EINVAL.
>
> Fixes: 6c913257226a ("scsi: block: Introduce ioprio hints")
> Fixes: eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
This makes it easier to get things right.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] block: improve ioprio value validity checks
2023-06-09 7:46 ` Linus Walleij
@ 2023-06-12 2:20 ` Damien Le Moal
0 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2023-06-12 2:20 UTC (permalink / raw)
To: Linus Walleij
Cc: Martin K . Petersen, linux-scsi, Jens Axboe, linux-block, Niklas Cassel
On 6/9/23 16:46, Linus Walleij wrote:
> On Thu, Jun 8, 2023 at 11:55 AM Damien Le Moal <dlemoal@kernel.org> wrote:
>
>> The introduction of the macro IOPRIO_PRIO_LEVEL() in commit
>> eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
>> results in an iopriority level to always be masked using the macro
>> IOPRIO_LEVEL_MASK, and thus to the kernel always seeing an acceptable
>> value for an I/O priority level when checked in ioprio_check_cap().
>> Before this patch, this function would return an error for some (but not
>> all) invalid values for a level valid range of [0..7].
>>
>> Restore and improve the detection of invalid priority levels by
>> introducing the inline function ioprio_value() to check an ioprio class,
>> level and hint value before combining these fields into a single value
>> to be used with ioprio_set() or AIOs. If an invalid value for the class,
>> level or hint of an ioprio is detected, ioprio_value() returns an ioprio
>> using the class IOPRIO_CLASS_INVALID, indicating an invalid value and
>> causing ioprio_check_cap() to return -EINVAL.
>>
>> Fixes: 6c913257226a ("scsi: block: Introduce ioprio hints")
>> Fixes: eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>
> This makes it easier to get things right.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks Linus !
Martin,
can you queue this please ?
>
> Yours,
> Linus Walleij
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] block: improve ioprio value validity checks
2023-06-08 9:55 [PATCH RESEND] block: improve ioprio value validity checks Damien Le Moal
2023-06-08 10:54 ` Niklas Cassel
2023-06-09 7:46 ` Linus Walleij
@ 2023-06-15 1:27 ` Martin K. Petersen
2023-06-22 1:26 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2023-06-15 1:27 UTC (permalink / raw)
To: Damien Le Moal
Cc: Martin K . Petersen, linux-scsi, Jens Axboe, linux-block,
Linus Walleij, Niklas Cassel
Damien,
> The introduction of the macro IOPRIO_PRIO_LEVEL() in commit
> eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
> results in an iopriority level to always be masked using the macro
> IOPRIO_LEVEL_MASK, and thus to the kernel always seeing an acceptable
> value for an I/O priority level when checked in ioprio_check_cap().
> Before this patch, this function would return an error for some (but
> not all) invalid values for a level valid range of [0..7].
Applied to 6.5/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] block: improve ioprio value validity checks
2023-06-08 9:55 [PATCH RESEND] block: improve ioprio value validity checks Damien Le Moal
` (2 preceding siblings ...)
2023-06-15 1:27 ` Martin K. Petersen
@ 2023-06-22 1:26 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2023-06-22 1:26 UTC (permalink / raw)
To: linux-scsi, Jens Axboe, linux-block, Damien Le Moal
Cc: Martin K . Petersen, Linus Walleij, Niklas Cassel
On Thu, 08 Jun 2023 18:55:56 +0900, Damien Le Moal wrote:
> The introduction of the macro IOPRIO_PRIO_LEVEL() in commit
> eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
> results in an iopriority level to always be masked using the macro
> IOPRIO_LEVEL_MASK, and thus to the kernel always seeing an acceptable
> value for an I/O priority level when checked in ioprio_check_cap().
> Before this patch, this function would return an error for some (but not
> all) invalid values for a level valid range of [0..7].
>
> [...]
Applied to 6.5/scsi-queue, thanks!
[1/1] block: improve ioprio value validity checks
https://git.kernel.org/mkp/scsi/c/01584c1e2337
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-22 1:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08 9:55 [PATCH RESEND] block: improve ioprio value validity checks Damien Le Moal
2023-06-08 10:54 ` Niklas Cassel
2023-06-09 7:46 ` Linus Walleij
2023-06-12 2:20 ` Damien Le Moal
2023-06-15 1:27 ` Martin K. Petersen
2023-06-22 1:26 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.