2005-11-17 22:07:15

by Steve Dickson

[permalink] [raw]
Subject: [PATCH] spinlock recursion on inode number mismatches

When compiling over NFS using a 2.6.14 kernel, the following
spinlock recursion BUG popped:

nfs_update_inode: inode number mismatch
expected (0:16/0x1dee71a), got (0:16/0x100000000000000)
nfs_update_inode: inode number mismatch
expected (0:16/0x1dee71a), got (0:16/0x1dee719)
nfs_update_inode: inode number mismatch
expected (0:16/0x1dee71a), got (0:16/0x0)
nfs_update_inode: inode number mismatch
expected (0:16/0x1dee71a), got (0:16/0x1dee719)
BUG: spinlock recursion on CPU#0, bhc/22635 (Not tainted)
lock: db7294e8, .magic: dead4ead, .owner: bhc/22635, .owner_cpu: 0
[<c01f53b3>] spin_bug+0xa3/0xd7
[<c01f551e>] _raw_spin_lock+0x68/0x6a
[<f8db1151>] nfs_zap_caches+0x1a/0xaa [nfs]
[<f8db2445>] nfs_update_inode+0x9f/0x618 [nfs]
[<c033be8c>] _spin_unlock_irq+0x5/0x7
[<f8db236b>] nfs_post_op_update_inode+0x2c/0x67 [nfs]
[<f8dba716>] nfs3_proc_remove+0x9e/0xd7 [nfs]
[<f8daed5a>] nfs_safe_remove+0x68/0xc4 [nfs]
[<f8daee5e>] nfs_unlink+0xa8/0x10e [nfs]
[<c017c8df>] vfs_unlink+0x19f/0x1a6
[<c017c9a3>] sys_unlink+0xbd/0x13e
[<c033cee2>] do_page_fault+0x262/0x650
[<c016cb1d>] do_sync_read+0x0/0x116
[<c01040a5>] syscall_call+0x7/0xb
Kernel panic - not syncing: bad locking
[<c01250a8>] panic+0x45/0x1c5
[<c01f53e7>] __spin_lock_debug+0x0/0xcf
[<c01f551e>] _raw_spin_lock+0x68/0x6a
[<f8db1151>] nfs_zap_caches+0x1a/0xaa [nfs]
[<f8db2445>] nfs_update_inode+0x9f/0x618 [nfs]
[<c033be8c>] _spin_unlock_irq+0x5/0x7
[<f8db236b>] nfs_post_op_update_inode+0x2c/0x67 [nfs]
[<f8dba716>] nfs3_proc_remove+0x9e/0xd7 [nfs]
[<f8daed5a>] nfs_safe_remove+0x68/0xc4 [nfs]
[<f8daee5e>] nfs_unlink+0xa8/0x10e [nfs]
[<c017c8df>] vfs_unlink+0x19f/0x1a6
[<c017c9a3>] sys_unlink+0xbd/0x13e
[<c033cee2>] do_page_fault+0x262/0x650
[<c016cb1d>] do_sync_read+0x0/0x116
[<c01040a5>] syscall_call+0x7/0xb

The attached patch solve the problem by the problem by moving
the call to nfs_invalidate_inode() out of nfs_invalidate_inode().

Commments?

steved.



Attachments:
linux-2.6.14-nfs-spinlock.patch (1.68 kB)

2005-11-17 22:34:32

by Lever, Charles

[permalink] [raw]
Subject: RE: [PATCH] spinlock recursion on inode number mismatches

> When compiling over NFS using a 2.6.14 kernel, the following
> spinlock recursion BUG popped:
>=20
> nfs_update_inode: inode number mismatch
> expected (0:16/0x1dee71a), got (0:16/0x100000000000000)
> nfs_update_inode: inode number mismatch
> expected (0:16/0x1dee71a), got (0:16/0x1dee719)
> nfs_update_inode: inode number mismatch
> expected (0:16/0x1dee71a), got (0:16/0x0)
> nfs_update_inode: inode number mismatch
> expected (0:16/0x1dee71a), got (0:16/0x1dee719)
> BUG: spinlock recursion on CPU#0, bhc/22635 (Not tainted)
> lock: db7294e8, .magic: dead4ead, .owner: bhc/22635, .owner_cpu: 0
> [<c01f53b3>] spin_bug+0xa3/0xd7
> [<c01f551e>] _raw_spin_lock+0x68/0x6a
> [<f8db1151>] nfs_zap_caches+0x1a/0xaa [nfs]
> [<f8db2445>] nfs_update_inode+0x9f/0x618 [nfs]
> [<c033be8c>] _spin_unlock_irq+0x5/0x7
> [<f8db236b>] nfs_post_op_update_inode+0x2c/0x67 [nfs]
> [<f8dba716>] nfs3_proc_remove+0x9e/0xd7 [nfs]
> [<f8daed5a>] nfs_safe_remove+0x68/0xc4 [nfs]
> [<f8daee5e>] nfs_unlink+0xa8/0x10e [nfs]
> [<c017c8df>] vfs_unlink+0x19f/0x1a6
> [<c017c9a3>] sys_unlink+0xbd/0x13e
> [<c033cee2>] do_page_fault+0x262/0x650
> [<c016cb1d>] do_sync_read+0x0/0x116
> [<c01040a5>] syscall_call+0x7/0xb
> Kernel panic - not syncing: bad locking
> [<c01250a8>] panic+0x45/0x1c5
> [<c01f53e7>] __spin_lock_debug+0x0/0xcf
> [<c01f551e>] _raw_spin_lock+0x68/0x6a
> [<f8db1151>] nfs_zap_caches+0x1a/0xaa [nfs]
> [<f8db2445>] nfs_update_inode+0x9f/0x618 [nfs]
> [<c033be8c>] _spin_unlock_irq+0x5/0x7
> [<f8db236b>] nfs_post_op_update_inode+0x2c/0x67 [nfs]
> [<f8dba716>] nfs3_proc_remove+0x9e/0xd7 [nfs]
> [<f8daed5a>] nfs_safe_remove+0x68/0xc4 [nfs]
> [<f8daee5e>] nfs_unlink+0xa8/0x10e [nfs]
> [<c017c8df>] vfs_unlink+0x19f/0x1a6
> [<c017c9a3>] sys_unlink+0xbd/0x13e
> [<c033cee2>] do_page_fault+0x262/0x650
> [<c016cb1d>] do_sync_read+0x0/0x116
> [<c01040a5>] syscall_call+0x7/0xb
>=20
> The attached patch solve the problem by the problem by moving
> the call to nfs_invalidate_inode() out of nfs_invalidate_inode().

steve-

can you send a diagram of the lock recursion you see?


-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc. Get Certified Today
Register for a JBoss Training Course. Free Certification Exam
for All Training Attendees Through End of 2005. For more info visit:
http://ads.osdn.com/?ad_id=7628&alloc_id=16845&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2005-11-17 23:30:53

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] spinlock recursion on inode number mismatches

On Thu, 2005-11-17 at 17:07 -0500, Steve Dickson wrote:
> The attached patch solve the problem by the problem by moving
> the call to nfs_invalidate_inode() out of nfs_invalidate_inode().

Hmm... That will cause us to call make_bad_inode on all errors in
nfs_update_inode().

Given the sort of things we have in NFSv4 these days (delegations etc)
that need to be cleaned up on inode release, I'm not sure we should ever
be calling make_bad_inode().

How about something like the appended patch instead?

Cheers,
Trond
-------
NFS: Fix a spinlock recursion inside nfs_update_inode()

Signed-off-by: Trond Myklebust <[email protected]>
---

fs/nfs/inode.c | 26 ++++++++++++--------------
1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 6391d89..aaab1a5 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -643,14 +643,11 @@ static int nfs_show_options(struct seq_f
/*
* Invalidate the local caches
*/
-void
-nfs_zap_caches(struct inode *inode)
+static void nfs_zap_caches_locked(struct inode *inode)
{
struct nfs_inode *nfsi = NFS_I(inode);
int mode = inode->i_mode;

- spin_lock(&inode->i_lock);
-
NFS_ATTRTIMEO(inode) = NFS_MINATTRTIMEO(inode);
NFS_ATTRTIMEO_UPDATE(inode) = jiffies;

@@ -659,7 +656,12 @@ nfs_zap_caches(struct inode *inode)
nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL|NFS_INO_REVAL_PAGECACHE;
else
nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL|NFS_INO_REVAL_PAGECACHE;
+}

+void nfs_zap_caches(struct inode *inode)
+{
+ spin_lock(&inode->i_lock);
+ nfs_zap_caches_locked(inode);
spin_unlock(&inode->i_lock);
}

@@ -676,16 +678,13 @@ static void nfs_zap_acl_cache(struct ino
}

/*
- * Invalidate, but do not unhash, the inode
+ * Invalidate, but do not unhash, the inode.
+ * NB: must be called with inode->i_lock held!
*/
-static void
-nfs_invalidate_inode(struct inode *inode)
+static void nfs_invalidate_inode(struct inode *inode)
{
- umode_t save_mode = inode->i_mode;
-
- make_bad_inode(inode);
- inode->i_mode = save_mode;
- nfs_zap_caches(inode);
+ set_bit(NFS_INO_STALE, &NFS_FLAGS(inode));
+ nfs_zap_caches_locked(inode);
}

struct nfs_find_desc {
@@ -1528,14 +1527,13 @@ static int nfs_update_inode(struct inode
printk(KERN_DEBUG "%s: inode %ld mode changed, %07o to %07o\n",
__FUNCTION__, inode->i_ino, inode->i_mode, fattr->mode);
#endif
+ out_err:
/*
* No need to worry about unhashing the dentry, as the
* lookup validation will know that the inode is bad.
* (But we fall through to invalidate the caches.)
*/
nfs_invalidate_inode(inode);
- out_err:
- set_bit(NFS_INO_STALE, &NFS_FLAGS(inode));
return -ESTALE;
}





-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc. Get Certified Today
Register for a JBoss Training Course. Free Certification Exam
for All Training Attendees Through End of 2005. For more info visit:
http://ads.osdn.com/?ad_id=7628&alloc_id=16845&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2005-11-18 11:42:51

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] spinlock recursion on inode number mismatches

Trond Myklebust wrote:
> Hmm... That will cause us to call make_bad_inode on all errors in
> nfs_update_inode().
Good point... I didn't see that...

>
> How about something like the appended patch instead?
Yes... creating away for nfs_zap_caches() to be called
while holding the inode spinlock does in deed fix the
problem... nice work... thanks!

steved.


-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc. Get Certified Today
Register for a JBoss Training Course. Free Certification Exam
for All Training Attendees Through End of 2005. For more info visit:
http://ads.osdn.com/?ad_id=7628&alloc_id=16845&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs