On Thu, Nov 02, 2023 at 02:48:42PM +0100, Jan Kara wrote: > On Fri 27-10-23 11:35:20, Christian Brauner wrote: > > So, I believe we can drop the sb_lock in bdev_super_lock() for all > > holder operations if we turn s_count into an atomic. It will slightly > > change semantics for list walks like iterate_supers() but imho that's > > fine. It'll mean that list walkes need a acquire sb_lock, then try to > > get reference via atomic_inc_not_zero(). > > > > The logic there is simply that if you still found the superblock on the > > list then yes, someone could now concurrently drop s_count to zero > > behind your back. But because you hold sb_lock they can't remove it from > > the list behind your back. > > > > So if you now fail atomic_inc_not_zero() then you know that someone > > concurrently managed to drop the ref to zero and wants to remove that sb > > from the list. So you just ignore that super block and go on walking the > > list. If however, you manage to get an active reference things are fine > > and you can try to trade that temporary reference for an active > > reference. So my theory at least... > > > > Yes, ofc we add atomics but for superblocks we shouldn't care especially > > we have less and less list walkers. Both get_super() and > > get_active_super() are gone after all. > > > > I'm running xfstests as I'm sending this and I need to start finishing > > PRs so in RFC mode. Thoughts? > > > > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx> > > So in principle I agree we can get rid of some sb_lock use if we convert > sb->s_count to an atomic type. However does this bring any significant > benefit? I would not expect sb_lock to be contended and as you say you need > to be more careful about the races then so that is a reason against such > change. I like that we elide the spinlock in cases where we go from block device to superblock. It's more elegant. But in practice I think it won't matter. It's not that bdev_*() calls are extremely performant.
