blk-iocost: prevent configuration update concurrent with io throttling

This won't cause any severe problem currently, however, this doesn't
seems appropriate:

1) 'ioc->params' is read from multiple places without holding
'ioc->lock', unexpected value might be read if writing it concurrently.

2) If configuration is changed while io is throttling, the functionality
might be affected. For example, if module params is updated and cost
becomes smaller, waiting for timer that is caculated under old
configuration is not appropriate.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/r/20221012094035.390056-4-yukuai1@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
This commit is contained in:
Yu Kuai 2022-10-12 17:40:34 +08:00 committed by Jens Axboe
parent 2c06479884
commit 2b2da2f6dc
1 changed files with 24 additions and 2 deletions

View File

@ -3187,6 +3187,9 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
ioc = q_to_ioc(disk->queue); ioc = q_to_ioc(disk->queue);
} }
blk_mq_freeze_queue(disk->queue);
blk_mq_quiesce_queue(disk->queue);
spin_lock_irq(&ioc->lock); spin_lock_irq(&ioc->lock);
memcpy(qos, ioc->params.qos, sizeof(qos)); memcpy(qos, ioc->params.qos, sizeof(qos));
enable = ioc->enabled; enable = ioc->enabled;
@ -3278,10 +3281,17 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
ioc_refresh_params(ioc, true); ioc_refresh_params(ioc, true);
spin_unlock_irq(&ioc->lock); spin_unlock_irq(&ioc->lock);
blk_mq_unquiesce_queue(disk->queue);
blk_mq_unfreeze_queue(disk->queue);
blkdev_put_no_open(bdev); blkdev_put_no_open(bdev);
return nbytes; return nbytes;
einval: einval:
spin_unlock_irq(&ioc->lock); spin_unlock_irq(&ioc->lock);
blk_mq_unquiesce_queue(disk->queue);
blk_mq_unfreeze_queue(disk->queue);
ret = -EINVAL; ret = -EINVAL;
err: err:
blkdev_put_no_open(bdev); blkdev_put_no_open(bdev);
@ -3336,6 +3346,7 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
size_t nbytes, loff_t off) size_t nbytes, loff_t off)
{ {
struct block_device *bdev; struct block_device *bdev;
struct request_queue *q;
struct ioc *ioc; struct ioc *ioc;
u64 u[NR_I_LCOEFS]; u64 u[NR_I_LCOEFS];
bool user; bool user;
@ -3346,14 +3357,18 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
if (IS_ERR(bdev)) if (IS_ERR(bdev))
return PTR_ERR(bdev); return PTR_ERR(bdev);
ioc = q_to_ioc(bdev_get_queue(bdev)); q = bdev_get_queue(bdev);
ioc = q_to_ioc(q);
if (!ioc) { if (!ioc) {
ret = blk_iocost_init(bdev->bd_disk); ret = blk_iocost_init(bdev->bd_disk);
if (ret) if (ret)
goto err; goto err;
ioc = q_to_ioc(bdev_get_queue(bdev)); ioc = q_to_ioc(q);
} }
blk_mq_freeze_queue(q);
blk_mq_quiesce_queue(q);
spin_lock_irq(&ioc->lock); spin_lock_irq(&ioc->lock);
memcpy(u, ioc->params.i_lcoefs, sizeof(u)); memcpy(u, ioc->params.i_lcoefs, sizeof(u));
user = ioc->user_cost_model; user = ioc->user_cost_model;
@ -3402,11 +3417,18 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
ioc_refresh_params(ioc, true); ioc_refresh_params(ioc, true);
spin_unlock_irq(&ioc->lock); spin_unlock_irq(&ioc->lock);
blk_mq_unquiesce_queue(q);
blk_mq_unfreeze_queue(q);
blkdev_put_no_open(bdev); blkdev_put_no_open(bdev);
return nbytes; return nbytes;
einval: einval:
spin_unlock_irq(&ioc->lock); spin_unlock_irq(&ioc->lock);
blk_mq_unquiesce_queue(q);
blk_mq_unfreeze_queue(q);
ret = -EINVAL; ret = -EINVAL;
err: err:
blkdev_put_no_open(bdev); blkdev_put_no_open(bdev);