On Wed, Feb 19, 2025 at 02:26:49PM +0530, Nilay Shroff wrote: > > > 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. OK, that looks much better! Thanks, Ming
