From 6fbaea28a75796bbdd65259f4d56728ff59339b3 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 4 Mar 2021 16:13:09 +0100 Subject: [PATCH] Revert "block: split .sysfs_lock into two locks" This reverts commit fa137b50f3264a157575413030464c19ab553b0e. It breaks the abi and is not needed in Android devices. Bug: 161946584 Signed-off-by: Greg Kroah-Hartman Change-Id: Ia9b92ae9b03e3e12b13584373448160c767bf4cb --- block/blk-core.c | 1 - block/blk-mq-sysfs.c | 12 ++++----- block/blk-sysfs.c | 46 ++++++++++++-------------------- block/blk.h | 2 +- block/elevator.c | 59 +++++++----------------------------------- include/linux/blkdev.h | 1 - 6 files changed, 34 insertions(+), 87 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 7c85643dd99b..a33775cd97be 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1061,7 +1061,6 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id, mutex_init(&q->blk_trace_mutex); #endif mutex_init(&q->sysfs_lock); - mutex_init(&q->sysfs_dir_lock); spin_lock_init(&q->__queue_lock); if (!q->mq_ops) diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c index 5e4b7ed1e897..5006a0d00990 100644 --- a/block/blk-mq-sysfs.c +++ b/block/blk-mq-sysfs.c @@ -264,7 +264,7 @@ void blk_mq_unregister_dev(struct device *dev, struct request_queue *q) struct blk_mq_hw_ctx *hctx; int i; - lockdep_assert_held(&q->sysfs_dir_lock); + lockdep_assert_held(&q->sysfs_lock); queue_for_each_hw_ctx(q, hctx, i) blk_mq_unregister_hctx(hctx); @@ -312,7 +312,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q) int ret, i; WARN_ON_ONCE(!q->kobj.parent); - lockdep_assert_held(&q->sysfs_dir_lock); + lockdep_assert_held(&q->sysfs_lock); ret = kobject_add(&q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq"); if (ret < 0) @@ -358,7 +358,7 @@ void blk_mq_sysfs_unregister(struct request_queue *q) struct blk_mq_hw_ctx *hctx; int i; - mutex_lock(&q->sysfs_dir_lock); + mutex_lock(&q->sysfs_lock); if (!q->mq_sysfs_init_done) goto unlock; @@ -366,7 +366,7 @@ void blk_mq_sysfs_unregister(struct request_queue *q) blk_mq_unregister_hctx(hctx); unlock: - mutex_unlock(&q->sysfs_dir_lock); + mutex_unlock(&q->sysfs_lock); } int blk_mq_sysfs_register(struct request_queue *q) @@ -374,7 +374,7 @@ int blk_mq_sysfs_register(struct request_queue *q) struct blk_mq_hw_ctx *hctx; int i, ret = 0; - mutex_lock(&q->sysfs_dir_lock); + mutex_lock(&q->sysfs_lock); if (!q->mq_sysfs_init_done) goto unlock; @@ -385,7 +385,7 @@ int blk_mq_sysfs_register(struct request_queue *q) } unlock: - mutex_unlock(&q->sysfs_dir_lock); + mutex_unlock(&q->sysfs_lock); return ret; } diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index b2208b69f04a..0a7636d24563 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -892,7 +892,6 @@ int blk_register_queue(struct gendisk *disk) int ret; struct device *dev = disk_to_dev(disk); struct request_queue *q = disk->queue; - bool has_elevator = false; if (WARN_ON(!q)) return -ENXIO; @@ -900,6 +899,7 @@ int blk_register_queue(struct gendisk *disk) WARN_ONCE(blk_queue_registered(q), "%s is registering an already registered queue\n", kobject_name(&dev->kobj)); + queue_flag_set_unlocked(QUEUE_FLAG_REGISTERED, q); /* * SCSI probing may synchronously create and destroy a lot of @@ -920,7 +920,8 @@ int blk_register_queue(struct gendisk *disk) if (ret) return ret; - mutex_lock(&q->sysfs_dir_lock); + /* Prevent changes through sysfs until registration is completed. */ + mutex_lock(&q->sysfs_lock); ret = kobject_add(&q->kobj, kobject_get(&dev->kobj), "%s", "queue"); if (ret < 0) { @@ -933,36 +934,26 @@ int blk_register_queue(struct gendisk *disk) blk_mq_debugfs_register(q); } - /* - * The flag of QUEUE_FLAG_REGISTERED isn't set yet, so elevator - * switch won't happen at all. - */ + kobject_uevent(&q->kobj, KOBJ_ADD); + + wbt_enable_default(q); + + blk_throtl_register_queue(q); + if (q->request_fn || (q->mq_ops && q->elevator)) { - ret = elv_register_queue(q, false); + ret = elv_register_queue(q); if (ret) { - mutex_unlock(&q->sysfs_dir_lock); + mutex_unlock(&q->sysfs_lock); + kobject_uevent(&q->kobj, KOBJ_REMOVE); kobject_del(&q->kobj); blk_trace_remove_sysfs(dev); kobject_put(&dev->kobj); return ret; } - has_elevator = true; } - - mutex_lock(&q->sysfs_lock); - blk_queue_flag_set(QUEUE_FLAG_REGISTERED, q); - wbt_enable_default(q); - blk_throtl_register_queue(q); - - /* Now everything is ready and send out KOBJ_ADD uevent */ - kobject_uevent(&q->kobj, KOBJ_ADD); - if (has_elevator) - kobject_uevent(&q->elevator->kobj, KOBJ_ADD); - mutex_unlock(&q->sysfs_lock); - ret = 0; unlock: - mutex_unlock(&q->sysfs_dir_lock); + mutex_unlock(&q->sysfs_lock); return ret; } EXPORT_SYMBOL_GPL(blk_register_queue); @@ -977,7 +968,6 @@ EXPORT_SYMBOL_GPL(blk_register_queue); void blk_unregister_queue(struct gendisk *disk) { struct request_queue *q = disk->queue; - bool has_elevator; if (WARN_ON(!q)) return; @@ -992,27 +982,25 @@ void blk_unregister_queue(struct gendisk *disk) * concurrent elv_iosched_store() calls. */ mutex_lock(&q->sysfs_lock); - blk_queue_flag_clear(QUEUE_FLAG_REGISTERED, q); - has_elevator = !!q->elevator; - mutex_unlock(&q->sysfs_lock); - mutex_lock(&q->sysfs_dir_lock); + blk_queue_flag_clear(QUEUE_FLAG_REGISTERED, q); + /* * Remove the sysfs attributes before unregistering the queue data * structures that can be modified through sysfs. */ if (q->mq_ops) blk_mq_unregister_dev(disk_to_dev(disk), q); + mutex_unlock(&q->sysfs_lock); kobject_uevent(&q->kobj, KOBJ_REMOVE); kobject_del(&q->kobj); blk_trace_remove_sysfs(disk_to_dev(disk)); mutex_lock(&q->sysfs_lock); - if (q->request_fn || has_elevator) + if (q->request_fn || (q->mq_ops && q->elevator)) elv_unregister_queue(q); mutex_unlock(&q->sysfs_lock); - mutex_unlock(&q->sysfs_dir_lock); kobject_put(&disk_to_dev(disk)->kobj); } diff --git a/block/blk.h b/block/blk.h index ae87e2a5f2bd..1a5b67b57e6b 100644 --- a/block/blk.h +++ b/block/blk.h @@ -244,7 +244,7 @@ int elevator_init_mq(struct request_queue *q); int elevator_switch_mq(struct request_queue *q, struct elevator_type *new_e); void elevator_exit(struct request_queue *, struct elevator_queue *); -int elv_register_queue(struct request_queue *q, bool uevent); +int elv_register_queue(struct request_queue *q); void elv_unregister_queue(struct request_queue *q); struct hd_struct *__disk_get_part(struct gendisk *disk, int partno); diff --git a/block/elevator.c b/block/elevator.c index 2ff0859e8b35..9bffe4558929 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -833,16 +833,13 @@ static struct kobj_type elv_ktype = { .release = elevator_release, }; -/* - * elv_register_queue is called from either blk_register_queue or - * elevator_switch, elevator switch is prevented from being happen - * in the two paths, so it is safe to not hold q->sysfs_lock. - */ -int elv_register_queue(struct request_queue *q, bool uevent) +int elv_register_queue(struct request_queue *q) { struct elevator_queue *e = q->elevator; int error; + lockdep_assert_held(&q->sysfs_lock); + error = kobject_add(&e->kobj, &q->kobj, "%s", "iosched"); if (!error) { struct elv_fs_entry *attr = e->type->elevator_attrs; @@ -853,36 +850,26 @@ int elv_register_queue(struct request_queue *q, bool uevent) attr++; } } - if (uevent) - kobject_uevent(&e->kobj, KOBJ_ADD); - - mutex_lock(&q->sysfs_lock); + kobject_uevent(&e->kobj, KOBJ_ADD); e->registered = 1; if (!e->uses_mq && e->type->ops.sq.elevator_registered_fn) e->type->ops.sq.elevator_registered_fn(q); - mutex_unlock(&q->sysfs_lock); } return error; } -/* - * elv_unregister_queue is called from either blk_unregister_queue or - * elevator_switch, elevator switch is prevented from being happen - * in the two paths, so it is safe to not hold q->sysfs_lock. - */ void elv_unregister_queue(struct request_queue *q) { + lockdep_assert_held(&q->sysfs_lock); + if (q) { struct elevator_queue *e = q->elevator; kobject_uevent(&e->kobj, KOBJ_REMOVE); kobject_del(&e->kobj); - - mutex_lock(&q->sysfs_lock); e->registered = 0; /* Re-enable throttling in case elevator disabled it */ wbt_enable_default(q); - mutex_unlock(&q->sysfs_lock); } } @@ -953,32 +940,10 @@ int elevator_switch_mq(struct request_queue *q, lockdep_assert_held(&q->sysfs_lock); if (q->elevator) { - if (q->elevator->registered) { - mutex_unlock(&q->sysfs_lock); - - /* - * Concurrent elevator switch can't happen becasue - * sysfs write is always exclusively on same file. - * - * Also the elevator queue won't be freed after - * sysfs_lock is released becasue kobject_del() in - * blk_unregister_queue() waits for completion of - * .store & .show on its attributes. - */ + if (q->elevator->registered) elv_unregister_queue(q); - - mutex_lock(&q->sysfs_lock); - } ioc_clear_queue(q); elevator_exit(q, q->elevator); - - /* - * sysfs_lock may be dropped, so re-check if queue is - * unregistered. If yes, don't switch to new elevator - * any more - */ - if (!blk_queue_registered(q)) - return 0; } ret = blk_mq_init_sched(q, new_e); @@ -986,11 +951,7 @@ int elevator_switch_mq(struct request_queue *q, goto out; if (new_e) { - mutex_unlock(&q->sysfs_lock); - - ret = elv_register_queue(q, true); - - mutex_lock(&q->sysfs_lock); + ret = elv_register_queue(q); if (ret) { elevator_exit(q, q->elevator); goto out; @@ -1086,7 +1047,7 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) if (err) goto fail_init; - err = elv_register_queue(q, true); + err = elv_register_queue(q); if (err) goto fail_register; @@ -1106,7 +1067,7 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) /* switch failed, restore and re-register old elevator */ if (old) { q->elevator = old; - elv_register_queue(q, true); + elv_register_queue(q); blk_queue_bypass_end(q); } diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index d219a58b4746..b587a3bf1966 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -642,7 +642,6 @@ struct request_queue { struct delayed_work requeue_work; struct mutex sysfs_lock; - struct mutex sysfs_dir_lock; int bypass_depth; atomic_t mq_freeze_depth;