On Wed, Mar 20, 2024 at 09:39:57AM +0100, Andre Noll wrote: > On Tue, Mar 19, 11:16, Dave Chinner wrote > > + /* > > + * Well, that sucks. Put the inode back on the inactive queue. > > + * Do this while still under the ILOCK so that we can set the > > + * NEED_INACTIVE flag and clear the INACTIVATING flag an not > > + * have another lookup race with us before we've finished > > + * putting the inode back on the inodegc queue. > > + */ > > + spin_unlock(&ip->i_flags_lock); > > + ip->i_flags |= XFS_NEED_INACTIVE; > > + ip->i_flags &= ~XFS_INACTIVATING; > > + spin_unlock(&ip->i_flags_lock); > > This doesn't look right. Shouldn't the first spin_unlock() be spin_lock()? Yes. So much for my hand inspection of code. :( (Doesn't simple lock debugging catch these sorts of things?) ((It sure would be nice if locking returned a droppable "object" to do the unlock ala Rust and then spin_lock could be __must_check.)) --D > Also, there's a typo in the comment (s/an/and). > Best > Andre > -- > Max Planck Institute for Biology > Tel: (+49) 7071 601 829 > Max-Planck-Ring 5, 72076 Tübingen, Germany > http://people.tuebingen.mpg.de/maan/
