Skip to content

Commit 245618f

Browse files
shroffniaxboe
authored andcommitted
block: protect wbt_lat_usec using q->elevator_lock
The wbt latency and state could be updated while initializing the elevator or exiting the elevator. It could be also updated while configuring IO latency QoS parameters using cgroup. The elevator code path is now protected with q->elevator_lock. So we should protect the access to sysfs attribute wbt_lat_usec using q->elevator _lock instead of q->sysfs_lock. White we're at it, also protect ioc_qos_write(), which configures wbt parameters via cgroup, using q->elevator_lock. Reviewed-by: Christoph Hellwig <[email protected]> Reviewed-by: Hannes Reinecke <[email protected]> Reviewed-by: Ming Lei <[email protected]> Signed-off-by: Nilay Shroff <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]>
1 parent 3efe757 commit 245618f

File tree

3 files changed

+12
-14
lines changed

3 files changed

+12
-14
lines changed

block/blk-iocost.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3248,6 +3248,7 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
32483248
}
32493249

32503250
memflags = blk_mq_freeze_queue(disk->queue);
3251+
mutex_lock(&disk->queue->elevator_lock);
32513252
blk_mq_quiesce_queue(disk->queue);
32523253

32533254
spin_lock_irq(&ioc->lock);
@@ -3355,6 +3356,7 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
33553356
spin_unlock_irq(&ioc->lock);
33563357

33573358
blk_mq_unquiesce_queue(disk->queue);
3359+
mutex_unlock(&disk->queue->elevator_lock);
33583360
blk_mq_unfreeze_queue(disk->queue, memflags);
33593361

33603362
ret = -EINVAL;

block/blk-sysfs.c

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ static ssize_t queue_wb_lat_show(struct gendisk *disk, char *page)
557557
ssize_t ret;
558558
struct request_queue *q = disk->queue;
559559

560-
mutex_lock(&q->sysfs_lock);
560+
mutex_lock(&q->elevator_lock);
561561
if (!wbt_rq_qos(q)) {
562562
ret = -EINVAL;
563563
goto out;
@@ -570,7 +570,7 @@ static ssize_t queue_wb_lat_show(struct gendisk *disk, char *page)
570570

571571
ret = sysfs_emit(page, "%llu\n", div_u64(wbt_get_min_lat(q), 1000));
572572
out:
573-
mutex_unlock(&q->sysfs_lock);
573+
mutex_unlock(&q->elevator_lock);
574574
return ret;
575575
}
576576

@@ -589,8 +589,8 @@ static ssize_t queue_wb_lat_store(struct gendisk *disk, const char *page,
589589
if (val < -1)
590590
return -EINVAL;
591591

592-
mutex_lock(&q->sysfs_lock);
593592
memflags = blk_mq_freeze_queue(q);
593+
mutex_lock(&q->elevator_lock);
594594

595595
rqos = wbt_rq_qos(q);
596596
if (!rqos) {
@@ -619,8 +619,8 @@ static ssize_t queue_wb_lat_store(struct gendisk *disk, const char *page,
619619

620620
blk_mq_unquiesce_queue(q);
621621
out:
622+
mutex_unlock(&q->elevator_lock);
622623
blk_mq_unfreeze_queue(q, memflags);
623-
mutex_unlock(&q->sysfs_lock);
624624

625625
return ret;
626626
}
@@ -689,19 +689,15 @@ static struct attribute *queue_attrs[] = {
689689

690690
/* Request-based queue attributes that are not relevant for bio-based queues. */
691691
static struct attribute *blk_mq_queue_attrs[] = {
692-
/*
693-
* Attributes which are protected with q->sysfs_lock.
694-
*/
695-
#ifdef CONFIG_BLK_WBT
696-
&queue_wb_lat_entry.attr,
697-
#endif
698692
/*
699693
* Attributes which require some form of locking other than
700694
* q->sysfs_lock.
701695
*/
702696
&elv_iosched_entry.attr,
703697
&queue_requests_entry.attr,
704-
698+
#ifdef CONFIG_BLK_WBT
699+
&queue_wb_lat_entry.attr,
700+
#endif
705701
/*
706702
* Attributes which don't require locking.
707703
*/
@@ -882,10 +878,10 @@ int blk_register_queue(struct gendisk *disk)
882878
goto out_crypto_sysfs_unregister;
883879
}
884880
}
881+
wbt_enable_default(disk);
885882
mutex_unlock(&q->elevator_lock);
886883

887884
blk_queue_flag_set(QUEUE_FLAG_REGISTERED, q);
888-
wbt_enable_default(disk);
889885

890886
/* Now everything is ready and send out KOBJ_ADD uevent */
891887
kobject_uevent(&disk->queue_kobj, KOBJ_ADD);

include/linux/blkdev.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,8 +563,8 @@ struct request_queue {
563563
/*
564564
* Protects against I/O scheduler switching, particularly when
565565
* updating q->elevator. Since the elevator update code path may
566-
* also modify q->nr_requests, this lock also protects the sysfs
567-
* attribute nr_requests.
566+
* also modify q->nr_requests and wbt latency, this lock also
567+
* protects the sysfs attributes nr_requests and wbt_lat_usec.
568568
* To ensure proper locking order during an elevator update, first
569569
* freeze the queue, then acquire ->elevator_lock.
570570
*/

0 commit comments

Comments
 (0)