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?
Thanks,
Ming
