Skip to content

Commit c99f66e

Browse files
Christoph Hellwigaxboe
authored andcommitted
block: fix queue freeze vs limits lock order in sysfs store methods
queue_attr_store() always freezes a device queue before calling the attribute store operation. For attributes that control queue limits, the store operation will also lock the queue limits with a call to queue_limits_start_update(). However, some drivers (e.g. SCSI sd) may need to issue commands to a device to obtain limit values from the hardware with the queue limits locked. This creates a potential ABBA deadlock situation if a user attempts to modify a limit (thus freezing the device queue) while the device driver starts a revalidation of the device queue limits. Avoid such deadlock by not freezing the queue before calling the ->store_limit() method in struct queue_sysfs_entry and instead use the queue_limits_commit_update_frozen helper to freeze the queue after taking the limits lock. This also removes taking the sysfs lock for the store_limit method as it doesn't protect anything here, but creates even more nesting. Hopefully it will go away from the actual sysfs methods entirely soon. (commit log adapted from a similar patch from Damien Le Moal) Fixes: ff956a3 ("block: use queue_limits_commit_update in queue_discard_max_store") Fixes: 0327ca9 ("block: use queue_limits_commit_update in queue_max_sectors_store") Signed-off-by: Christoph Hellwig <[email protected]> Reviewed-by: Ming Lei <[email protected]> Reviewed-by: Damien Le Moal <[email protected]> Reviewed-by: Martin K. Petersen <[email protected]> Reviewed-by: Nilay Shroff <[email protected]> Reviewed-by: Johannes Thumshirn <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]>
1 parent a162306 commit c99f66e

File tree

1 file changed

+10
-8
lines changed

1 file changed

+10
-8
lines changed

block/blk-sysfs.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -694,22 +694,24 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
694694
if (entry->load_module)
695695
entry->load_module(disk, page, length);
696696

697-
mutex_lock(&q->sysfs_lock);
698-
blk_mq_freeze_queue(q);
699697
if (entry->store_limit) {
700698
struct queue_limits lim = queue_limits_start_update(q);
701699

702700
res = entry->store_limit(disk, page, length, &lim);
703701
if (res < 0) {
704702
queue_limits_cancel_update(q);
705-
} else {
706-
res = queue_limits_commit_update(q, &lim);
707-
if (!res)
708-
res = length;
703+
return res;
709704
}
710-
} else {
711-
res = entry->store(disk, page, length);
705+
706+
res = queue_limits_commit_update_frozen(q, &lim);
707+
if (res)
708+
return res;
709+
return length;
712710
}
711+
712+
mutex_lock(&q->sysfs_lock);
713+
blk_mq_freeze_queue(q);
714+
res = entry->store(disk, page, length);
713715
blk_mq_unfreeze_queue(q);
714716
mutex_unlock(&q->sysfs_lock);
715717
return res;

0 commit comments

Comments
 (0)