Re: [PATCH V4 9/9] null_blk: serialize configfs attribute shows with the file-scope lock

[Nilay Shroff]

On 7/9/26 3:34 PM, Zizhi Wo wrote:
> The _show callback in the NULLB_DEVICE_ATTR macro reads dev->NAME and the
> _store path writes it. configfs does not serialize accesses across separate
> open file descriptions (buffer->mutex is per-fd), and _show takes no lock,
> so a concurrent read and write on the same attribute is a data race. The
> _show readers also race against writes to these fields that run after the
> configfs item becomes visible, e.g. in nullb_update_nr_hw_queues().
>
> All of those writers now run under the file-scope lock: _store takes it
> unconditionally, and the setup-side writers run under power_store() which
> holds the same lock. The only remaining unsynchronized accesses are the
> plain reads in _show. Rather than annotating every field with
> READ_ONCE()/WRITE_ONCE() across files, simply take the file-scope lock in
> _show (and in power_show) as well. This closes the remaining _show-vs-write
> data races with a single lock and keeps the writers as plain assignments.
>
> configfs attribute access is not on the I/O hot path, so taking the mutex
> in _show is acceptable from a performance standpoint. The dev fields
> written in null_alloc_dev() and dev->power in nullb_group_drop_item() need
> no locking: the former runs from .make_group before the item is published,
> and the latter is serialized by configfs frag_sem/frag_dead against
> attribute show/store.
>
> Suggested-by: Nilay Shroff<nilay@xxxxxxxxxxxxx>
> Signed-off-by: Zizhi Wo<wozizhi@xxxxxxxxxx>

Looks good to me.

Reviewed-by: Nilay Shroff <nilay@xxxxxxxxxxxxx>