On 2/18/25 5:40 PM, Ming Lei wrote:
> On Tue, Feb 18, 2025 at 01:58:54PM +0530, Nilay Shroff wrote:
>> There're few sysfs attributes in block layer which don't really need
>> acquiring q->sysfs_lock while accessing it. The reason being, writing
>> a value to such attributes are either atomic or could be easily
>> protected using WRITE_ONCE()/READ_ONCE(). Moreover, sysfs attributes
>> are inherently protected with sysfs/kernfs internal locking.
>>
>> So this change help segregate all existing sysfs attributes for which
>> we could avoid acquiring q->sysfs_lock. We group all such attributes,
>> which don't require any sorts of locking, using macro QUEUE_RO_ENTRY_
>> NOLOCK() or QUEUE_RW_ENTRY_NOLOCK(). The newly introduced show/store
>> method (show_nolock/store_nolock) is assigned to attributes using these
>> new macros. The show_nolock/store_nolock run without holding q->sysfs_
>> lock.
>>
>> Signed-off-by: Nilay Shroff <nilay@xxxxxxxxxxxxx>
>> ---
>
> ...
>
>>
>> +#define QUEUE_RO_ENTRY_NOLOCK(_prefix, _name) \
>> +static struct queue_sysfs_entry _prefix##_entry = { \
>> + .attr = {.name = _name, .mode = 0644 }, \
>> + .show_nolock = _prefix##_show, \
>> +}
>> +
>> +#define QUEUE_RW_ENTRY_NOLOCK(_prefix, _name) \
>> +static struct queue_sysfs_entry _prefix##_entry = { \
>> + .attr = {.name = _name, .mode = 0644 }, \
>> + .show_nolock = _prefix##_show, \
>> + .store_nolock = _prefix##_store, \
>> +}
>> +
>> #define QUEUE_RW_ENTRY(_prefix, _name) \
>> static struct queue_sysfs_entry _prefix##_entry = { \
>> .attr = { .name = _name, .mode = 0644 }, \
>> @@ -446,7 +470,7 @@ QUEUE_RO_ENTRY(queue_max_discard_segments, "max_discard_segments");
>> QUEUE_RO_ENTRY(queue_discard_granularity, "discard_granularity");
>> QUEUE_RO_ENTRY(queue_max_hw_discard_sectors, "discard_max_hw_bytes");
>> QUEUE_LIM_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
>> -QUEUE_RO_ENTRY(queue_discard_zeroes_data, "discard_zeroes_data");
>> +QUEUE_RO_ENTRY_NOLOCK(queue_discard_zeroes_data, "discard_zeroes_data");
>
> I think all QUEUE_RO_ENTRY needn't sysfs_lock, why do you just convert
> part of them?
>
I think we have few read-only attributes which still need protection
using q->limits_lock. So if you refer 2nd patch in the series then you'd
find it. In this patch we group only attributes which don't require any
locking and grouped them under show_nolock/store_nolock.
Thanks,
--Nilay
