Skip to content

Commit 2c2086a

Browse files
KAGA-KOKOaxboe
authored andcommitted
block: Protect less code with sysfs_lock in blk_{un,}register_queue()
The __blk_mq_register_dev(), blk_mq_unregister_dev(), elv_register_queue() and elv_unregister_queue() calls need to be protected with sysfs_lock but other code in these functions not. Hence protect only this code with sysfs_lock. This patch fixes a locking inversion issue in blk_unregister_queue() and also in an error path of blk_register_queue(): it is not allowed to hold sysfs_lock around the kobject_del(&q->kobj) call. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 14a2349 commit 2c2086a

File tree

1 file changed

+28
-9
lines changed

1 file changed

+28
-9
lines changed

block/blk-sysfs.c

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,10 @@ struct kobj_type blk_queue_ktype = {
853853
.release = blk_release_queue,
854854
};
855855

856+
/**
857+
* blk_register_queue - register a block layer queue with sysfs
858+
* @disk: Disk of which the request queue should be registered with sysfs.
859+
*/
856860
int blk_register_queue(struct gendisk *disk)
857861
{
858862
int ret;
@@ -909,11 +913,12 @@ int blk_register_queue(struct gendisk *disk)
909913
if (q->request_fn || (q->mq_ops && q->elevator)) {
910914
ret = elv_register_queue(q);
911915
if (ret) {
916+
mutex_unlock(&q->sysfs_lock);
912917
kobject_uevent(&q->kobj, KOBJ_REMOVE);
913918
kobject_del(&q->kobj);
914919
blk_trace_remove_sysfs(dev);
915920
kobject_put(&dev->kobj);
916-
goto unlock;
921+
return ret;
917922
}
918923
}
919924
ret = 0;
@@ -923,6 +928,13 @@ int blk_register_queue(struct gendisk *disk)
923928
}
924929
EXPORT_SYMBOL_GPL(blk_register_queue);
925930

931+
/**
932+
* blk_unregister_queue - counterpart of blk_register_queue()
933+
* @disk: Disk of which the request queue should be unregistered from sysfs.
934+
*
935+
* Note: the caller is responsible for guaranteeing that this function is called
936+
* after blk_register_queue() has finished.
937+
*/
926938
void blk_unregister_queue(struct gendisk *disk)
927939
{
928940
struct request_queue *q = disk->queue;
@@ -935,27 +947,34 @@ void blk_unregister_queue(struct gendisk *disk)
935947
return;
936948

937949
/*
938-
* Protect against the 'queue' kobj being accessed
939-
* while/after it is removed.
950+
* Since sysfs_remove_dir() prevents adding new directory entries
951+
* before removal of existing entries starts, protect against
952+
* concurrent elv_iosched_store() calls.
940953
*/
941954
mutex_lock(&q->sysfs_lock);
942955

943956
spin_lock_irq(q->queue_lock);
944957
queue_flag_clear(QUEUE_FLAG_REGISTERED, q);
945958
spin_unlock_irq(q->queue_lock);
946959

947-
wbt_exit(q);
948-
960+
/*
961+
* Remove the sysfs attributes before unregistering the queue data
962+
* structures that can be modified through sysfs.
963+
*/
949964
if (q->mq_ops)
950965
blk_mq_unregister_dev(disk_to_dev(disk), q);
951-
952-
if (q->request_fn || (q->mq_ops && q->elevator))
953-
elv_unregister_queue(q);
966+
mutex_unlock(&q->sysfs_lock);
954967

955968
kobject_uevent(&q->kobj, KOBJ_REMOVE);
956969
kobject_del(&q->kobj);
957970
blk_trace_remove_sysfs(disk_to_dev(disk));
958-
kobject_put(&disk_to_dev(disk)->kobj);
959971

972+
wbt_exit(q);
973+
974+
mutex_lock(&q->sysfs_lock);
975+
if (q->request_fn || (q->mq_ops && q->elevator))
976+
elv_unregister_queue(q);
960977
mutex_unlock(&q->sysfs_lock);
978+
979+
kobject_put(&disk_to_dev(disk)->kobj);
961980
}

0 commit comments

Comments
 (0)