On 2/25/25 1:23 PM, Hannes Reinecke wrote: > On 2/24/25 14:30, Nilay Shroff wrote: >> The wbt latency and state could be updated while initializing the >> elevator or exiting the elevator. It could be also updates 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. >> >> Signed-off-by: Nilay Shroff <nilay@xxxxxxxxxxxxx> >> --- >> block/blk-iocost.c | 2 ++ >> block/blk-sysfs.c | 20 ++++++++------------ >> 2 files changed, 10 insertions(+), 12 deletions(-) >> >> diff --git a/block/blk-iocost.c b/block/blk-iocost.c >> index 65a1d4427ccf..c68373361301 100644 >> --- a/block/blk-iocost.c >> +++ b/block/blk-iocost.c >> @@ -3249,6 +3249,7 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input, >> } >> memflags = blk_mq_freeze_queue(disk->queue); >> + mutex_lock(&disk->queue->elevator_lock); >> blk_mq_quiesce_queue(disk->queue); >> > Ordering? > > Do we have a defined order between 'freeze', 'quiesce' and taking the respective lock? > > Or, to put it the other way round: why do we tak the lock after the 'freeze', but before the 'quiesce'? This is the order followed currently in __blk_mq_update_nr_hw_queues() where we first freeze queue and then acquire the other locks. Yes this may look a bit unusual but that's how it is. If we need to change that ordering then we may require breaking the __blk_mq_update_nr_hw_queues() function. So the current locking sequence wrt ->elevator_lock is: freeze queue followed by acquiring ->elevator_lock. So we try follow the same locking order at all call sites where we need to freeze-queue and use ->elevator_lock. > > Isn't that worth a comment somewhere? Alright, I'd add comment about this ordering probably near the declaration of this lock in request_queue. Thanks, --Nilay
