* [PATCH 0/9 v5] block: Fix IO priority mess
@ 2022-06-23 7:48 Jan Kara
2022-06-23 7:48 ` [PATCH 1/9] block: fix default IO priority handling again Jan Kara
` (8 more replies)
0 siblings, 9 replies; 27+ messages in thread
From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel,
Jan Kara
Hello,
This is the fifth revision of my patches fixing IO priority handling in the
block layer. Damien has reviewed all the patches and tested them as well so
I think patches are ready to be merged.
Changes since v4:
* Added Reviewed-by and Tested-by tags
* Fixed prototype of stub function for !CONFIG_BLOCK
Changes since v3:
* Added Reviewed-by tags from Damien
* Fixed build failure without CONFIG_BLOCK
* Separated refactoring of get_current_ioprio() into a separate patch
Changes since v2:
* added some comments to better explain things
* changed handling of ioprio_get(2)
* a few small tweaks based on Damien's feedback
Original cover letter:
Recently, I've been looking into 10% regression reported by our performance
measurement infrastructure in reaim benchmark that was bisected down to
5a9d041ba2f6 ("block: move io_context creation into where it's needed"). This
didn't really make much sense and it took me a while to understand this but the
culprit is actually in even older commit e70344c05995 ("block: fix default IO
priority handling") and 5a9d041ba2f6 just made the breakage visible.
Essentially the problem was that after these commits some IO was queued with IO
priority class IOPRIO_CLASS_BE while other IO was queued with IOPRIO_CLASS_NONE
and as a result they could not be merged together resulting in performance
regression. I think what commit e70344c05995 ("block: fix default IO priority
handling") did is actually broken not only because of this performance
regression but because of other reasons as well (see changelog of patch 3/8 for
details). Besides this problem, there are multiple other inconsistencies in the
IO priority handling throughout the block stack we have identified when
discussing this with Damien Le Moal. So this patch set aims at fixing all these
various issues.
Note that there are a few choices I've made I'm not 100% sold on. In particular
the conversion of blk-ioprio from rqos is somewhat disputable since it now
incurs a cost similar to blk-throttle in the bio submission fast path (need to
load bio->bi_blkg->pd[ioprio_policy.plid]). If people think the removed
boilerplate code is not worth the cost, I can certainly go via the "additional
rqos hook" path.
Honza
Previous versions:
Link: http://lore.kernel.org/r/20220601132347.13543-1-jack@suse.cz # v1
Link: http://lore.kernel.org/r/20220615160437.5478-1-jack@suse.cz # v2
Link: http://lore.kernel.org/r/20220620160726.19798-1-jack@suse.cz # v3
Link: http://lore.kernel.org/r/20220621102201.26337-1-jack@suse.cz # v4
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 1/9] block: fix default IO priority handling again 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 13:57 ` Christoph Hellwig 2022-06-23 14:23 ` Jens Axboe 2022-06-23 7:48 ` [PATCH 2/9] block: Return effective IO priority from get_current_ioprio() Jan Kara ` (7 subsequent siblings) 8 siblings, 2 replies; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara, stable Commit e70344c05995 ("block: fix default IO priority handling") introduced an inconsistency in get_current_ioprio() that tasks without IO context return IOPRIO_DEFAULT priority while tasks with freshly allocated IO context will return 0 (IOPRIO_CLASS_NONE/0) IO priority. Tasks without IO context used to be rare before 5a9d041ba2f6 ("block: move io_context creation into where it's needed") but after this commit they became common because now only BFQ IO scheduler setups task's IO context. Similar inconsistency is there for get_task_ioprio() so this inconsistency is now exposed to userspace and userspace will see different IO priority for tasks operating on devices with BFQ compared to devices without BFQ. Furthemore the changes done by commit e70344c05995 change the behavior when no IO priority is set for BFQ IO scheduler which is also documented in ioprio_set(2) manpage: "If no I/O scheduler has been set for a thread, then by default the I/O priority will follow the CPU nice value (setpriority(2)). In Linux kernels before version 2.6.24, once an I/O priority had been set using ioprio_set(), there was no way to reset the I/O scheduling behavior to the default. Since Linux 2.6.24, specifying ioprio as 0 can be used to reset to the default I/O scheduling behavior." So make sure we default to IOPRIO_CLASS_NONE as used to be the case before commit e70344c05995. Also cleanup alloc_io_context() to explicitely set this IO priority for the allocated IO context to avoid future surprises. Note that we tweak ioprio_best() to maintain ioprio_get(2) behavior and make this commit easily backportable. CC: stable@vger.kernel.org Fixes: e70344c05995 ("block: fix default IO priority handling") Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/blk-ioc.c | 2 ++ block/ioprio.c | 4 ++-- include/linux/ioprio.h | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/block/blk-ioc.c b/block/blk-ioc.c index df9cfe4ca532..63fc02042408 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -247,6 +247,8 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node) INIT_HLIST_HEAD(&ioc->icq_list); INIT_WORK(&ioc->release_work, ioc_release_fn); #endif + ioc->ioprio = IOPRIO_DEFAULT; + return ioc; } diff --git a/block/ioprio.c b/block/ioprio.c index 2fe068fcaad5..2a34cbca18ae 100644 --- a/block/ioprio.c +++ b/block/ioprio.c @@ -157,9 +157,9 @@ static int get_task_ioprio(struct task_struct *p) int ioprio_best(unsigned short aprio, unsigned short bprio) { if (!ioprio_valid(aprio)) - aprio = IOPRIO_DEFAULT; + aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); if (!ioprio_valid(bprio)) - bprio = IOPRIO_DEFAULT; + bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); return min(aprio, bprio); } diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h index 3f53bc27a19b..3d088a88f832 100644 --- a/include/linux/ioprio.h +++ b/include/linux/ioprio.h @@ -11,7 +11,7 @@ /* * Default IO priority. */ -#define IOPRIO_DEFAULT IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM) +#define IOPRIO_DEFAULT IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0) /* * Check that a priority value has a valid class. -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] block: fix default IO priority handling again 2022-06-23 7:48 ` [PATCH 1/9] block: fix default IO priority handling again Jan Kara @ 2022-06-23 13:57 ` Christoph Hellwig 2022-06-23 14:23 ` Jens Axboe 1 sibling, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 13:57 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, stable Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] block: fix default IO priority handling again 2022-06-23 7:48 ` [PATCH 1/9] block: fix default IO priority handling again Jan Kara 2022-06-23 13:57 ` Christoph Hellwig @ 2022-06-23 14:23 ` Jens Axboe 1 sibling, 0 replies; 27+ messages in thread From: Jens Axboe @ 2022-06-23 14:23 UTC (permalink / raw) To: jack; +Cc: linux-block, damien.lemoal, stable, Niklas.Cassel, bvanassche On Thu, 23 Jun 2022 09:48:26 +0200, Jan Kara wrote: > Commit e70344c05995 ("block: fix default IO priority handling") > introduced an inconsistency in get_current_ioprio() that tasks without > IO context return IOPRIO_DEFAULT priority while tasks with freshly > allocated IO context will return 0 (IOPRIO_CLASS_NONE/0) IO priority. > Tasks without IO context used to be rare before 5a9d041ba2f6 ("block: > move io_context creation into where it's needed") but after this commit > they became common because now only BFQ IO scheduler setups task's IO > context. Similar inconsistency is there for get_task_ioprio() so this > inconsistency is now exposed to userspace and userspace will see > different IO priority for tasks operating on devices with BFQ compared > to devices without BFQ. Furthemore the changes done by commit > e70344c05995 change the behavior when no IO priority is set for BFQ IO > scheduler which is also documented in ioprio_set(2) manpage: > > [...] Applied, thanks! [1/9] block: fix default IO priority handling again commit: f0f5a5e24fa5412f187f429232334ad6832d1a66 [2/9] block: Return effective IO priority from get_current_ioprio() commit: 93fd10125cd702d86f1c4005349b54eeb3c02af3 [3/9] block: Generalize get_current_ioprio() for any task commit: 86f80bd5f639921c59afb113fa3ebb3ccb46be84 [4/9] block: Make ioprio_best() static commit: c85fb98c51a66ff7346f2e12f6c20fb4f60de812 [5/9] block: Fix handling of tasks without ioprio in ioprio_get(2) commit: caf2c269be20c536009ccd815a4e493d0c6c6634 [6/9] blk-ioprio: Remove unneeded field commit: d2adb01a5bcbe36bc05fbb383028da755f7a919b [7/9] blk-ioprio: Convert from rqos policy to direct call commit: 8f3d8d7f56aba4c6171e48b107b9167255044653 [8/9] block: Initialize bio priority earlier commit: 92c3dfe1cfce7dd7cf6cd32b78b05885d824656e [9/9] block: Always initialize bio IO priority on submit commit: 71ad7aabb8968164b1963fff7216b225fdd80f84 Best regards, -- Jens Axboe ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/9] block: Return effective IO priority from get_current_ioprio() 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara 2022-06-23 7:48 ` [PATCH 1/9] block: fix default IO priority handling again Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 13:57 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara ` (6 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara get_current_ioprio() is used to initialize IO priority of various requests. As such it should be returning the effective IO priority of the task (i.e., reflecting the fact that unset IO priority should get set based on task's CPU priority) so that the conversion is concentrated in one place. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- include/linux/ioprio.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h index 3d088a88f832..61ed6bb4998e 100644 --- a/include/linux/ioprio.h +++ b/include/linux/ioprio.h @@ -53,10 +53,17 @@ static inline int task_nice_ioclass(struct task_struct *task) static inline int get_current_ioprio(void) { struct io_context *ioc = current->io_context; + int prio; if (ioc) - return ioc->ioprio; - return IOPRIO_DEFAULT; + prio = ioc->ioprio; + else + prio = IOPRIO_DEFAULT; + + if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) + prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current), + task_nice_ioprio(current)); + return prio; } /* -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 2/9] block: Return effective IO priority from get_current_ioprio() 2022-06-23 7:48 ` [PATCH 2/9] block: Return effective IO priority from get_current_ioprio() Jan Kara @ 2022-06-23 13:57 ` Christoph Hellwig 0 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 13:57 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel On Thu, Jun 23, 2022 at 09:48:27AM +0200, Jan Kara wrote: > get_current_ioprio() is used to initialize IO priority of various > requests. As such it should be returning the effective IO priority of > the task (i.e., reflecting the fact that unset IO priority should get > set based on task's CPU priority) so that the conversion is concentrated > in one place. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara 2022-06-23 7:48 ` [PATCH 1/9] block: fix default IO priority handling again Jan Kara 2022-06-23 7:48 ` [PATCH 2/9] block: Return effective IO priority from get_current_ioprio() Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 13:59 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 4/9] block: Make ioprio_best() static Jan Kara ` (5 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara get_current_ioprio() operates only on current task. We will need the same functionality for other tasks as well. Generalize get_current_ioprio() for that and also move the bulk out of the header file because it is large enough. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/ioprio.c | 26 ++++++++++++++++++++++++++ include/linux/ioprio.h | 26 ++++++++++---------------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/block/ioprio.c b/block/ioprio.c index 2a34cbca18ae..c4e3476155a1 100644 --- a/block/ioprio.c +++ b/block/ioprio.c @@ -138,6 +138,32 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio) return ret; } +/* + * If the task has set an I/O priority, use that. Otherwise, return + * the default I/O priority. + * + * Expected to be called for current task or with task_lock() held to keep + * io_context stable. + */ +int __get_task_ioprio(struct task_struct *p) +{ + struct io_context *ioc = p->io_context; + int prio; + + if (p != current) + lockdep_assert_held(&p->alloc_lock); + if (ioc) + prio = ioc->ioprio; + else + prio = IOPRIO_DEFAULT; + + if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) + prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p), + task_nice_ioprio(p)); + return prio; +} +EXPORT_SYMBOL_GPL(__get_task_ioprio); + static int get_task_ioprio(struct task_struct *p) { int ret; diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h index 61ed6bb4998e..9752cf4a9c7c 100644 --- a/include/linux/ioprio.h +++ b/include/linux/ioprio.h @@ -46,24 +46,18 @@ static inline int task_nice_ioclass(struct task_struct *task) return IOPRIO_CLASS_BE; } -/* - * If the calling process has set an I/O priority, use that. Otherwise, return - * the default I/O priority. - */ -static inline int get_current_ioprio(void) +#ifdef CONFIG_BLOCK +int __get_task_ioprio(struct task_struct *p); +#else +static inline int __get_task_ioprio(struct task_struct *p) { - struct io_context *ioc = current->io_context; - int prio; - - if (ioc) - prio = ioc->ioprio; - else - prio = IOPRIO_DEFAULT; + return IOPRIO_DEFAULT; +} +#endif /* CONFIG_BLOCK */ - if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) - prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current), - task_nice_ioprio(current)); - return prio; +static inline int get_current_ioprio(void) +{ + return __get_task_ioprio(current); } /* -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-23 7:48 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara @ 2022-06-23 13:59 ` Christoph Hellwig 0 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 13:59 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel On Thu, Jun 23, 2022 at 09:48:28AM +0200, Jan Kara wrote: > get_current_ioprio() operates only on current task. We will need the > same functionality for other tasks as well. Generalize > get_current_ioprio() for that and also move the bulk out of the header > file because it is large enough. We don't really need the general version exported. But if you prefer an inline wrapper over a tail call I guess this is fine, too: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/9] block: Make ioprio_best() static 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara ` (2 preceding siblings ...) 2022-06-23 7:48 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 13:58 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 5/9] block: Fix handling of tasks without ioprio in ioprio_get(2) Jan Kara ` (4 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Nobody outside of block/ioprio.c uses it. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/ioprio.c | 2 +- include/linux/ioprio.h | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/block/ioprio.c b/block/ioprio.c index c4e3476155a1..8c46f672a0ba 100644 --- a/block/ioprio.c +++ b/block/ioprio.c @@ -180,7 +180,7 @@ static int get_task_ioprio(struct task_struct *p) return ret; } -int ioprio_best(unsigned short aprio, unsigned short bprio) +static int ioprio_best(unsigned short aprio, unsigned short bprio) { if (!ioprio_valid(aprio)) aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h index 9752cf4a9c7c..7578d4f6a969 100644 --- a/include/linux/ioprio.h +++ b/include/linux/ioprio.h @@ -60,11 +60,6 @@ static inline int get_current_ioprio(void) return __get_task_ioprio(current); } -/* - * For inheritance, return the highest of the two given priorities - */ -extern int ioprio_best(unsigned short aprio, unsigned short bprio); - extern int set_task_ioprio(struct task_struct *task, int ioprio); #ifdef CONFIG_BLOCK -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] block: Make ioprio_best() static 2022-06-23 7:48 ` [PATCH 4/9] block: Make ioprio_best() static Jan Kara @ 2022-06-23 13:58 ` Christoph Hellwig 0 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 13:58 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/9] block: Fix handling of tasks without ioprio in ioprio_get(2) 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara ` (3 preceding siblings ...) 2022-06-23 7:48 ` [PATCH 4/9] block: Make ioprio_best() static Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 7:48 ` [PATCH 6/9] blk-ioprio: Remove unneeded field Jan Kara ` (3 subsequent siblings) 8 siblings, 0 replies; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara ioprio_get(2) can be asked to return the best IO priority from several tasks (IOPRIO_WHO_PGRP, IOPRIO_WHO_USER). Currently the call treats tasks without set IO priority as having priority IOPRIO_CLASS_BE/IOPRIO_BE_NORM however this does not really reflect the IO priority the task will get (which depends on task's nice value). Fix the code to use the real IO priority task's IO will use. We have to modify code for ioprio_get(IOPRIO_WHO_PROCESS) to keep returning IOPRIO_CLASS_NONE priority for tasks without set IO priority as a special case to maintain userspace visible API. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/ioprio.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/block/ioprio.c b/block/ioprio.c index 8c46f672a0ba..32a456b45804 100644 --- a/block/ioprio.c +++ b/block/ioprio.c @@ -171,10 +171,31 @@ static int get_task_ioprio(struct task_struct *p) ret = security_task_getioprio(p); if (ret) goto out; - ret = IOPRIO_DEFAULT; + task_lock(p); + ret = __get_task_ioprio(p); + task_unlock(p); +out: + return ret; +} + +/* + * Return raw IO priority value as set by userspace. We use this for + * ioprio_get(pid, IOPRIO_WHO_PROCESS) so that we keep historical behavior and + * also so that userspace can distinguish unset IO priority (which just gets + * overriden based on task's nice value) from IO priority set to some value. + */ +static int get_task_raw_ioprio(struct task_struct *p) +{ + int ret; + + ret = security_task_getioprio(p); + if (ret) + goto out; task_lock(p); if (p->io_context) ret = p->io_context->ioprio; + else + ret = IOPRIO_DEFAULT; task_unlock(p); out: return ret; @@ -182,11 +203,6 @@ static int get_task_ioprio(struct task_struct *p) static int ioprio_best(unsigned short aprio, unsigned short bprio) { - if (!ioprio_valid(aprio)) - aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); - if (!ioprio_valid(bprio)) - bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); - return min(aprio, bprio); } @@ -207,7 +223,7 @@ SYSCALL_DEFINE2(ioprio_get, int, which, int, who) else p = find_task_by_vpid(who); if (p) - ret = get_task_ioprio(p); + ret = get_task_raw_ioprio(p); break; case IOPRIO_WHO_PGRP: if (!who) -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 6/9] blk-ioprio: Remove unneeded field 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara ` (4 preceding siblings ...) 2022-06-23 7:48 ` [PATCH 5/9] block: Fix handling of tasks without ioprio in ioprio_get(2) Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 14:00 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 7/9] blk-ioprio: Convert from rqos policy to direct call Jan Kara ` (2 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara blkcg->ioprio_set field is not really useful except for avoiding possibly more expensive checks inside blkcg_ioprio_track(). The check for blkcg->prio_policy being equal to POLICY_NO_CHANGE does the same service so just remove the ioprio_set field and replace the check. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/blk-ioprio.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c index 79e797f5d194..3f605583598b 100644 --- a/block/blk-ioprio.c +++ b/block/blk-ioprio.c @@ -62,7 +62,6 @@ struct ioprio_blkg { struct ioprio_blkcg { struct blkcg_policy_data cpd; enum prio_policy prio_policy; - bool prio_set; }; static inline struct ioprio_blkg *pd_to_ioprio(struct blkg_policy_data *pd) @@ -113,7 +112,6 @@ static ssize_t ioprio_set_prio_policy(struct kernfs_open_file *of, char *buf, if (ret < 0) return ret; blkcg->prio_policy = ret; - blkcg->prio_set = true; return nbytes; } @@ -193,16 +191,15 @@ static void blkcg_ioprio_track(struct rq_qos *rqos, struct request *rq, struct ioprio_blkcg *blkcg = ioprio_blkcg_from_bio(bio); u16 prio; - if (!blkcg->prio_set) + if (blkcg->prio_policy == POLICY_NO_CHANGE) return; /* * Except for IOPRIO_CLASS_NONE, higher I/O priority numbers * correspond to a lower priority. Hence, the max_t() below selects * the lower priority of bi_ioprio and the cgroup I/O priority class. - * If the cgroup policy has been set to POLICY_NO_CHANGE == 0, the - * bio I/O priority is not modified. If the bio I/O priority equals - * IOPRIO_CLASS_NONE, the cgroup I/O priority is assigned to the bio. + * If the bio I/O priority equals IOPRIO_CLASS_NONE, the cgroup I/O + * priority is assigned to the bio. */ prio = max_t(u16, bio->bi_ioprio, IOPRIO_PRIO_VALUE(blkcg->prio_policy, 0)); -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] blk-ioprio: Remove unneeded field 2022-06-23 7:48 ` [PATCH 6/9] blk-ioprio: Remove unneeded field Jan Kara @ 2022-06-23 14:00 ` Christoph Hellwig 0 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 14:00 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel Looks god: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 7/9] blk-ioprio: Convert from rqos policy to direct call 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara ` (5 preceding siblings ...) 2022-06-23 7:48 ` [PATCH 6/9] blk-ioprio: Remove unneeded field Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 14:01 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 8/9] block: Initialize bio priority earlier Jan Kara 2022-06-23 7:48 ` [PATCH 9/9] block: Always initialize bio IO priority on submit Jan Kara 8 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Convert blk-ioprio handling from a rqos policy to a direct call from blk_mq_submit_bio(). Firstly, blk-ioprio is not much of a rqos policy anyway, it just needs a hook in bio submission path to set the bio's IO priority. Secondly, the rqos .track hook gets actually called too late for blk-ioprio purposes and introducing a special rqos hook just for blk-ioprio looks even weirder. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/blk-cgroup.c | 1 + block/blk-ioprio.c | 50 +++++----------------------------------------- block/blk-ioprio.h | 9 +++++++++ block/blk-mq.c | 8 ++++++++ 4 files changed, 23 insertions(+), 45 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 764e740b0c0f..6906981563f8 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1299,6 +1299,7 @@ int blkcg_init_queue(struct request_queue *q) ret = blk_iolatency_init(q); if (ret) { blk_throtl_exit(q); + blk_ioprio_exit(q); goto err_destroy_all; } diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c index 3f605583598b..c00060a02c6e 100644 --- a/block/blk-ioprio.c +++ b/block/blk-ioprio.c @@ -181,17 +181,12 @@ static struct blkcg_policy ioprio_policy = { .pd_free_fn = ioprio_free_pd, }; -struct blk_ioprio { - struct rq_qos rqos; -}; - -static void blkcg_ioprio_track(struct rq_qos *rqos, struct request *rq, - struct bio *bio) +void blkcg_set_ioprio(struct bio *bio) { struct ioprio_blkcg *blkcg = ioprio_blkcg_from_bio(bio); u16 prio; - if (blkcg->prio_policy == POLICY_NO_CHANGE) + if (!blkcg || blkcg->prio_policy == POLICY_NO_CHANGE) return; /* @@ -207,49 +202,14 @@ static void blkcg_ioprio_track(struct rq_qos *rqos, struct request *rq, bio->bi_ioprio = prio; } -static void blkcg_ioprio_exit(struct rq_qos *rqos) +void blk_ioprio_exit(struct request_queue *q) { - struct blk_ioprio *blkioprio_blkg = - container_of(rqos, typeof(*blkioprio_blkg), rqos); - - blkcg_deactivate_policy(rqos->q, &ioprio_policy); - kfree(blkioprio_blkg); + blkcg_deactivate_policy(q, &ioprio_policy); } -static struct rq_qos_ops blkcg_ioprio_ops = { - .track = blkcg_ioprio_track, - .exit = blkcg_ioprio_exit, -}; - int blk_ioprio_init(struct request_queue *q) { - struct blk_ioprio *blkioprio_blkg; - struct rq_qos *rqos; - int ret; - - blkioprio_blkg = kzalloc(sizeof(*blkioprio_blkg), GFP_KERNEL); - if (!blkioprio_blkg) - return -ENOMEM; - - ret = blkcg_activate_policy(q, &ioprio_policy); - if (ret) { - kfree(blkioprio_blkg); - return ret; - } - - rqos = &blkioprio_blkg->rqos; - rqos->id = RQ_QOS_IOPRIO; - rqos->ops = &blkcg_ioprio_ops; - rqos->q = q; - - /* - * Registering the rq-qos policy after activating the blk-cgroup - * policy guarantees that ioprio_blkcg_from_bio(bio) != NULL in the - * rq-qos callbacks. - */ - rq_qos_add(q, rqos); - - return 0; + return blkcg_activate_policy(q, &ioprio_policy); } static int __init ioprio_init(void) diff --git a/block/blk-ioprio.h b/block/blk-ioprio.h index a7785c2f1aea..5a1eb550e178 100644 --- a/block/blk-ioprio.h +++ b/block/blk-ioprio.h @@ -6,14 +6,23 @@ #include <linux/kconfig.h> struct request_queue; +struct bio; #ifdef CONFIG_BLK_CGROUP_IOPRIO int blk_ioprio_init(struct request_queue *q); +void blk_ioprio_exit(struct request_queue *q); +void blkcg_set_ioprio(struct bio *bio); #else static inline int blk_ioprio_init(struct request_queue *q) { return 0; } +static inline void blk_ioprio_exit(struct request_queue *q) +{ +} +static inline void blkcg_set_ioprio(struct bio *bio) +{ +} #endif #endif /* _BLK_IOPRIO_H_ */ diff --git a/block/blk-mq.c b/block/blk-mq.c index e9bf950983c7..67a7bfa58b7c 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -42,6 +42,7 @@ #include "blk-stat.h" #include "blk-mq-sched.h" #include "blk-rq-qos.h" +#include "blk-ioprio.h" static DEFINE_PER_CPU(struct llist_head, blk_cpu_done); @@ -2790,6 +2791,11 @@ static inline struct request *blk_mq_get_cached_request(struct request_queue *q, return rq; } +static void bio_set_ioprio(struct bio *bio) +{ + blkcg_set_ioprio(bio); +} + /** * blk_mq_submit_bio - Create and send a request to block device. * @bio: Bio pointer. @@ -2830,6 +2836,8 @@ void blk_mq_submit_bio(struct bio *bio) trace_block_getrq(bio); + bio_set_ioprio(bio); + rq_qos_track(q, rq, bio); blk_mq_bio_to_request(rq, bio, nr_segs); -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 7/9] blk-ioprio: Convert from rqos policy to direct call 2022-06-23 7:48 ` [PATCH 7/9] blk-ioprio: Convert from rqos policy to direct call Jan Kara @ 2022-06-23 14:01 ` Christoph Hellwig 0 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 14:01 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel Nice! The rqos calling conventions always seem like a bit of obsfucation anyway.. Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 8/9] block: Initialize bio priority earlier 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara ` (6 preceding siblings ...) 2022-06-23 7:48 ` [PATCH 7/9] blk-ioprio: Convert from rqos policy to direct call Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 14:01 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 9/9] block: Always initialize bio IO priority on submit Jan Kara 8 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Bio's IO priority needs to be initialized before we try to merge the bio with other bios. Otherwise we could merge bios which would otherwise receive different IO priorities leading to possible QoS issues. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/blk-mq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 67a7bfa58b7c..e17d822e6051 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2825,6 +2825,8 @@ void blk_mq_submit_bio(struct bio *bio) if (!bio_integrity_prep(bio)) return; + bio_set_ioprio(bio); + rq = blk_mq_get_cached_request(q, plug, &bio, nr_segs); if (!rq) { if (!bio) @@ -2836,8 +2838,6 @@ void blk_mq_submit_bio(struct bio *bio) trace_block_getrq(bio); - bio_set_ioprio(bio); - rq_qos_track(q, rq, bio); blk_mq_bio_to_request(rq, bio, nr_segs); -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 8/9] block: Initialize bio priority earlier 2022-06-23 7:48 ` [PATCH 8/9] block: Initialize bio priority earlier Jan Kara @ 2022-06-23 14:01 ` Christoph Hellwig 0 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 14:01 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel On Thu, Jun 23, 2022 at 09:48:33AM +0200, Jan Kara wrote: > Bio's IO priority needs to be initialized before we try to merge the bio > with other bios. Otherwise we could merge bios which would otherwise > receive different IO priorities leading to possible QoS issues. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 9/9] block: Always initialize bio IO priority on submit 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara ` (7 preceding siblings ...) 2022-06-23 7:48 ` [PATCH 8/9] block: Initialize bio priority earlier Jan Kara @ 2022-06-23 7:48 ` Jan Kara 2022-06-23 14:01 ` Christoph Hellwig 8 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-23 7:48 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Currently, IO priority set in task's IO context is not reflected in the bio->bi_ioprio for most IO (only io_uring and direct IO set it). This results in odd results where process is submitting some bios with one priority and other bios with a different (unset) priority and due to differing priorities bios cannot be merged. Make sure bio->bi_ioprio is always set on bio submission. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Jan Kara <jack@suse.cz> --- block/blk-mq.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/blk-mq.c b/block/blk-mq.c index e17d822e6051..7548f8aebea8 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2793,6 +2793,9 @@ static inline struct request *blk_mq_get_cached_request(struct request_queue *q, static void bio_set_ioprio(struct bio *bio) { + /* Nobody set ioprio so far? Initialize it based on task's nice value */ + if (IOPRIO_PRIO_CLASS(bio->bi_ioprio) == IOPRIO_CLASS_NONE) + bio->bi_ioprio = get_current_ioprio(); blkcg_set_ioprio(bio); } -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 9/9] block: Always initialize bio IO priority on submit 2022-06-23 7:48 ` [PATCH 9/9] block: Always initialize bio IO priority on submit Jan Kara @ 2022-06-23 14:01 ` Christoph Hellwig 0 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2022-06-23 14:01 UTC (permalink / raw) To: Jan Kara Cc: Jens Axboe, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel On Thu, Jun 23, 2022 at 09:48:34AM +0200, Jan Kara wrote: > Currently, IO priority set in task's IO context is not reflected in the > bio->bi_ioprio for most IO (only io_uring and direct IO set it). This > results in odd results where process is submitting some bios with one > priority and other bios with a different (unset) priority and due to > differing priorities bios cannot be merged. Make sure bio->bi_ioprio is > always set on bio submission. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 0/9 v4] block: Fix IO priority mess
@ 2022-06-21 10:24 Jan Kara
2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara
0 siblings, 1 reply; 27+ messages in thread
From: Jan Kara @ 2022-06-21 10:24 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel,
Jan Kara
Hello,
This is the fourth revision of my patches fixing IO priority handling in the
block layer.
Changes since v3:
* Added Reviewed-by tags from Damien
* Fixed build failure without CONFIG_BLOCK
* Separated refactoring of get_current_ioprio() into a separate patch
Changes since v2:
* added some comments to better explain things
* changed handling of ioprio_get(2)
* a few small tweaks based on Damien's feedback
Original cover letter:
Recently, I've been looking into 10% regression reported by our performance
measurement infrastructure in reaim benchmark that was bisected down to
5a9d041ba2f6 ("block: move io_context creation into where it's needed"). This
didn't really make much sense and it took me a while to understand this but the
culprit is actually in even older commit e70344c05995 ("block: fix default IO
priority handling") and 5a9d041ba2f6 just made the breakage visible.
Essentially the problem was that after these commits some IO was queued with IO
priority class IOPRIO_CLASS_BE while other IO was queued with IOPRIO_CLASS_NONE
and as a result they could not be merged together resulting in performance
regression. I think what commit e70344c05995 ("block: fix default IO priority
handling") did is actually broken not only because of this performance
regression but because of other reasons as well (see changelog of patch 3/8 for
details). Besides this problem, there are multiple other inconsistencies in the
IO priority handling throughout the block stack we have identified when
discussing this with Damien Le Moal. So this patch set aims at fixing all these
various issues.
Note that there are a few choices I've made I'm not 100% sold on. In particular
the conversion of blk-ioprio from rqos is somewhat disputable since it now
incurs a cost similar to blk-throttle in the bio submission fast path (need to
load bio->bi_blkg->pd[ioprio_policy.plid]). If people think the removed
boilerplate code is not worth the cost, I can certainly go via the "additional
rqos hook" path.
Honza
Previous versions:
Link: http://lore.kernel.org/r/20220601132347.13543-1-jack@suse.cz # v1
Link: http://lore.kernel.org/r/20220615160437.5478-1-jack@suse.cz # v2
Link: http://lore.kernel.org/r/20220620160726.19798-1-jack@suse.cz # v3
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 10:24 [PATCH 0/9 v4] block: Fix IO priority mess Jan Kara @ 2022-06-21 10:24 ` Jan Kara 2022-06-21 12:36 ` Jan Kara ` (5 more replies) 0 siblings, 6 replies; 27+ messages in thread From: Jan Kara @ 2022-06-21 10:24 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara get_current_ioprio() operates only on current task. We will need the same functionality for other tasks as well. Generalize get_current_ioprio() for that and also move the bulk out of the header file because it is large enough. Signed-off-by: Jan Kara <jack@suse.cz> --- block/ioprio.c | 26 ++++++++++++++++++++++++++ include/linux/ioprio.h | 26 ++++++++++---------------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/block/ioprio.c b/block/ioprio.c index 2a34cbca18ae..c4e3476155a1 100644 --- a/block/ioprio.c +++ b/block/ioprio.c @@ -138,6 +138,32 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio) return ret; } +/* + * If the task has set an I/O priority, use that. Otherwise, return + * the default I/O priority. + * + * Expected to be called for current task or with task_lock() held to keep + * io_context stable. + */ +int __get_task_ioprio(struct task_struct *p) +{ + struct io_context *ioc = p->io_context; + int prio; + + if (p != current) + lockdep_assert_held(&p->alloc_lock); + if (ioc) + prio = ioc->ioprio; + else + prio = IOPRIO_DEFAULT; + + if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) + prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p), + task_nice_ioprio(p)); + return prio; +} +EXPORT_SYMBOL_GPL(__get_task_ioprio); + static int get_task_ioprio(struct task_struct *p) { int ret; diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h index 61ed6bb4998e..788a8ff57068 100644 --- a/include/linux/ioprio.h +++ b/include/linux/ioprio.h @@ -46,24 +46,18 @@ static inline int task_nice_ioclass(struct task_struct *task) return IOPRIO_CLASS_BE; } -/* - * If the calling process has set an I/O priority, use that. Otherwise, return - * the default I/O priority. - */ -static inline int get_current_ioprio(void) +#ifdef CONFIG_BLOCK +int __get_task_ioprio(struct task_struct *p); +#else +static inline int __get_task_ioprio(int ioprio) { - struct io_context *ioc = current->io_context; - int prio; - - if (ioc) - prio = ioc->ioprio; - else - prio = IOPRIO_DEFAULT; + return IOPRIO_DEFAULT; +} +#endif /* CONFIG_BLOCK */ - if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) - prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current), - task_nice_ioprio(current)); - return prio; +static inline int get_current_ioprio(void) +{ + return __get_task_ioprio(current); } /* -- 2.35.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara @ 2022-06-21 12:36 ` Jan Kara 2022-06-21 13:05 ` Damien Le Moal 2022-06-21 13:01 ` Damien Le Moal ` (4 subsequent siblings) 5 siblings, 1 reply; 27+ messages in thread From: Jan Kara @ 2022-06-21 12:36 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara On Tue 21-06-22 12:24:40, Jan Kara wrote: > get_current_ioprio() operates only on current task. We will need the > same functionality for other tasks as well. Generalize > get_current_ioprio() for that and also move the bulk out of the header > file because it is large enough. > > Signed-off-by: Jan Kara <jack@suse.cz> Bah, I've messed up the prototype of the stub function for !CONFIG_BLOCK. One more fixup will be needed here but let me wait if people have more comments... Honza > --- > block/ioprio.c | 26 ++++++++++++++++++++++++++ > include/linux/ioprio.h | 26 ++++++++++---------------- > 2 files changed, 36 insertions(+), 16 deletions(-) > > diff --git a/block/ioprio.c b/block/ioprio.c > index 2a34cbca18ae..c4e3476155a1 100644 > --- a/block/ioprio.c > +++ b/block/ioprio.c > @@ -138,6 +138,32 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio) > return ret; > } > > +/* > + * If the task has set an I/O priority, use that. Otherwise, return > + * the default I/O priority. > + * > + * Expected to be called for current task or with task_lock() held to keep > + * io_context stable. > + */ > +int __get_task_ioprio(struct task_struct *p) > +{ > + struct io_context *ioc = p->io_context; > + int prio; > + > + if (p != current) > + lockdep_assert_held(&p->alloc_lock); > + if (ioc) > + prio = ioc->ioprio; > + else > + prio = IOPRIO_DEFAULT; > + > + if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) > + prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p), > + task_nice_ioprio(p)); > + return prio; > +} > +EXPORT_SYMBOL_GPL(__get_task_ioprio); > + > static int get_task_ioprio(struct task_struct *p) > { > int ret; > diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h > index 61ed6bb4998e..788a8ff57068 100644 > --- a/include/linux/ioprio.h > +++ b/include/linux/ioprio.h > @@ -46,24 +46,18 @@ static inline int task_nice_ioclass(struct task_struct *task) > return IOPRIO_CLASS_BE; > } > > -/* > - * If the calling process has set an I/O priority, use that. Otherwise, return > - * the default I/O priority. > - */ > -static inline int get_current_ioprio(void) > +#ifdef CONFIG_BLOCK > +int __get_task_ioprio(struct task_struct *p); > +#else > +static inline int __get_task_ioprio(int ioprio) > { > - struct io_context *ioc = current->io_context; > - int prio; > - > - if (ioc) > - prio = ioc->ioprio; > - else > - prio = IOPRIO_DEFAULT; > + return IOPRIO_DEFAULT; > +} > +#endif /* CONFIG_BLOCK */ > > - if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) > - prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current), > - task_nice_ioprio(current)); > - return prio; > +static inline int get_current_ioprio(void) > +{ > + return __get_task_ioprio(current); > } > > /* > -- > 2.35.3 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 12:36 ` Jan Kara @ 2022-06-21 13:05 ` Damien Le Moal 0 siblings, 0 replies; 27+ messages in thread From: Damien Le Moal @ 2022-06-21 13:05 UTC (permalink / raw) To: Jan Kara, Jens Axboe; +Cc: linux-block, Bart Van Assche, Niklas Cassel On 6/21/22 21:36, Jan Kara wrote: > On Tue 21-06-22 12:24:40, Jan Kara wrote: >> get_current_ioprio() operates only on current task. We will need the >> same functionality for other tasks as well. Generalize >> get_current_ioprio() for that and also move the bulk out of the header >> file because it is large enough. >> >> Signed-off-by: Jan Kara <jack@suse.cz> > > Bah, I've messed up the prototype of the stub function for !CONFIG_BLOCK. > One more fixup will be needed here but let me wait if people have more > comments... Oops. Missed it :) > > Honza > >> --- >> block/ioprio.c | 26 ++++++++++++++++++++++++++ >> include/linux/ioprio.h | 26 ++++++++++---------------- >> 2 files changed, 36 insertions(+), 16 deletions(-) >> >> diff --git a/block/ioprio.c b/block/ioprio.c >> index 2a34cbca18ae..c4e3476155a1 100644 >> --- a/block/ioprio.c >> +++ b/block/ioprio.c >> @@ -138,6 +138,32 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio) >> return ret; >> } >> >> +/* >> + * If the task has set an I/O priority, use that. Otherwise, return >> + * the default I/O priority. >> + * >> + * Expected to be called for current task or with task_lock() held to keep >> + * io_context stable. >> + */ >> +int __get_task_ioprio(struct task_struct *p) >> +{ >> + struct io_context *ioc = p->io_context; >> + int prio; >> + >> + if (p != current) >> + lockdep_assert_held(&p->alloc_lock); >> + if (ioc) >> + prio = ioc->ioprio; >> + else >> + prio = IOPRIO_DEFAULT; >> + >> + if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) >> + prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p), >> + task_nice_ioprio(p)); >> + return prio; >> +} >> +EXPORT_SYMBOL_GPL(__get_task_ioprio); >> + >> static int get_task_ioprio(struct task_struct *p) >> { >> int ret; >> diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h >> index 61ed6bb4998e..788a8ff57068 100644 >> --- a/include/linux/ioprio.h >> +++ b/include/linux/ioprio.h >> @@ -46,24 +46,18 @@ static inline int task_nice_ioclass(struct task_struct *task) >> return IOPRIO_CLASS_BE; >> } >> >> -/* >> - * If the calling process has set an I/O priority, use that. Otherwise, return >> - * the default I/O priority. >> - */ >> -static inline int get_current_ioprio(void) >> +#ifdef CONFIG_BLOCK >> +int __get_task_ioprio(struct task_struct *p); >> +#else >> +static inline int __get_task_ioprio(int ioprio) >> { >> - struct io_context *ioc = current->io_context; >> - int prio; >> - >> - if (ioc) >> - prio = ioc->ioprio; >> - else >> - prio = IOPRIO_DEFAULT; >> + return IOPRIO_DEFAULT; >> +} >> +#endif /* CONFIG_BLOCK */ >> >> - if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) >> - prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current), >> - task_nice_ioprio(current)); >> - return prio; >> +static inline int get_current_ioprio(void) >> +{ >> + return __get_task_ioprio(current); >> } >> >> /* >> -- >> 2.35.3 >> -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara 2022-06-21 12:36 ` Jan Kara @ 2022-06-21 13:01 ` Damien Le Moal 2022-06-21 23:58 ` kernel test robot ` (3 subsequent siblings) 5 siblings, 0 replies; 27+ messages in thread From: Damien Le Moal @ 2022-06-21 13:01 UTC (permalink / raw) To: Jan Kara, Jens Axboe; +Cc: linux-block, Bart Van Assche, Niklas Cassel On 6/21/22 19:24, Jan Kara wrote: > get_current_ioprio() operates only on current task. We will need the > same functionality for other tasks as well. Generalize > get_current_ioprio() for that and also move the bulk out of the header > file because it is large enough. > > Signed-off-by: Jan Kara <jack@suse.cz> Looks OK to me. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > --- > block/ioprio.c | 26 ++++++++++++++++++++++++++ > include/linux/ioprio.h | 26 ++++++++++---------------- > 2 files changed, 36 insertions(+), 16 deletions(-) > > diff --git a/block/ioprio.c b/block/ioprio.c > index 2a34cbca18ae..c4e3476155a1 100644 > --- a/block/ioprio.c > +++ b/block/ioprio.c > @@ -138,6 +138,32 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio) > return ret; > } > > +/* > + * If the task has set an I/O priority, use that. Otherwise, return > + * the default I/O priority. > + * > + * Expected to be called for current task or with task_lock() held to keep > + * io_context stable. > + */ > +int __get_task_ioprio(struct task_struct *p) > +{ > + struct io_context *ioc = p->io_context; > + int prio; > + > + if (p != current) > + lockdep_assert_held(&p->alloc_lock); > + if (ioc) > + prio = ioc->ioprio; > + else > + prio = IOPRIO_DEFAULT; > + > + if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) > + prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p), > + task_nice_ioprio(p)); > + return prio; > +} > +EXPORT_SYMBOL_GPL(__get_task_ioprio); > + > static int get_task_ioprio(struct task_struct *p) > { > int ret; > diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h > index 61ed6bb4998e..788a8ff57068 100644 > --- a/include/linux/ioprio.h > +++ b/include/linux/ioprio.h > @@ -46,24 +46,18 @@ static inline int task_nice_ioclass(struct task_struct *task) > return IOPRIO_CLASS_BE; > } > > -/* > - * If the calling process has set an I/O priority, use that. Otherwise, return > - * the default I/O priority. > - */ > -static inline int get_current_ioprio(void) > +#ifdef CONFIG_BLOCK > +int __get_task_ioprio(struct task_struct *p); > +#else > +static inline int __get_task_ioprio(int ioprio) > { > - struct io_context *ioc = current->io_context; > - int prio; > - > - if (ioc) > - prio = ioc->ioprio; > - else > - prio = IOPRIO_DEFAULT; > + return IOPRIO_DEFAULT; > +} > +#endif /* CONFIG_BLOCK */ > > - if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) > - prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current), > - task_nice_ioprio(current)); > - return prio; > +static inline int get_current_ioprio(void) > +{ > + return __get_task_ioprio(current); > } > > /* -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara 2022-06-21 12:36 ` Jan Kara 2022-06-21 13:01 ` Damien Le Moal @ 2022-06-21 23:58 ` kernel test robot 2022-06-22 0:50 ` kernel test robot ` (2 subsequent siblings) 5 siblings, 0 replies; 27+ messages in thread From: kernel test robot @ 2022-06-21 23:58 UTC (permalink / raw) To: Jan Kara, Jens Axboe Cc: llvm, kbuild-all, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Hi Jan, I love your patch! Perhaps something to improve: [auto build test WARNING on axboe-block/for-next] [also build test WARNING on linus/master v5.19-rc3 next-20220621] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next config: hexagon-randconfig-r016-20220622 (https://download.01.org/0day-ci/archive/20220622/202206220716.sxn2tinw-lkp@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 git checkout 8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon prepare If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from arch/hexagon/kernel/asm-offsets.c:12: In file included from include/linux/compat.h:17: In file included from include/linux/fs.h:38: >> include/linux/ioprio.h:60:27: warning: incompatible pointer to integer conversion passing 'struct task_struct *' to parameter of type 'int' [-Wint-conversion] return __get_task_ioprio(current); ^~~~~~~ include/asm-generic/current.h:8:17: note: expanded from macro 'current' #define current get_current() ^~~~~~~~~~~~~ include/asm-generic/current.h:7:23: note: expanded from macro 'get_current' #define get_current() (current_thread_info()->task) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/ioprio.h:52:41: note: passing argument to parameter 'ioprio' here static inline int __get_task_ioprio(int ioprio) ^ 1 warning generated. -- In file included from drivers/iio/proximity/isl29501.c:13: In file included from include/linux/i2c.h:19: In file included from include/linux/regulator/consumer.h:35: In file included from include/linux/suspend.h:5: In file included from include/linux/swap.h:9: In file included from include/linux/memcontrol.h:13: In file included from include/linux/cgroup.h:17: In file included from include/linux/fs.h:38: >> include/linux/ioprio.h:60:27: warning: incompatible pointer to integer conversion passing 'struct task_struct *' to parameter of type 'int' [-Wint-conversion] return __get_task_ioprio(current); ^~~~~~~ include/asm-generic/current.h:8:17: note: expanded from macro 'current' #define current get_current() ^~~~~~~~~~~~~ include/asm-generic/current.h:7:23: note: expanded from macro 'get_current' #define get_current() (current_thread_info()->task) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/ioprio.h:52:41: note: passing argument to parameter 'ioprio' here static inline int __get_task_ioprio(int ioprio) ^ drivers/iio/proximity/isl29501.c:1000:34: warning: unused variable 'isl29501_i2c_matches' [-Wunused-const-variable] static const struct of_device_id isl29501_i2c_matches[] = { ^ 2 warnings generated. -- In file included from drivers/iio/proximity/sx9500.c:13: In file included from include/linux/i2c.h:19: In file included from include/linux/regulator/consumer.h:35: In file included from include/linux/suspend.h:5: In file included from include/linux/swap.h:9: In file included from include/linux/memcontrol.h:13: In file included from include/linux/cgroup.h:17: In file included from include/linux/fs.h:38: >> include/linux/ioprio.h:60:27: warning: incompatible pointer to integer conversion passing 'struct task_struct *' to parameter of type 'int' [-Wint-conversion] return __get_task_ioprio(current); ^~~~~~~ include/asm-generic/current.h:8:17: note: expanded from macro 'current' #define current get_current() ^~~~~~~~~~~~~ include/asm-generic/current.h:7:23: note: expanded from macro 'get_current' #define get_current() (current_thread_info()->task) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/ioprio.h:52:41: note: passing argument to parameter 'ioprio' here static inline int __get_task_ioprio(int ioprio) ^ drivers/iio/proximity/sx9500.c:1035:36: warning: unused variable 'sx9500_acpi_match' [-Wunused-const-variable] static const struct acpi_device_id sx9500_acpi_match[] = { ^ 2 warnings generated. -- In file included from arch/hexagon/kernel/asm-offsets.c:12: In file included from include/linux/compat.h:17: In file included from include/linux/fs.h:38: >> include/linux/ioprio.h:60:27: warning: incompatible pointer to integer conversion passing 'struct task_struct *' to parameter of type 'int' [-Wint-conversion] return __get_task_ioprio(current); ^~~~~~~ include/asm-generic/current.h:8:17: note: expanded from macro 'current' #define current get_current() ^~~~~~~~~~~~~ include/asm-generic/current.h:7:23: note: expanded from macro 'get_current' #define get_current() (current_thread_info()->task) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/ioprio.h:52:41: note: passing argument to parameter 'ioprio' here static inline int __get_task_ioprio(int ioprio) ^ 1 warning generated. <stdin>:1517:2: warning: syscall clone3 not implemented [-W#warnings] #warning syscall clone3 not implemented ^ 1 warning generated. vim +60 include/linux/ioprio.h 57 58 static inline int get_current_ioprio(void) 59 { > 60 return __get_task_ioprio(current); 61 } 62 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara ` (2 preceding siblings ...) 2022-06-21 23:58 ` kernel test robot @ 2022-06-22 0:50 ` kernel test robot 2022-06-22 1:41 ` kernel test robot 2022-06-22 11:49 ` kernel test robot 5 siblings, 0 replies; 27+ messages in thread From: kernel test robot @ 2022-06-22 0:50 UTC (permalink / raw) To: Jan Kara, Jens Axboe Cc: kbuild-all, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Hi Jan, I love your patch! Perhaps something to improve: [auto build test WARNING on axboe-block/for-next] [also build test WARNING on linus/master v5.19-rc3 next-20220621] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next config: riscv-nommu_k210_defconfig (https://download.01.org/0day-ci/archive/20220622/202206220810.R6I8vpfk-lkp@intel.com/config) compiler: riscv64-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 git checkout 8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv prepare If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from include/linux/thread_info.h:23, from include/asm-generic/preempt.h:5, from ./arch/riscv/include/generated/asm/preempt.h:1, from include/linux/preempt.h:78, from include/linux/spinlock.h:55, from include/linux/mmzone.h:8, from include/linux/gfp.h:6, from include/linux/mm.h:7, from arch/riscv/kernel/asm-offsets.c:10: include/linux/ioprio.h: In function 'get_current_ioprio': >> arch/riscv/include/asm/current.h:34:17: warning: passing argument 1 of '__get_task_ioprio' makes integer from pointer without a cast [-Wint-conversion] 34 | #define current get_current() | ^~~~~~~~~~~~~ | | | struct task_struct * include/linux/ioprio.h:60:34: note: in expansion of macro 'current' 60 | return __get_task_ioprio(current); | ^~~~~~~ In file included from include/linux/fs.h:38, from include/linux/huge_mm.h:8, from include/linux/mm.h:700, from arch/riscv/kernel/asm-offsets.c:10: include/linux/ioprio.h:52:41: note: expected 'int' but argument is of type 'struct task_struct *' 52 | static inline int __get_task_ioprio(int ioprio) | ~~~~^~~~~~ -- In file included from include/linux/thread_info.h:23, from include/asm-generic/preempt.h:5, from ./arch/riscv/include/generated/asm/preempt.h:1, from include/linux/preempt.h:78, from include/linux/smp.h:110, from arch/riscv/include/asm/mmiowb.h:12, from arch/riscv/include/asm/mmio.h:15, from arch/riscv/include/asm/clint.h:10, from arch/riscv/include/asm/timex.h:15, from include/linux/timex.h:67, from include/linux/time32.h:13, from include/linux/time.h:60, from include/linux/stat.h:19, from include/linux/module.h:13, from drivers/clk/clkdev.c:9: include/linux/ioprio.h: In function 'get_current_ioprio': >> arch/riscv/include/asm/current.h:34:17: warning: passing argument 1 of '__get_task_ioprio' makes integer from pointer without a cast [-Wint-conversion] 34 | #define current get_current() | ^~~~~~~~~~~~~ | | | struct task_struct * include/linux/ioprio.h:60:34: note: in expansion of macro 'current' 60 | return __get_task_ioprio(current); | ^~~~~~~ In file included from include/linux/fs.h:38, from include/linux/compat.h:17, from arch/riscv/include/asm/elf.h:12, from include/linux/elf.h:6, from include/linux/module.h:19, from drivers/clk/clkdev.c:9: include/linux/ioprio.h:52:41: note: expected 'int' but argument is of type 'struct task_struct *' 52 | static inline int __get_task_ioprio(int ioprio) | ~~~~^~~~~~ drivers/clk/clkdev.c: In function 'vclkdev_alloc': drivers/clk/clkdev.c:173:17: warning: function 'vclkdev_alloc' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format] 173 | vscnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); | ^~~~~~~~~~ -- In file included from include/linux/thread_info.h:23, from include/asm-generic/preempt.h:5, from ./arch/riscv/include/generated/asm/preempt.h:1, from include/linux/preempt.h:78, from include/linux/spinlock.h:55, from include/linux/mmzone.h:8, from include/linux/gfp.h:6, from include/linux/mm.h:7, from arch/riscv/kernel/asm-offsets.c:10: include/linux/ioprio.h: In function 'get_current_ioprio': >> arch/riscv/include/asm/current.h:34:17: warning: passing argument 1 of '__get_task_ioprio' makes integer from pointer without a cast [-Wint-conversion] 34 | #define current get_current() | ^~~~~~~~~~~~~ | | | struct task_struct * include/linux/ioprio.h:60:34: note: in expansion of macro 'current' 60 | return __get_task_ioprio(current); | ^~~~~~~ In file included from include/linux/fs.h:38, from include/linux/huge_mm.h:8, from include/linux/mm.h:700, from arch/riscv/kernel/asm-offsets.c:10: include/linux/ioprio.h:52:41: note: expected 'int' but argument is of type 'struct task_struct *' 52 | static inline int __get_task_ioprio(int ioprio) | ~~~~^~~~~~ vim +/__get_task_ioprio +34 arch/riscv/include/asm/current.h 7db91e57a0acde Palmer Dabbelt 2017-07-10 33 7db91e57a0acde Palmer Dabbelt 2017-07-10 @34 #define current get_current() 7db91e57a0acde Palmer Dabbelt 2017-07-10 35 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara ` (3 preceding siblings ...) 2022-06-22 0:50 ` kernel test robot @ 2022-06-22 1:41 ` kernel test robot 2022-06-22 11:49 ` kernel test robot 5 siblings, 0 replies; 27+ messages in thread From: kernel test robot @ 2022-06-22 1:41 UTC (permalink / raw) To: Jan Kara, Jens Axboe Cc: llvm, kbuild-all, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Hi Jan, I love your patch! Yet something to improve: [auto build test ERROR on axboe-block/for-next] [also build test ERROR on linus/master v5.19-rc3 next-20220621] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next config: s390-randconfig-r021-20220622 (https://download.01.org/0day-ci/archive/20220622/202206220956.wXt005QG-lkp@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install s390 cross compiling tool for clang build # apt-get install binutils-s390x-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 git checkout 8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash lib/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from lib/test_bitops.c:9: In file included from include/linux/module.h:19: In file included from include/linux/elf.h:6: In file included from arch/s390/include/asm/elf.h:160: In file included from include/linux/compat.h:17: In file included from include/linux/fs.h:38: >> include/linux/ioprio.h:60:27: error: incompatible pointer to integer conversion passing 'struct task_struct *' to parameter of type 'int' [-Werror,-Wint-conversion] return __get_task_ioprio(current); ^~~~~~~ arch/s390/include/asm/current.h:17:17: note: expanded from macro 'current' #define current ((struct task_struct *const)S390_lowcore.current_task) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/ioprio.h:52:41: note: passing argument to parameter 'ioprio' here static inline int __get_task_ioprio(int ioprio) ^ 1 error generated. vim +60 include/linux/ioprio.h 57 58 static inline int get_current_ioprio(void) 59 { > 60 return __get_task_ioprio(current); 61 } 62 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block: Generalize get_current_ioprio() for any task 2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara ` (4 preceding siblings ...) 2022-06-22 1:41 ` kernel test robot @ 2022-06-22 11:49 ` kernel test robot 5 siblings, 0 replies; 27+ messages in thread From: kernel test robot @ 2022-06-22 11:49 UTC (permalink / raw) To: Jan Kara, Jens Axboe Cc: kbuild-all, linux-block, Damien Le Moal, Bart Van Assche, Niklas Cassel, Jan Kara Hi Jan, I love your patch! Perhaps something to improve: [auto build test WARNING on axboe-block/for-next] [also build test WARNING on linus/master v5.19-rc3 next-20220622] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next config: x86_64-randconfig-s022 (https://download.01.org/0day-ci/archive/20220622/202206221920.pxBCy0Di-lkp@intel.com/config) compiler: gcc-11 (Debian 11.3.0-3) 11.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.4-31-g4880bd19-dirty # https://github.com/intel-lab-lkp/linux/commit/8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jan-Kara/block-Fix-IO-priority-mess/20220621-183235 git checkout 8421c851d4fe5f4b9d9d6870ada8ccd0b48a4012 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) fs/read_write.c: note: in included file (through include/linux/fs.h, include/linux/fsnotify_backend.h, include/linux/fsnotify.h): include/linux/ioprio.h:60:34: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected int ioprio @@ got struct task_struct * @@ include/linux/ioprio.h:60:34: sparse: expected int ioprio include/linux/ioprio.h:60:34: sparse: got struct task_struct * fs/read_write.c: note: in included file (through include/linux/thread_info.h, arch/x86/include/asm/preempt.h, include/linux/preempt.h, ...): >> arch/x86/include/asm/current.h:15:16: sparse: sparse: non size-preserving pointer to integer cast fs/read_write.c: note: in included file (through include/linux/fs.h, include/linux/fsnotify_backend.h, include/linux/fsnotify.h): include/linux/ioprio.h:60:34: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected int ioprio @@ got struct task_struct * @@ include/linux/ioprio.h:60:34: sparse: expected int ioprio include/linux/ioprio.h:60:34: sparse: got struct task_struct * fs/read_write.c: note: in included file (through include/linux/thread_info.h, arch/x86/include/asm/preempt.h, include/linux/preempt.h, ...): >> arch/x86/include/asm/current.h:15:16: sparse: sparse: non size-preserving pointer to integer cast fs/read_write.c: note: in included file (through include/linux/fs.h, include/linux/fsnotify_backend.h, include/linux/fsnotify.h): include/linux/ioprio.h:60:34: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected int ioprio @@ got struct task_struct * @@ include/linux/ioprio.h:60:34: sparse: expected int ioprio include/linux/ioprio.h:60:34: sparse: got struct task_struct * fs/read_write.c: note: in included file (through include/linux/thread_info.h, arch/x86/include/asm/preempt.h, include/linux/preempt.h, ...): >> arch/x86/include/asm/current.h:15:16: sparse: sparse: non size-preserving pointer to integer cast fs/read_write.c: note: in included file (through include/linux/fs.h, include/linux/fsnotify_backend.h, include/linux/fsnotify.h): include/linux/ioprio.h:60:34: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected int ioprio @@ got struct task_struct * @@ include/linux/ioprio.h:60:34: sparse: expected int ioprio include/linux/ioprio.h:60:34: sparse: got struct task_struct * fs/read_write.c: note: in included file (through include/linux/thread_info.h, arch/x86/include/asm/preempt.h, include/linux/preempt.h, ...): >> arch/x86/include/asm/current.h:15:16: sparse: sparse: non size-preserving pointer to integer cast fs/read_write.c: note: in included file (through include/linux/fs.h, include/linux/fsnotify_backend.h, include/linux/fsnotify.h): include/linux/ioprio.h:60:34: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected int ioprio @@ got struct task_struct * @@ include/linux/ioprio.h:60:34: sparse: expected int ioprio include/linux/ioprio.h:60:34: sparse: got struct task_struct * fs/read_write.c: note: in included file (through include/linux/thread_info.h, arch/x86/include/asm/preempt.h, include/linux/preempt.h, ...): >> arch/x86/include/asm/current.h:15:16: sparse: sparse: non size-preserving pointer to integer cast -- fs/seq_file.c:938:9: sparse: sparse: incompatible types in comparison expression (different address spaces): fs/seq_file.c:938:9: sparse: struct list_head [noderef] __rcu * fs/seq_file.c:938:9: sparse: struct list_head * fs/seq_file.c:938:9: sparse: sparse: incompatible types in comparison expression (different address spaces): fs/seq_file.c:938:9: sparse: struct list_head [noderef] __rcu * fs/seq_file.c:938:9: sparse: struct list_head * fs/seq_file.c:960:12: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct list_head *lh @@ got struct list_head [noderef] __rcu * @@ fs/seq_file.c:960:12: sparse: expected struct list_head *lh fs/seq_file.c:960:12: sparse: got struct list_head [noderef] __rcu * fs/seq_file.c:1087:24: sparse: sparse: incompatible types in comparison expression (different address spaces): fs/seq_file.c:1087:24: sparse: struct hlist_node [noderef] __rcu * fs/seq_file.c:1087:24: sparse: struct hlist_node * fs/seq_file.c:1089:24: sparse: sparse: incompatible types in comparison expression (different address spaces): fs/seq_file.c:1089:24: sparse: struct hlist_node [noderef] __rcu * fs/seq_file.c:1089:24: sparse: struct hlist_node * fs/seq_file.c: note: in included file (through include/linux/fs.h): include/linux/ioprio.h:60:34: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected int ioprio @@ got struct task_struct * @@ include/linux/ioprio.h:60:34: sparse: expected int ioprio include/linux/ioprio.h:60:34: sparse: got struct task_struct * fs/seq_file.c: note: in included file (through include/linux/thread_info.h, arch/x86/include/asm/preempt.h, include/linux/preempt.h, ...): >> arch/x86/include/asm/current.h:15:16: sparse: sparse: non size-preserving pointer to integer cast -- fs/splice.c: note: in included file (through include/linux/fs.h, include/linux/highmem.h, include/linux/bvec.h): include/linux/ioprio.h:60:34: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected int ioprio @@ got struct task_struct * @@ include/linux/ioprio.h:60:34: sparse: expected int ioprio include/linux/ioprio.h:60:34: sparse: got struct task_struct * fs/splice.c: note: in included file (through include/linux/thread_info.h, arch/x86/include/asm/preempt.h, include/linux/preempt.h, ...): >> arch/x86/include/asm/current.h:15:16: sparse: sparse: non size-preserving pointer to integer cast vim +15 arch/x86/include/asm/current.h f0766440dda7ace include/asm-x86/current.h Christoph Lameter 2008-05-09 12 f0766440dda7ace include/asm-x86/current.h Christoph Lameter 2008-05-09 13 static __always_inline struct task_struct *get_current(void) f0766440dda7ace include/asm-x86/current.h Christoph Lameter 2008-05-09 14 { c6ae41e7d469f00 arch/x86/include/asm/current.h Alex Shi 2012-05-11 @15 return this_cpu_read_stable(current_task); f0766440dda7ace include/asm-x86/current.h Christoph Lameter 2008-05-09 16 } f0766440dda7ace include/asm-x86/current.h Christoph Lameter 2008-05-09 17 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-06-23 14:23 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-23 7:48 [PATCH 0/9 v5] block: Fix IO priority mess Jan Kara 2022-06-23 7:48 ` [PATCH 1/9] block: fix default IO priority handling again Jan Kara 2022-06-23 13:57 ` Christoph Hellwig 2022-06-23 14:23 ` Jens Axboe 2022-06-23 7:48 ` [PATCH 2/9] block: Return effective IO priority from get_current_ioprio() Jan Kara 2022-06-23 13:57 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara 2022-06-23 13:59 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 4/9] block: Make ioprio_best() static Jan Kara 2022-06-23 13:58 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 5/9] block: Fix handling of tasks without ioprio in ioprio_get(2) Jan Kara 2022-06-23 7:48 ` [PATCH 6/9] blk-ioprio: Remove unneeded field Jan Kara 2022-06-23 14:00 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 7/9] blk-ioprio: Convert from rqos policy to direct call Jan Kara 2022-06-23 14:01 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 8/9] block: Initialize bio priority earlier Jan Kara 2022-06-23 14:01 ` Christoph Hellwig 2022-06-23 7:48 ` [PATCH 9/9] block: Always initialize bio IO priority on submit Jan Kara 2022-06-23 14:01 ` Christoph Hellwig -- strict thread matches above, loose matches on Subject: below -- 2022-06-21 10:24 [PATCH 0/9 v4] block: Fix IO priority mess Jan Kara 2022-06-21 10:24 ` [PATCH 3/9] block: Generalize get_current_ioprio() for any task Jan Kara 2022-06-21 12:36 ` Jan Kara 2022-06-21 13:05 ` Damien Le Moal 2022-06-21 13:01 ` Damien Le Moal 2022-06-21 23:58 ` kernel test robot 2022-06-22 0:50 ` kernel test robot 2022-06-22 1:41 ` kernel test robot 2022-06-22 11:49 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).