Re: [PATCH v8 03/10] block: support nesting for blk-mq flag QUEUE_FLAG_SAME_FORCE
Nilay Shroff <[email protected]>
| Newsgroups | org.infradead.lists.linux-nvme |
|---|---|
| Message-ID | <[email protected]> |
On 8/18/26 3:28 PM, Hannes Reinecke wrote: > On 8/15/26 7:34 PM, Nilay Shroff wrote: >> QUEUE_FLAG_SAME_FORCE is currently used when setting rq_affinity >> through sysfs as well as by UFS mediatek driver while configuring scsi >> parameters. A subsequent patch adding a latency-based I/O policy >> for NVMe multipath will also use this flag. >> >> With multiple users able to set and clear QUEUE_FLAG_SAME_FORCE, the >> flag needs to support nesting so that one user clearing the flag does >> not inadvertently disable it for another user. >> >> Add a nesting counter, q->same_force_depth, for QUEUE_FLAG_SAME_FORCE. >> The flag is set when the first user acquires it and the nesting counter >> is incremented for each subsequent user. Similarly, each user releases >> its reference by decrementing the counter. The flag is cleared only >> when the last user releases it and the counter reaches zero. >> >> Preserve the existing sysfs rq_affinity semantics with a new >> q->same_force_sysfs flag. When userspace enables QUEUE_FLAG_SAME_FORCE >> by writing 2 to rq_affinity, mark q->same_force_sysfs as set and >> increment q->same_force_depth by one. Subsequent writes of 2 to >> rq_affinity while q->same_force_sysfs is already set are ignored, so >> repeated writes of 2 from userspace do not increase q->same_force_depth. >> Similarly, writing 0 or 1 decrements the q->same_force_depth and if >> nesting counter reached to 0 then clears the QUEUE_FLAG_SAME_FORCE. >> This ensures that multiple writes of 2 to rq_affinity do not require >> multiple writes of 0 or 1. >> >> This change ensures that sysfs interface retains its existing set/clear >> semantics while also allowing other kernel users to hold or release >> QUEUE_FLAG_SAME_FORCE. >> >> Added two new APIs blk_mq_same_force_set() and blk_mq_same_force_clear() >> to set and clear QUEUE_FLAG_SAME_FORCE respectively. Also, updated >> existing call paths using these new APIs which toggles >> QUEUE_FLAG_SAME_FORCE. >> > Yikes. Is this really necessary? > The flag can be set by different subsystems/drivers, sure, but from > what I've seen each user is mutually exclusive. > I'd rather disallow modifying this flag if the driver sets it, to > avoid upper layers or sysfs modifying it. > If we want to disallow userspace from modifying QUEUE_FLAG_SAME_FORCE while a driver owns it, then a userspace write to rq_affinity would have to either fail (e.g. with -EINVAL) or be silently ignored. In either case, that changes the existing userspace/sysfs semantics and may break userspace that expects the write to succeed. Or are you suggesting that a write to rq_affinity to enable QUEUE_FLAG_SAME_FORCE should succeed when the driver has already set it? If so, what should happen when the driver subsequently releases QUEUE_FLAG_SAME_FORCE? In that case, we would clear QUEUE_FLAG_SAME_FORCE when the driver releases it, even though userspace may have previously written 2 to rq_affinity. That would effectively reset the state requested by userspace without informing userspace. This seems to require keeping track of the userspace request separately from the driver's ownership of the flag, which is one of the reasons I introduced same_force_sysfs along with the nesting counter. Thanks, --Nilay