On 2/19/25 2:04 PM, Nilay Shroff wrote: > > > On 2/19/25 8:54 AM, Ming Lei wrote: >> On Tue, Feb 18, 2025 at 05:29:53PM +0100, Christoph Hellwig wrote: >>> On Tue, Feb 18, 2025 at 09:45:02PM +0800, Ming Lei wrote: >>>> 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. >> >>> >>> 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. > Sorry a typo, I meant we will only have ->store_limit()/->show_limit() and ->store()/show() methods. Also, we'll cleanup load_module() as well as get away with show_nolock() and store_nolock() methods in the next patchset as discussed with Christoph in another thread. Thanks, --Nilay
