Hi Ingo,
On Tue, Jul 04, 2006 at 10:41:43AM +0200, Ingo Molnar wrote:
> * Ingo Molnar <mingo@xxxxxxx> wrote:
> > correct, lockdep has to be taught about relations between locks within
> > the same lock-class. (it detects relations between different
> > lock-classes automatically) It's usually a straightforward process.
> >
> > In this particular case we probably need to do something similar to
> > the VFS's 'enum inode_i_mutex_lock_class' subclass categorizations: we
> > need xfs_ilock_nested(.., subclass), where in xfs_lock_dir_and_entry()
> > we'd pass in ILOCK_PARENT. [normal locking calls have a default
> > subclass ID of 0]
>
> the patch below should solve the case mentioned above, but i'm sure
> there will be other cases as well.
OK, I'm looking at the next one - we have a routine which will take
two arbitary xfs_inode's and lock them in inode number order, named
xfs_lock_inodes. Its used on link, rename, and by the online file
defragmenter... how could I approach annotating that? I can't see
an obvious way to fit it into this "nesting subclasses" model you've
outlined (below). Also, how/where does your new xfs_ilock_class enum
get used?
> +/*
> + * ilock nesting subclasses for the lock validator:
> + *
> + * 0: the object of the current VFS operation
> + * 1: parent
> + * 2: child/target
> + * 3: quota file
> + *
> + * The locking order between these classes is
> + * parent -> child -> normal -> quota
> + */
> +enum xfs_ilock_class
> +{
> + ILOCK_NORMAL,
> + ILOCK_PARENT,
> + ILOCK_CHILD,
> + ILOCK_QUOTA
> +};
Thanks. Oh, here's the details of the lockdep report from a link
operation, for example:
=============================================
[ INFO: possible recursive locking detected ]
---------------------------------------------
mount/997 is trying to acquire lock:
(&(&ip->i_lock)->mr_lock){----}, at: [<c028aeae>] xfs_ilock+0xae/0xe0
but task is already holding lock:
(&(&ip->i_lock)->mr_lock){----}, at: [<c028aeae>] xfs_ilock+0xae/0xe0
other info that might help us debug this:
3 locks held by mount/997:
#0: (&inode->i_mutex/1){--..}, at: [<c016f885>] lookup_create+0x25/0x80
#1: (&inode->i_mutex){--..}, at: [<c04177b8>] mutex_lock+0x8/0x10
#2: (&(&ip->i_lock)->mr_lock){----}, at: [<c028aeae>] xfs_ilock+0xae/0xe0
stack backtrace:
[<c01041bb>] show_trace+0x1b/0x20
[<c0104964>] dump_stack+0x24/0x30
[<c0135276>] __lock_acquire+0x606/0xd10
[<c0135d09>] lock_acquire+0x69/0x90
[<c0131987>] down_write+0x37/0x50
[<c028aeae>] xfs_ilock+0xae/0xe0
[<c02acce3>] xfs_lock_inodes+0x123/0x150
[<c02b1554>] xfs_link+0x1a4/0x510
[<c02bc74b>] xfs_vn_link+0x4b/0xb0
[<c016ec50>] vfs_link+0xe0/0x150
[<c0171d4c>] sys_linkat+0xfc/0x120
[<c0171d9f>] sys_link+0x2f/0x40
[<c01030e3>] syscall_call+0x7/0xb
--
Nathan
|