Re: [PATCHv2 1/6] blk-sysfs: remove q->sysfs_lock for attributes which don't need it

On 2/19/25 8:54 AM, Ming Lei wrote:

On Tue, Feb 18, 2025 at 05:29:53PM +0100, Christoph Hellwig wrote:

quoted

On Tue, Feb 18, 2025 at 09:45:02PM +0800, Ming Lei wrote:

quoted

IMO, this RO attributes needn't protection from q->limits_lock:

- no lifetime issue

- in-tree code needn't limits_lock.

- all are scalar variable, so the attribute itself is updated atomically

Except in the memory model they aren't without READ_ONCE/WRITE_ONCE.

RW_ONCE is supposed for avoiding compiler optimization, and scalar
variable atomic update should be decided by hardware.

quoted

Given that the limits_lock is not a hot lock taking the lock is a very
easy way to mark our intent.  And if we get things like thread thread
sanitizer patches merged that will become essential.  Even KCSAN
might object already without it.

My main concern is that there are too many ->store()/->load() variants
now, but not deal if you think this way is fine, :-)

We will only have ->store_limit()/->show_limit() and ->store()/->load() in
the next patchset as I am going to cleanup load_module() as well as get away with show_nolock() and store_nolock() methods as discussed with Christoph in 
another thread.

Thanks,
--Nilay