Re: XFS reclaim lock order bug

Peter Zijlstra
2010-11-25T07:26:55Z

On Thu, 2010-11-25 at 18:08 +1100, Nick Piggin wrote:
> > +static struct lock_class_key xfs_dead_inode;
> > +
> > STATIC void
> > xfs_fs_evict_inode(
> > struct inode *inode)
> > @@ -1118,6 +1120,8 @@ xfs_fs_evict_inode(
> > */
> > ASSERT(!rwsem_is_locked(&ip->i_iolock.mr_lock));
> > mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino);
> > + lockdep_set_class_and_name(&ip->i_iolock->mr_lock, &xfs_dead_inode,
> > + "xfd_dead_inode");
> >
> > xfs_inactive(ip);
> > }
>
> With this change, I assume the mrlock_init can go? (it would be nice
> to have a wrapper to allocate the class by itself)

mrlock_init() does allocate a class (well rwsem_init, really), but sets
the name to a stringified version of the lock argument.

The lockdep_set_class*() interface is only guaranteed to work on a
freshly initialized lock structure -- which in this case is a bit of a
waste, but for debugging purposes would allow setting a clearer name.

Alternatively, you can write the code like:

xfs_inode_t dead_ip = XFS_I(inode);

mrlock_init(&dead_ip->i_iolock, ...);

In which case its also obvious, as that would result in:

(&(&dead_ip->i_iolock)->mr_lock)

as opposed to:

(&(&ip->i_iolock)->mr_lock)