Re: [PATCH v8 03/10] block: support nesting for blk-mq flag QUEUE_FLAG_SAME_FORCE

On 8/18/26 1:59 PM, Nilay Shroff wrote:
> 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.
> 
But _allowing_ userspace to modify it for drivers which requires it to
be set will break the drivers.
Similar for the upcoming latency-based I/O scheduler; disabling
QUEUE_FLAG_SAME_FORCE there will cause misaccounting and the scheduler
will not work properly.

So I would argue to have a flag for making QUEUE_FLAG_SAME_FORCE driver 
owned, and reject any attempts to change it via sysfs with EINVAL.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich