On 1/6/25 7:06 PM, Christoph Hellwig wrote:
> De-duplicate the code for updating queue limits by adding a store_limit
> method that allows having common code handle the actual queue limits
> update.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
[...]
> @@ -706,11 +687,24 @@ queue_attr_store(struct kobject *kobj, struct attribute
> *attr,
> if (entry->load_module)
> entry->load_module(disk, page, length);
>
> - blk_mq_freeze_queue(q);
> mutex_lock(&q->sysfs_lock);
> - res = entry->store(disk, page, length);
> - mutex_unlock(&q->sysfs_lock);
> + blk_mq_freeze_queue(q);
The freeze must NOT be done for the "if (entry->store_limit)" case. So this
needs to go int the "else". Otherwise, you still have freeze the take limit
lock order which can cause the ABBA deadlocks in SCSI sd.
> + if (entry->store_limit) {
> + struct queue_limits lim = queue_limits_start_update(q);
> +
> + res = entry->store_limit(disk, page, length, &lim);
> + if (res < 0) {
> + queue_limits_cancel_update(q);
> + } else {
> + res = queue_limits_commit_update(q, &lim);
Here you must use queue_limits_commit_update_frozen().
> + if (!res)
> + res = length;
> + }
> + } else {
> + res = entry->store(disk, page, length);
> + }
> blk_mq_unfreeze_queue(q);
And this one needs to go in the else after the call to entry->store().
> + mutex_unlock(&q->sysfs_lock);
> return res;
> }
>
--
Damien Le Moal
Western Digital Research
