iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

block: fix up elevator_type refcounting

Browse Source 
The current reference management logic of io scheduler modules contains
refcnt problems. For example, blk_mq_init_sched may fail before or after
the calling of e->ops.init_sched. If it fails before the calling, it does
nothing to the reference to the io scheduler module. But if it fails after
the calling, it releases the reference by calling kobject_put(&eq->kobj).

As the callers of blk_mq_init_sched can't know exactly where the failure
happens, they can't handle the reference to the io scheduler module
properly: releasing the reference on failure results in double-release if
blk_mq_init_sched has released it, and not releasing the reference results
in ghost reference if blk_mq_init_sched did not release it either.

The same problem also exists in io schedulers' init_sched implementations.

We can address the problem by adding releasing statements to the error
handling procedures of blk_mq_init_sched and init_sched implementations.
But that is counterintuitive and requires modifications to existing io
schedulers.

Instead, We make elevator_alloc get the io scheduler module references
that will be released by elevator_release. And then, we match each
elevator_get with an elevator_put. Therefore, each reference to an io
scheduler module explicitly has its own getter and releaser, and we no
longer need to worry about the refcnt problems.

The bugs and the patch can be validated with tools here:
https://github.com/nickyc975/linux_elv_refcnt_bug.git

[hch: split out a few bits into separate patches, use a non-try
      module_get in elevator_alloc]

Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20221020064819.1469928-5-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>
This commit is contained in:
Jinlong Chen 2022-10-20 08:48:19 +02:00 committed by Jens Axboe
parent b54c2ad9b7
commit 8ed40ee35d
3 changed files with 10 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
block/blk-mq-sched.c
View File

@ -555,6 +555,7 @@ static int blk_mq_init_sched_shared_tags(struct request_queue *queue)
return 0; return 0;
} }
/* caller must have a reference to @e, will grab another one if successful */
int blk_mq_init_sched(struct request_queue *q, struct elevator_type *e) int blk_mq_init_sched(struct request_queue *q, struct elevator_type *e)
{ {
unsigned int flags = q->tag_set->flags; unsigned int flags = q->tag_set->flags;

2
block/blk-mq.c
View File

@ -4591,6 +4591,8 @@ static void blk_mq_elv_switch_back(struct list_head *head,
mutex_lock(&q->sysfs_lock); mutex_lock(&q->sysfs_lock);
elevator_switch(q, t); elevator_switch(q, t);
/* drop the reference acquired in blk_mq_elv_switch_none */
elevator_put(t);
mutex_unlock(&q->sysfs_lock); mutex_unlock(&q->sysfs_lock);
} }

10
block/elevator.c
View File

@ -165,6 +165,7 @@ struct elevator_queue *elevator_alloc(struct request_queue *q,
if (unlikely(!eq)) if (unlikely(!eq))
return NULL; return NULL;
__elevator_get(e);
eq->type = e; eq->type = e;
kobject_init(&eq->kobj, &elv_ktype); kobject_init(&eq->kobj, &elv_ktype);
mutex_init(&eq->sysfs_lock); mutex_init(&eq->sysfs_lock);
@ -704,8 +705,9 @@ void elevator_init_mq(struct request_queue *q)
if (err) { if (err) {
pr_warn("\"%s\" elevator initialization failed, " pr_warn("\"%s\" elevator initialization failed, "
"falling back to \"none\"\n", e->elevator_name); "falling back to \"none\"\n", e->elevator_name);
elevator_put(e);
} }
elevator_put(e);
} }
/* /*
@ -737,6 +739,7 @@ int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
static int elevator_change(struct request_queue *q, const char *elevator_name) static int elevator_change(struct request_queue *q, const char *elevator_name)
{ {
struct elevator_type *e; struct elevator_type *e;
int ret;
/* Make sure queue is not in the middle of being removed */ /* Make sure queue is not in the middle of being removed */
if (!blk_queue_registered(q)) if (!blk_queue_registered(q))
@ -757,8 +760,9 @@ static int elevator_change(struct request_queue *q, const char *elevator_name)
e = elevator_get(q, elevator_name, true); e = elevator_get(q, elevator_name, true);
if (!e) if (!e)
return -EINVAL; return -EINVAL;
ret = elevator_switch(q, e);
return elevator_switch(q, e); elevator_put(e);
return ret;
} }
ssize_t elv_iosched_store(struct request_queue *q, const char *buf, ssize_t elv_iosched_store(struct request_queue *q, const char *buf,