* more I/O context cleanup v2
@ 2021-12-09 6:31 Christoph Hellwig
2021-12-09 6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
` (11 more replies)
0 siblings, 12 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Hi Jens,
this series dusts off a few more bits in the I/O context handling, and
makes a chunk of code only needed by the BFQ I/O scheduler optional.
Changes since v1:
- fix a comment typo
- keep the RCU critical section in ioc_clear_queue
- add a few more cleanups
Diffstat:
block/Kconfig | 3
block/Kconfig.iosched | 1
block/blk-ioc.c | 247 ++++++++++++++++++++--------------------------
block/blk.h | 6 +
block/ioprio.c | 32 -----
include/linux/iocontext.h | 9 -
6 files changed, 125 insertions(+), 173 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 01/11] block: remove the nr_task field from struct io_context
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-09 6:31 ` [PATCH 02/11] block: simplify struct io_context refcounting Christoph Hellwig
` (10 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Nothing ever looks at ->nr_tasks, so remove it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
block/blk-ioc.c | 3 ---
include/linux/iocontext.h | 1 -
2 files changed, 4 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 536fb496ad763..96336c2134efa 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -207,7 +207,6 @@ void exit_io_context(struct task_struct *task)
task->io_context = NULL;
task_unlock(task);
- atomic_dec(&ioc->nr_tasks);
put_io_context_active(ioc);
}
@@ -259,7 +258,6 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
return NULL;
atomic_long_set(&ioc->refcount, 1);
- atomic_set(&ioc->nr_tasks, 1);
atomic_set(&ioc->active_ref, 1);
spin_lock_init(&ioc->lock);
INIT_RADIX_TREE(&ioc->icq_tree, GFP_ATOMIC);
@@ -339,7 +337,6 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
if (clone_flags & CLONE_IO) {
atomic_long_inc(&ioc->refcount);
atomic_inc(&ioc->active_ref);
- atomic_inc(&ioc->nr_tasks);
tsk->io_context = ioc;
} else if (ioprio_valid(ioc->ioprio)) {
tsk->io_context = alloc_io_context(GFP_KERNEL, NUMA_NO_NODE);
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index c1229fbd6691c..82c7f4f5f4f59 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -99,7 +99,6 @@ struct io_cq {
struct io_context {
atomic_long_t refcount;
atomic_t active_ref;
- atomic_t nr_tasks;
/* all the fields below are protected by this lock */
spinlock_t lock;
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/11] block: simplify struct io_context refcounting
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
2021-12-09 6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-09 6:31 ` [PATCH 03/11] block: refactor put_iocontext_active Christoph Hellwig
` (9 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Don't hold a reference to ->refcount for each active reference, but
just one for all active references.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
block/blk-ioc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 96336c2134efa..9cde3906be3c6 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -180,10 +180,8 @@ static void put_io_context_active(struct io_context *ioc)
{
struct io_cq *icq;
- if (!atomic_dec_and_test(&ioc->active_ref)) {
- put_io_context(ioc);
+ if (!atomic_dec_and_test(&ioc->active_ref))
return;
- }
spin_lock_irq(&ioc->lock);
hlist_for_each_entry(icq, &ioc->icq_list, ioc_node) {
@@ -335,7 +333,6 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
* Share io context with parent, if CLONE_IO is set
*/
if (clone_flags & CLONE_IO) {
- atomic_long_inc(&ioc->refcount);
atomic_inc(&ioc->active_ref);
tsk->io_context = ioc;
} else if (ioprio_valid(ioc->ioprio)) {
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/11] block: refactor put_iocontext_active
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
2021-12-09 6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
2021-12-09 6:31 ` [PATCH 02/11] block: simplify struct io_context refcounting Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-09 6:31 ` [PATCH 04/11] block: remove the NULL ioc check in put_io_context Christoph Hellwig
` (8 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Factor out a ioc_exit_icqs helper to tear down the icqs and the fold
the rest of put_iocontext_active into exit_io_context.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
block/blk-ioc.c | 41 ++++++++++++++---------------------------
1 file changed, 14 insertions(+), 27 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 9cde3906be3c6..0380e33930e31 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -54,6 +54,16 @@ static void ioc_exit_icq(struct io_cq *icq)
icq->flags |= ICQ_EXITED;
}
+static void ioc_exit_icqs(struct io_context *ioc)
+{
+ struct io_cq *icq;
+
+ spin_lock_irq(&ioc->lock);
+ hlist_for_each_entry(icq, &ioc->icq_list, ioc_node)
+ ioc_exit_icq(icq);
+ spin_unlock_irq(&ioc->lock);
+}
+
/*
* Release an icq. Called with ioc locked for blk-mq, and with both ioc
* and queue locked for legacy.
@@ -169,32 +179,6 @@ void put_io_context(struct io_context *ioc)
}
EXPORT_SYMBOL_GPL(put_io_context);
-/**
- * put_io_context_active - put active reference on ioc
- * @ioc: ioc of interest
- *
- * Put an active reference to an ioc. If active reference reaches zero after
- * put, @ioc can never issue further IOs and ioscheds are notified.
- */
-static void put_io_context_active(struct io_context *ioc)
-{
- struct io_cq *icq;
-
- if (!atomic_dec_and_test(&ioc->active_ref))
- return;
-
- spin_lock_irq(&ioc->lock);
- hlist_for_each_entry(icq, &ioc->icq_list, ioc_node) {
- if (icq->flags & ICQ_EXITED)
- continue;
-
- ioc_exit_icq(icq);
- }
- spin_unlock_irq(&ioc->lock);
-
- put_io_context(ioc);
-}
-
/* Called by the exiting task */
void exit_io_context(struct task_struct *task)
{
@@ -205,7 +189,10 @@ void exit_io_context(struct task_struct *task)
task->io_context = NULL;
task_unlock(task);
- put_io_context_active(ioc);
+ if (atomic_dec_and_test(&ioc->active_ref)) {
+ ioc_exit_icqs(ioc);
+ put_io_context(ioc);
+ }
}
static void __ioc_clear_queue(struct list_head *icq_list)
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/11] block: remove the NULL ioc check in put_io_context
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (2 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 03/11] block: refactor put_iocontext_active Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-09 6:31 ` [PATCH 05/11] block: refactor put_io_context Christoph Hellwig
` (7 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
No caller passes in a NULL pointer, so remove the check.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
block/blk-ioc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 0380e33930e31..04f3d2b0ca7db 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -155,9 +155,6 @@ void put_io_context(struct io_context *ioc)
unsigned long flags;
bool free_ioc = false;
- if (ioc == NULL)
- return;
-
BUG_ON(atomic_long_read(&ioc->refcount) <= 0);
/*
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/11] block: refactor put_io_context
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (3 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 04/11] block: remove the NULL ioc check in put_io_context Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-09 6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
` (6 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Move the code to delay freeing the icqs into a separate helper.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
block/blk-ioc.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 04f3d2b0ca7db..ca996214c10a6 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -143,6 +143,24 @@ static void ioc_release_fn(struct work_struct *work)
kmem_cache_free(iocontext_cachep, ioc);
}
+/*
+ * Releasing icqs requires reverse order double locking and we may already be
+ * holding a queue_lock. Do it asynchronously from a workqueue.
+ */
+static bool ioc_delay_free(struct io_context *ioc)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ioc->lock, flags);
+ if (!hlist_empty(&ioc->icq_list)) {
+ queue_work(system_power_efficient_wq, &ioc->release_work);
+ spin_unlock_irqrestore(&ioc->lock, flags);
+ return true;
+ }
+ spin_unlock_irqrestore(&ioc->lock, flags);
+ return false;
+}
+
/**
* put_io_context - put a reference of io_context
* @ioc: io_context to put
@@ -152,26 +170,8 @@ static void ioc_release_fn(struct work_struct *work)
*/
void put_io_context(struct io_context *ioc)
{
- unsigned long flags;
- bool free_ioc = false;
-
BUG_ON(atomic_long_read(&ioc->refcount) <= 0);
-
- /*
- * Releasing ioc requires reverse order double locking and we may
- * already be holding a queue_lock. Do it asynchronously from wq.
- */
- if (atomic_long_dec_and_test(&ioc->refcount)) {
- spin_lock_irqsave(&ioc->lock, flags);
- if (!hlist_empty(&ioc->icq_list))
- queue_work(system_power_efficient_wq,
- &ioc->release_work);
- else
- free_ioc = true;
- spin_unlock_irqrestore(&ioc->lock, flags);
- }
-
- if (free_ioc)
+ if (atomic_long_dec_and_test(&ioc->refcount) && !ioc_delay_free(ioc))
kmem_cache_free(iocontext_cachep, ioc);
}
EXPORT_SYMBOL_GPL(put_io_context);
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/11] block: cleanup ioc_clear_queue
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (4 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 05/11] block: refactor put_io_context Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-14 15:50 ` Jan Kara
2021-12-09 6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
` (5 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Fold __ioc_clear_queue into ioc_clear_queue and switch to always
use plain _irq locking instead of the more expensive _irqsave that
is not needed here.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-ioc.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index ca996214c10a6..f98a29ee8f362 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -192,27 +192,6 @@ void exit_io_context(struct task_struct *task)
}
}
-static void __ioc_clear_queue(struct list_head *icq_list)
-{
- unsigned long flags;
-
- rcu_read_lock();
- while (!list_empty(icq_list)) {
- struct io_cq *icq = list_entry(icq_list->next,
- struct io_cq, q_node);
- struct io_context *ioc = icq->ioc;
-
- spin_lock_irqsave(&ioc->lock, flags);
- if (icq->flags & ICQ_DESTROYED) {
- spin_unlock_irqrestore(&ioc->lock, flags);
- continue;
- }
- ioc_destroy_icq(icq);
- spin_unlock_irqrestore(&ioc->lock, flags);
- }
- rcu_read_unlock();
-}
-
/**
* ioc_clear_queue - break any ioc association with the specified queue
* @q: request_queue being cleared
@@ -227,7 +206,17 @@ void ioc_clear_queue(struct request_queue *q)
list_splice_init(&q->icq_list, &icq_list);
spin_unlock_irq(&q->queue_lock);
- __ioc_clear_queue(&icq_list);
+ rcu_read_lock();
+ while (!list_empty(&icq_list)) {
+ struct io_cq *icq =
+ list_entry(icq_list.next, struct io_cq, q_node);
+
+ spin_lock_irq(&icq->ioc->lock);
+ if (!(icq->flags & ICQ_DESTROYED))
+ ioc_destroy_icq(icq);
+ spin_unlock_irq(&icq->ioc->lock);
+ }
+ rcu_read_unlock();
}
static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (5 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-14 15:52 ` Jan Kara
2021-12-09 6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
` (4 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Keep set_task_ioprio with the other low-level code that accesses the
io_context structure.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-ioc.c | 34 ++++++++++++++++++++++++++++++++--
block/ioprio.c | 32 --------------------------------
include/linux/iocontext.h | 2 --
3 files changed, 32 insertions(+), 36 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index f98a29ee8f362..c25ce2f3eb191 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -8,6 +8,7 @@
#include <linux/bio.h>
#include <linux/blkdev.h>
#include <linux/slab.h>
+#include <linux/security.h>
#include <linux/sched/task.h>
#include "blk.h"
@@ -280,8 +281,8 @@ static struct io_context *create_task_io_context(struct task_struct *task,
* This function always goes through task_lock() and it's better to use
* %current->io_context + get_io_context() for %current.
*/
-struct io_context *get_task_io_context(struct task_struct *task,
- gfp_t gfp_flags, int node)
+static struct io_context *get_task_io_context(struct task_struct *task,
+ gfp_t gfp_flags, int node)
{
struct io_context *ioc;
@@ -298,6 +299,35 @@ struct io_context *get_task_io_context(struct task_struct *task,
return ioc;
}
+int set_task_ioprio(struct task_struct *task, int ioprio)
+{
+ int err;
+ struct io_context *ioc;
+ const struct cred *cred = current_cred(), *tcred;
+
+ rcu_read_lock();
+ tcred = __task_cred(task);
+ if (!uid_eq(tcred->uid, cred->euid) &&
+ !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
+ rcu_read_unlock();
+ return -EPERM;
+ }
+ rcu_read_unlock();
+
+ err = security_task_setioprio(task, ioprio);
+ if (err)
+ return err;
+
+ ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
+ if (ioc) {
+ ioc->ioprio = ioprio;
+ put_io_context(ioc);
+ }
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(set_task_ioprio);
+
int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
{
struct io_context *ioc = current->io_context;
diff --git a/block/ioprio.c b/block/ioprio.c
index 313c14a70bbd3..e118f4bf2dc65 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -22,46 +22,14 @@
*/
#include <linux/gfp.h>
#include <linux/kernel.h>
-#include <linux/export.h>
#include <linux/ioprio.h>
#include <linux/cred.h>
#include <linux/blkdev.h>
#include <linux/capability.h>
-#include <linux/sched/user.h>
-#include <linux/sched/task.h>
#include <linux/syscalls.h>
#include <linux/security.h>
#include <linux/pid_namespace.h>
-int set_task_ioprio(struct task_struct *task, int ioprio)
-{
- int err;
- struct io_context *ioc;
- const struct cred *cred = current_cred(), *tcred;
-
- rcu_read_lock();
- tcred = __task_cred(task);
- if (!uid_eq(tcred->uid, cred->euid) &&
- !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
- rcu_read_unlock();
- return -EPERM;
- }
- rcu_read_unlock();
-
- err = security_task_setioprio(task, ioprio);
- if (err)
- return err;
-
- ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
- if (ioc) {
- ioc->ioprio = ioprio;
- put_io_context(ioc);
- }
-
- return err;
-}
-EXPORT_SYMBOL_GPL(set_task_ioprio);
-
int ioprio_check_cap(int ioprio)
{
int class = IOPRIO_PRIO_CLASS(ioprio);
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index 82c7f4f5f4f59..648331f35fc66 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -116,8 +116,6 @@ struct task_struct;
#ifdef CONFIG_BLOCK
void put_io_context(struct io_context *ioc);
void exit_io_context(struct task_struct *task);
-struct io_context *get_task_io_context(struct task_struct *task,
- gfp_t gfp_flags, int node);
int __copy_io(unsigned long clone_flags, struct task_struct *tsk);
static inline int copy_io(unsigned long clone_flags, struct task_struct *tsk)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (6 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-14 15:52 ` Jan Kara
2021-12-09 6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
` (3 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Fold get_task_io_context into its only caller, and simplify the code
as no reference to the I/O context is required to just set the ioprio
field.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-ioc.c | 52 +++++++++++++------------------------------------
1 file changed, 14 insertions(+), 38 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index c25ce2f3eb191..1ba7cfedca2d9 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -268,41 +268,9 @@ static struct io_context *create_task_io_context(struct task_struct *task,
return ioc;
}
-/**
- * get_task_io_context - get io_context of a task
- * @task: task of interest
- * @gfp_flags: allocation flags, used if allocation is necessary
- * @node: allocation node, used if allocation is necessary
- *
- * Return io_context of @task. If it doesn't exist, it is created with
- * @gfp_flags and @node. The returned io_context has its reference count
- * incremented.
- *
- * This function always goes through task_lock() and it's better to use
- * %current->io_context + get_io_context() for %current.
- */
-static struct io_context *get_task_io_context(struct task_struct *task,
- gfp_t gfp_flags, int node)
-{
- struct io_context *ioc;
-
- might_sleep_if(gfpflags_allow_blocking(gfp_flags));
-
- task_lock(task);
- ioc = task->io_context;
- if (unlikely(!ioc)) {
- task_unlock(task);
- return create_task_io_context(task, gfp_flags, node);
- }
- get_io_context(ioc);
- task_unlock(task);
- return ioc;
-}
-
int set_task_ioprio(struct task_struct *task, int ioprio)
{
int err;
- struct io_context *ioc;
const struct cred *cred = current_cred(), *tcred;
rcu_read_lock();
@@ -318,13 +286,21 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
if (err)
return err;
- ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
- if (ioc) {
- ioc->ioprio = ioprio;
- put_io_context(ioc);
- }
+ task_lock(task);
+ if (unlikely(!task->io_context)) {
+ struct io_context *ioc;
- return err;
+ task_unlock(task);
+ ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
+ if (ioc) {
+ ioc->ioprio = ioprio;
+ put_io_context(ioc);
+ }
+ return 0;
+ }
+ task->io_context->ioprio = ioprio;
+ task_unlock(task);
+ return 0;
}
EXPORT_SYMBOL_GPL(set_task_ioprio);
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (7 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-14 15:56 ` Jan Kara
2021-12-09 6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
` (2 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
The flow in set_task_ioprio can be simplified by simply open coding
create_task_io_context, which removes a refcount roundtrip on the I/O
context.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-ioc.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 1ba7cfedca2d9..cff0e3bdae53c 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -291,12 +291,18 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
struct io_context *ioc;
task_unlock(task);
- ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
- if (ioc) {
- ioc->ioprio = ioprio;
- put_io_context(ioc);
+
+ ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
+ if (!ioc)
+ return -ENOMEM;
+
+ task_lock(task);
+ if (task->io_context || (task->flags & PF_EXITING)) {
+ kmem_cache_free(iocontext_cachep, ioc);
+ ioc = task->io_context;
+ } else {
+ task->io_context = ioc;
}
- return 0;
}
task->io_context->ioprio = ioprio;
task_unlock(task);
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (8 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-14 16:01 ` Jan Kara
2021-12-09 6:31 ` [PATCH 11/11] block: only build the icq tracking code when needed Christoph Hellwig
2021-12-16 18:02 ` more I/O context cleanup v2 Jens Axboe
11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Fold create_task_io_context into the only remaining caller.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-ioc.c | 43 ++++++++++++-------------------------------
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index cff0e3bdae53c..dc7fb064fd5f7 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -238,36 +238,6 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
return ioc;
}
-static struct io_context *create_task_io_context(struct task_struct *task,
- gfp_t gfp_flags, int node)
-{
- struct io_context *ioc;
-
- ioc = alloc_io_context(gfp_flags, node);
- if (!ioc)
- return NULL;
-
- /*
- * Try to install. ioc shouldn't be installed if someone else
- * already did or @task, which isn't %current, is exiting. Note
- * that we need to allow ioc creation on exiting %current as exit
- * path may issue IOs from e.g. exit_files(). The exit path is
- * responsible for not issuing IO after exit_io_context().
- */
- task_lock(task);
- if (!task->io_context &&
- (task == current || !(task->flags & PF_EXITING)))
- task->io_context = ioc;
- else
- kmem_cache_free(iocontext_cachep, ioc);
-
- ioc = task->io_context;
- if (ioc)
- get_io_context(ioc);
- task_unlock(task);
- return ioc;
-}
-
int set_task_ioprio(struct task_struct *task, int ioprio)
{
int err;
@@ -426,9 +396,20 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
struct io_cq *icq = NULL;
if (unlikely(!ioc)) {
- ioc = create_task_io_context(current, GFP_ATOMIC, q->node);
+ ioc = alloc_io_context(GFP_ATOMIC, q->node);
if (!ioc)
return NULL;
+
+ task_lock(current);
+ if (current->io_context) {
+ kmem_cache_free(iocontext_cachep, ioc);
+ ioc = current->io_context;
+ } else {
+ current->io_context = ioc;
+ }
+
+ get_io_context(ioc);
+ task_unlock(current);
} else {
get_io_context(ioc);
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 11/11] block: only build the icq tracking code when needed
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (9 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
@ 2021-12-09 6:31 ` Christoph Hellwig
2021-12-16 18:02 ` more I/O context cleanup v2 Jens Axboe
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09 6:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block
Only bfq needs to code to track icq, so make it conditional.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
block/Kconfig | 3 ++
block/Kconfig.iosched | 1 +
block/blk-ioc.c | 68 +++++++++++++++++++++++----------------
block/blk.h | 6 ++++
include/linux/iocontext.h | 6 ++--
5 files changed, 55 insertions(+), 29 deletions(-)
diff --git a/block/Kconfig b/block/Kconfig
index c6ce41a5e5b27..d5d4197b7ed2d 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -35,6 +35,9 @@ config BLK_CGROUP_RWSTAT
config BLK_DEV_BSG_COMMON
tristate
+config BLK_ICQ
+ bool
+
config BLK_DEV_BSGLIB
bool "Block layer SG support v4 helper lib"
select BLK_DEV_BSG_COMMON
diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched
index 885fee86dfcae..6155161460862 100644
--- a/block/Kconfig.iosched
+++ b/block/Kconfig.iosched
@@ -18,6 +18,7 @@ config MQ_IOSCHED_KYBER
config IOSCHED_BFQ
tristate "BFQ I/O scheduler"
+ select BLK_ICQ
help
BFQ I/O scheduler for BLK-MQ. BFQ distributes the bandwidth of
of the device among all processes according to their weights,
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index dc7fb064fd5f7..87bdc9ca82959 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -19,6 +19,7 @@
*/
static struct kmem_cache *iocontext_cachep;
+#ifdef CONFIG_BLK_ICQ
/**
* get_io_context - increment reference count to io_context
* @ioc: io_context to get
@@ -162,6 +163,42 @@ static bool ioc_delay_free(struct io_context *ioc)
return false;
}
+/**
+ * ioc_clear_queue - break any ioc association with the specified queue
+ * @q: request_queue being cleared
+ *
+ * Walk @q->icq_list and exit all io_cq's.
+ */
+void ioc_clear_queue(struct request_queue *q)
+{
+ LIST_HEAD(icq_list);
+
+ spin_lock_irq(&q->queue_lock);
+ list_splice_init(&q->icq_list, &icq_list);
+ spin_unlock_irq(&q->queue_lock);
+
+ rcu_read_lock();
+ while (!list_empty(&icq_list)) {
+ struct io_cq *icq =
+ list_entry(icq_list.next, struct io_cq, q_node);
+
+ spin_lock_irq(&icq->ioc->lock);
+ if (!(icq->flags & ICQ_DESTROYED))
+ ioc_destroy_icq(icq);
+ spin_unlock_irq(&icq->ioc->lock);
+ }
+ rcu_read_unlock();
+}
+#else /* CONFIG_BLK_ICQ */
+static inline void ioc_exit_icqs(struct io_context *ioc)
+{
+}
+static inline bool ioc_delay_free(struct io_context *ioc)
+{
+ return false;
+}
+#endif /* CONFIG_BLK_ICQ */
+
/**
* put_io_context - put a reference of io_context
* @ioc: io_context to put
@@ -193,33 +230,6 @@ void exit_io_context(struct task_struct *task)
}
}
-/**
- * ioc_clear_queue - break any ioc association with the specified queue
- * @q: request_queue being cleared
- *
- * Walk @q->icq_list and exit all io_cq's.
- */
-void ioc_clear_queue(struct request_queue *q)
-{
- LIST_HEAD(icq_list);
-
- spin_lock_irq(&q->queue_lock);
- list_splice_init(&q->icq_list, &icq_list);
- spin_unlock_irq(&q->queue_lock);
-
- rcu_read_lock();
- while (!list_empty(&icq_list)) {
- struct io_cq *icq =
- list_entry(icq_list.next, struct io_cq, q_node);
-
- spin_lock_irq(&icq->ioc->lock);
- if (!(icq->flags & ICQ_DESTROYED))
- ioc_destroy_icq(icq);
- spin_unlock_irq(&icq->ioc->lock);
- }
- rcu_read_unlock();
-}
-
static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
{
struct io_context *ioc;
@@ -231,10 +241,12 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
atomic_long_set(&ioc->refcount, 1);
atomic_set(&ioc->active_ref, 1);
+#ifdef CONFIG_BLK_ICQ
spin_lock_init(&ioc->lock);
INIT_RADIX_TREE(&ioc->icq_tree, GFP_ATOMIC);
INIT_HLIST_HEAD(&ioc->icq_list);
INIT_WORK(&ioc->release_work, ioc_release_fn);
+#endif
return ioc;
}
@@ -300,6 +312,7 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
return 0;
}
+#ifdef CONFIG_BLK_ICQ
/**
* ioc_lookup_icq - lookup io_cq from ioc
* @q: the associated request_queue
@@ -428,6 +441,7 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
return icq;
}
EXPORT_SYMBOL_GPL(ioc_find_get_icq);
+#endif /* CONFIG_BLK_ICQ */
static int __init blk_ioc_init(void)
{
diff --git a/block/blk.h b/block/blk.h
index 7ccb7c7d86b38..8bd43b3ad33d5 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -366,7 +366,13 @@ static inline unsigned int bio_aligned_discard_max_sectors(
*/
struct io_cq *ioc_find_get_icq(struct request_queue *q);
struct io_cq *ioc_lookup_icq(struct request_queue *q);
+#ifdef CONFIG_BLK_ICQ
void ioc_clear_queue(struct request_queue *q);
+#else
+static inline void ioc_clear_queue(struct request_queue *q)
+{
+}
+#endif /* CONFIG_BLK_ICQ */
#ifdef CONFIG_BLK_DEV_THROTTLING_LOW
extern ssize_t blk_throtl_sample_time_show(struct request_queue *q, char *page);
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index 648331f35fc66..14f7eaf1b4437 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -100,16 +100,18 @@ struct io_context {
atomic_long_t refcount;
atomic_t active_ref;
+ unsigned short ioprio;
+
+#ifdef CONFIG_BLK_ICQ
/* all the fields below are protected by this lock */
spinlock_t lock;
- unsigned short ioprio;
-
struct radix_tree_root icq_tree;
struct io_cq __rcu *icq_hint;
struct hlist_head icq_list;
struct work_struct release_work;
+#endif /* CONFIG_BLK_ICQ */
};
struct task_struct;
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 06/11] block: cleanup ioc_clear_queue
2021-12-09 6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
@ 2021-12-14 15:50 ` Jan Kara
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block
On Thu 09-12-21 07:31:26, Christoph Hellwig wrote:
> Fold __ioc_clear_queue into ioc_clear_queue and switch to always
> use plain _irq locking instead of the more expensive _irqsave that
> is not needed here.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/blk-ioc.c | 33 +++++++++++----------------------
> 1 file changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index ca996214c10a6..f98a29ee8f362 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -192,27 +192,6 @@ void exit_io_context(struct task_struct *task)
> }
> }
>
> -static void __ioc_clear_queue(struct list_head *icq_list)
> -{
> - unsigned long flags;
> -
> - rcu_read_lock();
> - while (!list_empty(icq_list)) {
> - struct io_cq *icq = list_entry(icq_list->next,
> - struct io_cq, q_node);
> - struct io_context *ioc = icq->ioc;
> -
> - spin_lock_irqsave(&ioc->lock, flags);
> - if (icq->flags & ICQ_DESTROYED) {
> - spin_unlock_irqrestore(&ioc->lock, flags);
> - continue;
> - }
> - ioc_destroy_icq(icq);
> - spin_unlock_irqrestore(&ioc->lock, flags);
> - }
> - rcu_read_unlock();
> -}
> -
> /**
> * ioc_clear_queue - break any ioc association with the specified queue
> * @q: request_queue being cleared
> @@ -227,7 +206,17 @@ void ioc_clear_queue(struct request_queue *q)
> list_splice_init(&q->icq_list, &icq_list);
> spin_unlock_irq(&q->queue_lock);
>
> - __ioc_clear_queue(&icq_list);
> + rcu_read_lock();
> + while (!list_empty(&icq_list)) {
> + struct io_cq *icq =
> + list_entry(icq_list.next, struct io_cq, q_node);
> +
> + spin_lock_irq(&icq->ioc->lock);
> + if (!(icq->flags & ICQ_DESTROYED))
> + ioc_destroy_icq(icq);
> + spin_unlock_irq(&icq->ioc->lock);
> + }
> + rcu_read_unlock();
> }
>
> static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
> --
> 2.30.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c
2021-12-09 6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
@ 2021-12-14 15:52 ` Jan Kara
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:52 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block
On Thu 09-12-21 07:31:27, Christoph Hellwig wrote:
> Keep set_task_ioprio with the other low-level code that accesses the
> io_context structure.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/blk-ioc.c | 34 ++++++++++++++++++++++++++++++++--
> block/ioprio.c | 32 --------------------------------
> include/linux/iocontext.h | 2 --
> 3 files changed, 32 insertions(+), 36 deletions(-)
>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index f98a29ee8f362..c25ce2f3eb191 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -8,6 +8,7 @@
> #include <linux/bio.h>
> #include <linux/blkdev.h>
> #include <linux/slab.h>
> +#include <linux/security.h>
> #include <linux/sched/task.h>
>
> #include "blk.h"
> @@ -280,8 +281,8 @@ static struct io_context *create_task_io_context(struct task_struct *task,
> * This function always goes through task_lock() and it's better to use
> * %current->io_context + get_io_context() for %current.
> */
> -struct io_context *get_task_io_context(struct task_struct *task,
> - gfp_t gfp_flags, int node)
> +static struct io_context *get_task_io_context(struct task_struct *task,
> + gfp_t gfp_flags, int node)
> {
> struct io_context *ioc;
>
> @@ -298,6 +299,35 @@ struct io_context *get_task_io_context(struct task_struct *task,
> return ioc;
> }
>
> +int set_task_ioprio(struct task_struct *task, int ioprio)
> +{
> + int err;
> + struct io_context *ioc;
> + const struct cred *cred = current_cred(), *tcred;
> +
> + rcu_read_lock();
> + tcred = __task_cred(task);
> + if (!uid_eq(tcred->uid, cred->euid) &&
> + !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
> + rcu_read_unlock();
> + return -EPERM;
> + }
> + rcu_read_unlock();
> +
> + err = security_task_setioprio(task, ioprio);
> + if (err)
> + return err;
> +
> + ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> + if (ioc) {
> + ioc->ioprio = ioprio;
> + put_io_context(ioc);
> + }
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(set_task_ioprio);
> +
> int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
> {
> struct io_context *ioc = current->io_context;
> diff --git a/block/ioprio.c b/block/ioprio.c
> index 313c14a70bbd3..e118f4bf2dc65 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -22,46 +22,14 @@
> */
> #include <linux/gfp.h>
> #include <linux/kernel.h>
> -#include <linux/export.h>
> #include <linux/ioprio.h>
> #include <linux/cred.h>
> #include <linux/blkdev.h>
> #include <linux/capability.h>
> -#include <linux/sched/user.h>
> -#include <linux/sched/task.h>
> #include <linux/syscalls.h>
> #include <linux/security.h>
> #include <linux/pid_namespace.h>
>
> -int set_task_ioprio(struct task_struct *task, int ioprio)
> -{
> - int err;
> - struct io_context *ioc;
> - const struct cred *cred = current_cred(), *tcred;
> -
> - rcu_read_lock();
> - tcred = __task_cred(task);
> - if (!uid_eq(tcred->uid, cred->euid) &&
> - !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
> - rcu_read_unlock();
> - return -EPERM;
> - }
> - rcu_read_unlock();
> -
> - err = security_task_setioprio(task, ioprio);
> - if (err)
> - return err;
> -
> - ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> - if (ioc) {
> - ioc->ioprio = ioprio;
> - put_io_context(ioc);
> - }
> -
> - return err;
> -}
> -EXPORT_SYMBOL_GPL(set_task_ioprio);
> -
> int ioprio_check_cap(int ioprio)
> {
> int class = IOPRIO_PRIO_CLASS(ioprio);
> diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
> index 82c7f4f5f4f59..648331f35fc66 100644
> --- a/include/linux/iocontext.h
> +++ b/include/linux/iocontext.h
> @@ -116,8 +116,6 @@ struct task_struct;
> #ifdef CONFIG_BLOCK
> void put_io_context(struct io_context *ioc);
> void exit_io_context(struct task_struct *task);
> -struct io_context *get_task_io_context(struct task_struct *task,
> - gfp_t gfp_flags, int node);
> int __copy_io(unsigned long clone_flags, struct task_struct *tsk);
> static inline int copy_io(unsigned long clone_flags, struct task_struct *tsk)
> {
> --
> 2.30.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio
2021-12-09 6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
@ 2021-12-14 15:52 ` Jan Kara
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:52 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block
On Thu 09-12-21 07:31:28, Christoph Hellwig wrote:
> Fold get_task_io_context into its only caller, and simplify the code
> as no reference to the I/O context is required to just set the ioprio
> field.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/blk-ioc.c | 52 +++++++++++++------------------------------------
> 1 file changed, 14 insertions(+), 38 deletions(-)
>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index c25ce2f3eb191..1ba7cfedca2d9 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -268,41 +268,9 @@ static struct io_context *create_task_io_context(struct task_struct *task,
> return ioc;
> }
>
> -/**
> - * get_task_io_context - get io_context of a task
> - * @task: task of interest
> - * @gfp_flags: allocation flags, used if allocation is necessary
> - * @node: allocation node, used if allocation is necessary
> - *
> - * Return io_context of @task. If it doesn't exist, it is created with
> - * @gfp_flags and @node. The returned io_context has its reference count
> - * incremented.
> - *
> - * This function always goes through task_lock() and it's better to use
> - * %current->io_context + get_io_context() for %current.
> - */
> -static struct io_context *get_task_io_context(struct task_struct *task,
> - gfp_t gfp_flags, int node)
> -{
> - struct io_context *ioc;
> -
> - might_sleep_if(gfpflags_allow_blocking(gfp_flags));
> -
> - task_lock(task);
> - ioc = task->io_context;
> - if (unlikely(!ioc)) {
> - task_unlock(task);
> - return create_task_io_context(task, gfp_flags, node);
> - }
> - get_io_context(ioc);
> - task_unlock(task);
> - return ioc;
> -}
> -
> int set_task_ioprio(struct task_struct *task, int ioprio)
> {
> int err;
> - struct io_context *ioc;
> const struct cred *cred = current_cred(), *tcred;
>
> rcu_read_lock();
> @@ -318,13 +286,21 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
> if (err)
> return err;
>
> - ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> - if (ioc) {
> - ioc->ioprio = ioprio;
> - put_io_context(ioc);
> - }
> + task_lock(task);
> + if (unlikely(!task->io_context)) {
> + struct io_context *ioc;
>
> - return err;
> + task_unlock(task);
> + ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> + if (ioc) {
> + ioc->ioprio = ioprio;
> + put_io_context(ioc);
> + }
> + return 0;
> + }
> + task->io_context->ioprio = ioprio;
> + task_unlock(task);
> + return 0;
> }
> EXPORT_SYMBOL_GPL(set_task_ioprio);
>
> --
> 2.30.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio
2021-12-09 6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
@ 2021-12-14 15:56 ` Jan Kara
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:56 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block
On Thu 09-12-21 07:31:29, Christoph Hellwig wrote:
> The flow in set_task_ioprio can be simplified by simply open coding
> create_task_io_context, which removes a refcount roundtrip on the I/O
> context.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
OK, why not :). Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/blk-ioc.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index 1ba7cfedca2d9..cff0e3bdae53c 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -291,12 +291,18 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
> struct io_context *ioc;
>
> task_unlock(task);
> - ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> - if (ioc) {
> - ioc->ioprio = ioprio;
> - put_io_context(ioc);
> +
> + ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
> + if (!ioc)
> + return -ENOMEM;
> +
> + task_lock(task);
> + if (task->io_context || (task->flags & PF_EXITING)) {
> + kmem_cache_free(iocontext_cachep, ioc);
> + ioc = task->io_context;
> + } else {
> + task->io_context = ioc;
> }
> - return 0;
> }
> task->io_context->ioprio = ioprio;
> task_unlock(task);
> --
> 2.30.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq
2021-12-09 6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
@ 2021-12-14 16:01 ` Jan Kara
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 16:01 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block
On Thu 09-12-21 07:31:30, Christoph Hellwig wrote:
> Fold create_task_io_context into the only remaining caller.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/blk-ioc.c | 43 ++++++++++++-------------------------------
> 1 file changed, 12 insertions(+), 31 deletions(-)
>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index cff0e3bdae53c..dc7fb064fd5f7 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -238,36 +238,6 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
> return ioc;
> }
>
> -static struct io_context *create_task_io_context(struct task_struct *task,
> - gfp_t gfp_flags, int node)
> -{
> - struct io_context *ioc;
> -
> - ioc = alloc_io_context(gfp_flags, node);
> - if (!ioc)
> - return NULL;
> -
> - /*
> - * Try to install. ioc shouldn't be installed if someone else
> - * already did or @task, which isn't %current, is exiting. Note
> - * that we need to allow ioc creation on exiting %current as exit
> - * path may issue IOs from e.g. exit_files(). The exit path is
> - * responsible for not issuing IO after exit_io_context().
> - */
> - task_lock(task);
> - if (!task->io_context &&
> - (task == current || !(task->flags & PF_EXITING)))
> - task->io_context = ioc;
> - else
> - kmem_cache_free(iocontext_cachep, ioc);
> -
> - ioc = task->io_context;
> - if (ioc)
> - get_io_context(ioc);
> - task_unlock(task);
> - return ioc;
> -}
> -
> int set_task_ioprio(struct task_struct *task, int ioprio)
> {
> int err;
> @@ -426,9 +396,20 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
> struct io_cq *icq = NULL;
>
> if (unlikely(!ioc)) {
> - ioc = create_task_io_context(current, GFP_ATOMIC, q->node);
> + ioc = alloc_io_context(GFP_ATOMIC, q->node);
> if (!ioc)
> return NULL;
> +
> + task_lock(current);
> + if (current->io_context) {
> + kmem_cache_free(iocontext_cachep, ioc);
> + ioc = current->io_context;
> + } else {
> + current->io_context = ioc;
> + }
> +
> + get_io_context(ioc);
> + task_unlock(current);
> } else {
> get_io_context(ioc);
>
> --
> 2.30.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: more I/O context cleanup v2
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
` (10 preceding siblings ...)
2021-12-09 6:31 ` [PATCH 11/11] block: only build the icq tracking code when needed Christoph Hellwig
@ 2021-12-16 18:02 ` Jens Axboe
11 siblings, 0 replies; 18+ messages in thread
From: Jens Axboe @ 2021-12-16 18:02 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-block, Jan Kara, Paolo Valente
On Thu, 9 Dec 2021 07:31:20 +0100, Christoph Hellwig wrote:
> this series dusts off a few more bits in the I/O context handling, and
> makes a chunk of code only needed by the BFQ I/O scheduler optional.
>
> Changes since v1:
> - fix a comment typo
> - keep the RCU critical section in ioc_clear_queue
> - add a few more cleanups
>
> [...]
Applied, thanks!
[01/11] block: remove the nr_task field from struct io_context
commit: 8a2ba1785c5803d59a63b6320ff54fd4a37a41ce
[02/11] block: simplify struct io_context refcounting
commit: 0aed2f162bbc7853fe91c0d70492ea73c4e9cb07
[03/11] block: refactor put_iocontext_active
commit: 4be8a2eaff2e4473b6e8ad9a3857bc9b1e79c8ba
[04/11] block: remove the NULL ioc check in put_io_context
commit: 8a20c0c7e0cea7eb0c32fd6b63ff514c9ac32b8f
[05/11] block: refactor put_io_context
commit: edf70ff5a1ed9769da35178454d743828061a6a3
[06/11] block: cleanup ioc_clear_queue
commit: 091abcb3efd71cb18e80c8f040d9e4a634d8906d
[07/11] block: move set_task_ioprio to blk-ioc.c
commit: a411cd3cfdc5bbd1329d5b33dbf39e2b5213969d
[08/11] block: fold get_task_io_context into set_task_ioprio
commit: 8472161b77c41d260c5ba0af6bf940269b297bb6
[09/11] block: open code create_task_io_context in set_task_ioprio
commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7
[10/11] block: fold create_task_io_context into ioc_find_get_icq
commit: 90b627f5426ce144cdd4ea585d1f7812359a1a6a
[11/11] block: only build the icq tracking code when needed
commit: 5ef1630586317e92c9ebd7b4ce48f393b7ff790f
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-12-16 18:02 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 6:31 more I/O context cleanup v2 Christoph Hellwig
2021-12-09 6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
2021-12-09 6:31 ` [PATCH 02/11] block: simplify struct io_context refcounting Christoph Hellwig
2021-12-09 6:31 ` [PATCH 03/11] block: refactor put_iocontext_active Christoph Hellwig
2021-12-09 6:31 ` [PATCH 04/11] block: remove the NULL ioc check in put_io_context Christoph Hellwig
2021-12-09 6:31 ` [PATCH 05/11] block: refactor put_io_context Christoph Hellwig
2021-12-09 6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
2021-12-14 15:50 ` Jan Kara
2021-12-09 6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
2021-12-14 15:52 ` Jan Kara
2021-12-09 6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
2021-12-14 15:52 ` Jan Kara
2021-12-09 6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
2021-12-14 15:56 ` Jan Kara
2021-12-09 6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
2021-12-14 16:01 ` Jan Kara
2021-12-09 6:31 ` [PATCH 11/11] block: only build the icq tracking code when needed Christoph Hellwig
2021-12-16 18:02 ` more I/O context cleanup v2 Jens Axboe
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).