Re: lockdep wierdness...

2007-09-27 Thread Peter Zijlstra
On Thu, 2007-09-27 at 15:00 +0100, Christoph Hellwig wrote:
> On Thu, Sep 27, 2007 at 03:51:07PM +0200, Peter Zijlstra wrote:
> > Christoph,
> > 
> > does Steve's story make sense? 
> 
> Yes.
> 
> > All that would need to be done is add an extra lock_class_key to
> > file_system_type for i_mutex_dir_key, and extend alloc_inode to say
> > something like:
> > 
> >   if (dir)
> > lockdep_set_class(>i_mutex, >s_type->i_mutex_dir_key);
> >   else
> > lockdep_set_class(>i_mutex, >s_type->i_mutex_key);
> 
> Unfortunately we don't know what type of inode we have when calling
> alloc_inode.  We only know it after reading in the inode from disk,
> aka in unlock_new_inode.  Then again there is no reason to use
> i_mutex before unlock_new_inode returns, so maybe we could defer
> initializing it until unlock_new_inode.  I'm pretty sure we'll have
> to fix a few filesystems that take i_mutex before that despite not
> needing it, e.g. through i_size_write, though.

How about this:

---
Make a distinction between file and dir usage of i_mutex.

The inode should be complete and unused at unlock_new_inode(), re-init
i_mutex depending on its type.

Signed-off-by: Peter Zijlstra <[EMAIL PROTECTED]>
---
 fs/inode.c |   12 
 include/linux/fs.h |1 +
 2 files changed, 13 insertions(+)

Index: linux-2.6/fs/inode.c
===
--- linux-2.6.orig/fs/inode.c
+++ linux-2.6/fs/inode.c
@@ -576,6 +576,18 @@ EXPORT_SYMBOL(new_inode);
 
 void unlock_new_inode(struct inode *inode)
 {
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+   struct file_system_type *type = inode->i_sb->s_type;
+   /*
+* ensure nobody is actually holding i_mutex
+*/
+   mutex_destroy(>i_mutex);
+   mutex_init(>i_mutex);
+   if (inode->i_mode & S_IFDIR)
+   lockdep_set_class(>i_mutex, >i_mutex_dir_key);
+   else
+   lockdep_set_class(>i_mutex, >i_mutex_key);
+#endif
/*
 * This is special!  We do not need the spinlock
 * when clearing I_LOCK, because we're guaranteed
Index: linux-2.6/include/linux/fs.h
===
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -1426,6 +1426,7 @@ struct file_system_type {
 
struct lock_class_key i_lock_key;
struct lock_class_key i_mutex_key;
+   struct lock_class_key i_mutex_dir_key;
struct lock_class_key i_alloc_sem_key;
 };
 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: lockdep wierdness...

2007-09-27 Thread Christoph Hellwig
On Thu, Sep 27, 2007 at 03:51:07PM +0200, Peter Zijlstra wrote:
> Christoph,
> 
> does Steve's story make sense? 

Yes.

> All that would need to be done is add an extra lock_class_key to
> file_system_type for i_mutex_dir_key, and extend alloc_inode to say
> something like:
> 
>   if (dir)
> lockdep_set_class(>i_mutex, >s_type->i_mutex_dir_key);
>   else
> lockdep_set_class(>i_mutex, >s_type->i_mutex_key);

Unfortunately we don't know what type of inode we have when calling
alloc_inode.  We only know it after reading in the inode from disk,
aka in unlock_new_inode.  Then again there is no reason to use
i_mutex before unlock_new_inode returns, so maybe we could defer
initializing it until unlock_new_inode.  I'm pretty sure we'll have
to fix a few filesystems that take i_mutex before that despite not
needing it, e.g. through i_size_write, though.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: lockdep wierdness...

2007-09-27 Thread Peter Zijlstra
On Mon, 2007-09-24 at 22:13 -0400, Steven Rostedt wrote:
> On Mon, Sep 24, 2007 at 06:07:38PM -0400, Trond Myklebust wrote:
> > I'm seeing lockdep warning about a potential lock inversion between
> > >mmap_sem and >i_mutex in NFS (see attachment).
> > 
> > Unfortunately the basis for the warning appears to be the behaviour in
> > ext3(???). AFAICS there is no way for NFS to share an inode->i_mutex
> > with ext3. What to do?
> 
> Actually this can probably happen just on NFS alone.
> 
> > 
> > Trond
> 
> > ===
> > [ INFO: possible circular locking dependency detected ]
> > 2.6.23-rc7-g8809e921 #1
> > ---
> > beagle-build-in/24375 is trying to acquire lock:
> >  (>mmap_sem){}, at: [] do_page_fault+0x17d/0x591
> > 
> > but task is already holding lock:
> >  (>i_mutex){--..}, at: [] mutex_lock+0x1c/0x1f
> > 
> > which lock already depends on the new lock.
> > 
> > 
> > the existing dependency chain (in reverse order) is:
> > 
> > -> #1 (>i_mutex){--..}:
> >[] __lock_acquire+0x9f3/0xba6
> >[] lock_acquire+0x5f/0x78
> >[] __mutex_lock_slowpath+0xe5/0x27a
> >[] mutex_lock+0x1c/0x1f
> >[] nfs_revalidate_mapping+0x64/0x9c [nfs]
> >[] nfs_file_mmap+0x46/0x75 [nfs]
> >[] mmap_region+0x1ea/0x3b8
> >[] do_mmap_pgoff+0x27b/0x2da
> >[] sys_mmap2+0x9b/0xb5
> >[] sysenter_past_esp+0x5f/0x99
> >[] 0x
> > 
> > -> #0 (>mmap_sem){}:
> >[] __lock_acquire+0x8df/0xba6
> >[] lock_acquire+0x5f/0x78
> >[] down_read+0x3a/0x4c
> >[] do_page_fault+0x17d/0x591
> >[] error_code+0x72/0x78
> >[] call_filldir+0xac/0xc3 [ext3]
> >[] ext3_readdir+0x217/0x5e5 [ext3]
> >[] vfs_readdir+0x67/0x93
> >[] sys_getdents+0x5f/0x9d
> >[] sysenter_past_esp+0x5f/0x99
> >[] 0x
> 
> The circular lock seems to be this:
> 
> #1:
> 
>   sys_mmap2:  down_write(>mmap_sem);
>   nfs_revalidate_mapping: mutex_lock(>i_mutex);
> 
> 
> #0:
> 
>   vfs_readdir: mutex_lock(>i_mutex);
>- during the readdir (filldir64), we take a user fault (missing page?)
> and call do_page_fault -
>   do_page_fault:   down_read(>mmap_sem);
> 
> 
> So it does indeed look like a circular locking. Now the question is, "is
> this a bug?".  Looking like the inode of #1 must be a file or something
> else that you can mmap and the inode of #0 seems it must be a directory.
> I would say "no".
> 
> Now if you can readdir on a file or mmap a directory, then this could be
> an issue.
> 
> Otherwise, I'd love to see someone teach lockdep about this issue! ;-)

Christoph,

does Steve's story make sense? 

If so, do we know at alloc_inode() time what type of inode we're
requesting; file or dir. Again, if so, the lockdep annotation should be
trivial in the light of the recently merged patch:

 + lockdep-give-each-filesystem-its-own-inode-lock-class.patch

All that would need to be done is add an extra lock_class_key to
file_system_type for i_mutex_dir_key, and extend alloc_inode to say
something like:

  if (dir)
lockdep_set_class(>i_mutex, >s_type->i_mutex_dir_key);
  else
lockdep_set_class(>i_mutex, >s_type->i_mutex_key);


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: lockdep wierdness...

2007-09-27 Thread Peter Zijlstra
On Mon, 2007-09-24 at 22:13 -0400, Steven Rostedt wrote:
 On Mon, Sep 24, 2007 at 06:07:38PM -0400, Trond Myklebust wrote:
  I'm seeing lockdep warning about a potential lock inversion between
  mm-mmap_sem and inode-i_mutex in NFS (see attachment).
  
  Unfortunately the basis for the warning appears to be the behaviour in
  ext3(???). AFAICS there is no way for NFS to share an inode-i_mutex
  with ext3. What to do?
 
 Actually this can probably happen just on NFS alone.
 
  
  Trond
 
  ===
  [ INFO: possible circular locking dependency detected ]
  2.6.23-rc7-g8809e921 #1
  ---
  beagle-build-in/24375 is trying to acquire lock:
   (mm-mmap_sem){}, at: [c05a2887] do_page_fault+0x17d/0x591
  
  but task is already holding lock:
   (inode-i_mutex){--..}, at: [c059f9e3] mutex_lock+0x1c/0x1f
  
  which lock already depends on the new lock.
  
  
  the existing dependency chain (in reverse order) is:
  
  - #1 (inode-i_mutex){--..}:
 [c043d4da] __lock_acquire+0x9f3/0xba6
 [c043da62] lock_acquire+0x5f/0x78
 [c059f832] __mutex_lock_slowpath+0xe5/0x27a
 [c059f9e3] mutex_lock+0x1c/0x1f
 [f8c92495] nfs_revalidate_mapping+0x64/0x9c [nfs]
 [f8c8ff2a] nfs_file_mmap+0x46/0x75 [nfs]
 [c046097c] mmap_region+0x1ea/0x3b8
 [c0460e9b] do_mmap_pgoff+0x27b/0x2da
 [c0407d77] sys_mmap2+0x9b/0xb5
 [c040405e] sysenter_past_esp+0x5f/0x99
 [] 0x
  
  - #0 (mm-mmap_sem){}:
 [c043d3c6] __lock_acquire+0x8df/0xba6
 [c043da62] lock_acquire+0x5f/0x78
 [c04360db] down_read+0x3a/0x4c
 [c05a2887] do_page_fault+0x17d/0x591
 [c05a1382] error_code+0x72/0x78
 [f88acaac] call_filldir+0xac/0xc3 [ext3]
 [f88acdb2] ext3_readdir+0x217/0x5e5 [ext3]
 [c04798a1] vfs_readdir+0x67/0x93
 [c0479af6] sys_getdents+0x5f/0x9d
 [c040405e] sysenter_past_esp+0x5f/0x99
 [] 0x
 
 The circular lock seems to be this:
 
 #1:
 
   sys_mmap2:  down_write(mm-mmap_sem);
   nfs_revalidate_mapping: mutex_lock(inode-i_mutex);
 
 
 #0:
 
   vfs_readdir: mutex_lock(inode-i_mutex);
- during the readdir (filldir64), we take a user fault (missing page?)
 and call do_page_fault -
   do_page_fault:   down_read(mm-mmap_sem);
 
 
 So it does indeed look like a circular locking. Now the question is, is
 this a bug?.  Looking like the inode of #1 must be a file or something
 else that you can mmap and the inode of #0 seems it must be a directory.
 I would say no.
 
 Now if you can readdir on a file or mmap a directory, then this could be
 an issue.
 
 Otherwise, I'd love to see someone teach lockdep about this issue! ;-)

Christoph,

does Steve's story make sense? 

If so, do we know at alloc_inode() time what type of inode we're
requesting; file or dir. Again, if so, the lockdep annotation should be
trivial in the light of the recently merged patch:

 + lockdep-give-each-filesystem-its-own-inode-lock-class.patch

All that would need to be done is add an extra lock_class_key to
file_system_type for i_mutex_dir_key, and extend alloc_inode to say
something like:

  if (dir)
lockdep_set_class(inode-i_mutex, sb-s_type-i_mutex_dir_key);
  else
lockdep_set_class(inode-i_mutex, sb-s_type-i_mutex_key);


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: lockdep wierdness...

2007-09-27 Thread Christoph Hellwig
On Thu, Sep 27, 2007 at 03:51:07PM +0200, Peter Zijlstra wrote:
 Christoph,
 
 does Steve's story make sense? 

Yes.

 All that would need to be done is add an extra lock_class_key to
 file_system_type for i_mutex_dir_key, and extend alloc_inode to say
 something like:
 
   if (dir)
 lockdep_set_class(inode-i_mutex, sb-s_type-i_mutex_dir_key);
   else
 lockdep_set_class(inode-i_mutex, sb-s_type-i_mutex_key);

Unfortunately we don't know what type of inode we have when calling
alloc_inode.  We only know it after reading in the inode from disk,
aka in unlock_new_inode.  Then again there is no reason to use
i_mutex before unlock_new_inode returns, so maybe we could defer
initializing it until unlock_new_inode.  I'm pretty sure we'll have
to fix a few filesystems that take i_mutex before that despite not
needing it, e.g. through i_size_write, though.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: lockdep wierdness...

2007-09-27 Thread Peter Zijlstra
On Thu, 2007-09-27 at 15:00 +0100, Christoph Hellwig wrote:
 On Thu, Sep 27, 2007 at 03:51:07PM +0200, Peter Zijlstra wrote:
  Christoph,
  
  does Steve's story make sense? 
 
 Yes.
 
  All that would need to be done is add an extra lock_class_key to
  file_system_type for i_mutex_dir_key, and extend alloc_inode to say
  something like:
  
if (dir)
  lockdep_set_class(inode-i_mutex, sb-s_type-i_mutex_dir_key);
else
  lockdep_set_class(inode-i_mutex, sb-s_type-i_mutex_key);
 
 Unfortunately we don't know what type of inode we have when calling
 alloc_inode.  We only know it after reading in the inode from disk,
 aka in unlock_new_inode.  Then again there is no reason to use
 i_mutex before unlock_new_inode returns, so maybe we could defer
 initializing it until unlock_new_inode.  I'm pretty sure we'll have
 to fix a few filesystems that take i_mutex before that despite not
 needing it, e.g. through i_size_write, though.

How about this:

---
Make a distinction between file and dir usage of i_mutex.

The inode should be complete and unused at unlock_new_inode(), re-init
i_mutex depending on its type.

Signed-off-by: Peter Zijlstra [EMAIL PROTECTED]
---
 fs/inode.c |   12 
 include/linux/fs.h |1 +
 2 files changed, 13 insertions(+)

Index: linux-2.6/fs/inode.c
===
--- linux-2.6.orig/fs/inode.c
+++ linux-2.6/fs/inode.c
@@ -576,6 +576,18 @@ EXPORT_SYMBOL(new_inode);
 
 void unlock_new_inode(struct inode *inode)
 {
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+   struct file_system_type *type = inode-i_sb-s_type;
+   /*
+* ensure nobody is actually holding i_mutex
+*/
+   mutex_destroy(inode-i_mutex);
+   mutex_init(inode-i_mutex);
+   if (inode-i_mode  S_IFDIR)
+   lockdep_set_class(inode-i_mutex, type-i_mutex_dir_key);
+   else
+   lockdep_set_class(inode-i_mutex, type-i_mutex_key);
+#endif
/*
 * This is special!  We do not need the spinlock
 * when clearing I_LOCK, because we're guaranteed
Index: linux-2.6/include/linux/fs.h
===
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -1426,6 +1426,7 @@ struct file_system_type {
 
struct lock_class_key i_lock_key;
struct lock_class_key i_mutex_key;
+   struct lock_class_key i_mutex_dir_key;
struct lock_class_key i_alloc_sem_key;
 };
 


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: lockdep wierdness...

2007-09-24 Thread Steven Rostedt
On Mon, Sep 24, 2007 at 06:07:38PM -0400, Trond Myklebust wrote:
> I'm seeing lockdep warning about a potential lock inversion between
> >mmap_sem and >i_mutex in NFS (see attachment).
> 
> Unfortunately the basis for the warning appears to be the behaviour in
> ext3(???). AFAICS there is no way for NFS to share an inode->i_mutex
> with ext3. What to do?

Actually this can probably happen just on NFS alone.

> 
> Trond

> ===
> [ INFO: possible circular locking dependency detected ]
> 2.6.23-rc7-g8809e921 #1
> ---
> beagle-build-in/24375 is trying to acquire lock:
>  (>mmap_sem){}, at: [] do_page_fault+0x17d/0x591
> 
> but task is already holding lock:
>  (>i_mutex){--..}, at: [] mutex_lock+0x1c/0x1f
> 
> which lock already depends on the new lock.
> 
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #1 (>i_mutex){--..}:
>[] __lock_acquire+0x9f3/0xba6
>[] lock_acquire+0x5f/0x78
>[] __mutex_lock_slowpath+0xe5/0x27a
>[] mutex_lock+0x1c/0x1f
>[] nfs_revalidate_mapping+0x64/0x9c [nfs]
>[] nfs_file_mmap+0x46/0x75 [nfs]
>[] mmap_region+0x1ea/0x3b8
>[] do_mmap_pgoff+0x27b/0x2da
>[] sys_mmap2+0x9b/0xb5
>[] sysenter_past_esp+0x5f/0x99
>[] 0x
> 
> -> #0 (>mmap_sem){}:
>[] __lock_acquire+0x8df/0xba6
>[] lock_acquire+0x5f/0x78
>[] down_read+0x3a/0x4c
>[] do_page_fault+0x17d/0x591
>[] error_code+0x72/0x78
>[] call_filldir+0xac/0xc3 [ext3]
>[] ext3_readdir+0x217/0x5e5 [ext3]
>[] vfs_readdir+0x67/0x93
>[] sys_getdents+0x5f/0x9d
>[] sysenter_past_esp+0x5f/0x99
>[] 0x

The circular lock seems to be this:

#1:

  sys_mmap2:  down_write(>mmap_sem);
  nfs_revalidate_mapping: mutex_lock(>i_mutex);


#0:

  vfs_readdir: mutex_lock(>i_mutex);
   - during the readdir (filldir64), we take a user fault (missing page?)
and call do_page_fault -
  do_page_fault:   down_read(>mmap_sem);


So it does indeed look like a circular locking. Now the question is, "is
this a bug?".  Looking like the inode of #1 must be a file or something
else that you can mmap and the inode of #0 seems it must be a directory.
I would say "no".

Now if you can readdir on a file or mmap a directory, then this could be
an issue.

Otherwise, I'd love to see someone teach lockdep about this issue! ;-)

-- Steve


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


lockdep wierdness...

2007-09-24 Thread Trond Myklebust
I'm seeing lockdep warning about a potential lock inversion between
>mmap_sem and >i_mutex in NFS (see attachment).

Unfortunately the basis for the warning appears to be the behaviour in
ext3(???). AFAICS there is no way for NFS to share an inode->i_mutex
with ext3. What to do?

Trond
===
[ INFO: possible circular locking dependency detected ]
2.6.23-rc7-g8809e921 #1
---
beagle-build-in/24375 is trying to acquire lock:
 (>mmap_sem){}, at: [] do_page_fault+0x17d/0x591

but task is already holding lock:
 (>i_mutex){--..}, at: [] mutex_lock+0x1c/0x1f

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (>i_mutex){--..}:
   [] __lock_acquire+0x9f3/0xba6
   [] lock_acquire+0x5f/0x78
   [] __mutex_lock_slowpath+0xe5/0x27a
   [] mutex_lock+0x1c/0x1f
   [] nfs_revalidate_mapping+0x64/0x9c [nfs]
   [] nfs_file_mmap+0x46/0x75 [nfs]
   [] mmap_region+0x1ea/0x3b8
   [] do_mmap_pgoff+0x27b/0x2da
   [] sys_mmap2+0x9b/0xb5
   [] sysenter_past_esp+0x5f/0x99
   [] 0x

-> #0 (>mmap_sem){}:
   [] __lock_acquire+0x8df/0xba6
   [] lock_acquire+0x5f/0x78
   [] down_read+0x3a/0x4c
   [] do_page_fault+0x17d/0x591
   [] error_code+0x72/0x78
   [] call_filldir+0xac/0xc3 [ext3]
   [] ext3_readdir+0x217/0x5e5 [ext3]
   [] vfs_readdir+0x67/0x93
   [] sys_getdents+0x5f/0x9d
   [] sysenter_past_esp+0x5f/0x99
   [] 0x

other info that might help us debug this:

1 lock held by beagle-build-in/24375:
 #0:  (>i_mutex){--..}, at: [] mutex_lock+0x1c/0x1f

stack backtrace:
 [] show_trace_log_lvl+0x1a/0x2f
 [] show_trace+0x12/0x14
 [] dump_stack+0x16/0x18
 [] print_circular_bug_tail+0x5f/0x68
 [] __lock_acquire+0x8df/0xba6
 [] lock_acquire+0x5f/0x78
 [] down_read+0x3a/0x4c
 [] do_page_fault+0x17d/0x591
 [] error_code+0x72/0x78
 [] call_filldir+0xac/0xc3 [ext3]
 [] ext3_readdir+0x217/0x5e5 [ext3]
 [] vfs_readdir+0x67/0x93
 [] sys_getdents+0x5f/0x9d
 [] sysenter_past_esp+0x5f/0x99
 ===


lockdep wierdness...

2007-09-24 Thread Trond Myklebust
I'm seeing lockdep warning about a potential lock inversion between
mm-mmap_sem and inode-i_mutex in NFS (see attachment).

Unfortunately the basis for the warning appears to be the behaviour in
ext3(???). AFAICS there is no way for NFS to share an inode-i_mutex
with ext3. What to do?

Trond
===
[ INFO: possible circular locking dependency detected ]
2.6.23-rc7-g8809e921 #1
---
beagle-build-in/24375 is trying to acquire lock:
 (mm-mmap_sem){}, at: [c05a2887] do_page_fault+0x17d/0x591

but task is already holding lock:
 (inode-i_mutex){--..}, at: [c059f9e3] mutex_lock+0x1c/0x1f

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

- #1 (inode-i_mutex){--..}:
   [c043d4da] __lock_acquire+0x9f3/0xba6
   [c043da62] lock_acquire+0x5f/0x78
   [c059f832] __mutex_lock_slowpath+0xe5/0x27a
   [c059f9e3] mutex_lock+0x1c/0x1f
   [f8c92495] nfs_revalidate_mapping+0x64/0x9c [nfs]
   [f8c8ff2a] nfs_file_mmap+0x46/0x75 [nfs]
   [c046097c] mmap_region+0x1ea/0x3b8
   [c0460e9b] do_mmap_pgoff+0x27b/0x2da
   [c0407d77] sys_mmap2+0x9b/0xb5
   [c040405e] sysenter_past_esp+0x5f/0x99
   [] 0x

- #0 (mm-mmap_sem){}:
   [c043d3c6] __lock_acquire+0x8df/0xba6
   [c043da62] lock_acquire+0x5f/0x78
   [c04360db] down_read+0x3a/0x4c
   [c05a2887] do_page_fault+0x17d/0x591
   [c05a1382] error_code+0x72/0x78
   [f88acaac] call_filldir+0xac/0xc3 [ext3]
   [f88acdb2] ext3_readdir+0x217/0x5e5 [ext3]
   [c04798a1] vfs_readdir+0x67/0x93
   [c0479af6] sys_getdents+0x5f/0x9d
   [c040405e] sysenter_past_esp+0x5f/0x99
   [] 0x

other info that might help us debug this:

1 lock held by beagle-build-in/24375:
 #0:  (inode-i_mutex){--..}, at: [c059f9e3] mutex_lock+0x1c/0x1f

stack backtrace:
 [c04050ee] show_trace_log_lvl+0x1a/0x2f
 [c0405b58] show_trace+0x12/0x14
 [c0405b70] dump_stack+0x16/0x18
 [c043bc05] print_circular_bug_tail+0x5f/0x68
 [c043d3c6] __lock_acquire+0x8df/0xba6
 [c043da62] lock_acquire+0x5f/0x78
 [c04360db] down_read+0x3a/0x4c
 [c05a2887] do_page_fault+0x17d/0x591
 [c05a1382] error_code+0x72/0x78
 [f88acaac] call_filldir+0xac/0xc3 [ext3]
 [f88acdb2] ext3_readdir+0x217/0x5e5 [ext3]
 [c04798a1] vfs_readdir+0x67/0x93
 [c0479af6] sys_getdents+0x5f/0x9d
 [c040405e] sysenter_past_esp+0x5f/0x99
 ===


Re: lockdep wierdness...

2007-09-24 Thread Steven Rostedt
On Mon, Sep 24, 2007 at 06:07:38PM -0400, Trond Myklebust wrote:
 I'm seeing lockdep warning about a potential lock inversion between
 mm-mmap_sem and inode-i_mutex in NFS (see attachment).
 
 Unfortunately the basis for the warning appears to be the behaviour in
 ext3(???). AFAICS there is no way for NFS to share an inode-i_mutex
 with ext3. What to do?

Actually this can probably happen just on NFS alone.

 
 Trond

 ===
 [ INFO: possible circular locking dependency detected ]
 2.6.23-rc7-g8809e921 #1
 ---
 beagle-build-in/24375 is trying to acquire lock:
  (mm-mmap_sem){}, at: [c05a2887] do_page_fault+0x17d/0x591
 
 but task is already holding lock:
  (inode-i_mutex){--..}, at: [c059f9e3] mutex_lock+0x1c/0x1f
 
 which lock already depends on the new lock.
 
 
 the existing dependency chain (in reverse order) is:
 
 - #1 (inode-i_mutex){--..}:
[c043d4da] __lock_acquire+0x9f3/0xba6
[c043da62] lock_acquire+0x5f/0x78
[c059f832] __mutex_lock_slowpath+0xe5/0x27a
[c059f9e3] mutex_lock+0x1c/0x1f
[f8c92495] nfs_revalidate_mapping+0x64/0x9c [nfs]
[f8c8ff2a] nfs_file_mmap+0x46/0x75 [nfs]
[c046097c] mmap_region+0x1ea/0x3b8
[c0460e9b] do_mmap_pgoff+0x27b/0x2da
[c0407d77] sys_mmap2+0x9b/0xb5
[c040405e] sysenter_past_esp+0x5f/0x99
[] 0x
 
 - #0 (mm-mmap_sem){}:
[c043d3c6] __lock_acquire+0x8df/0xba6
[c043da62] lock_acquire+0x5f/0x78
[c04360db] down_read+0x3a/0x4c
[c05a2887] do_page_fault+0x17d/0x591
[c05a1382] error_code+0x72/0x78
[f88acaac] call_filldir+0xac/0xc3 [ext3]
[f88acdb2] ext3_readdir+0x217/0x5e5 [ext3]
[c04798a1] vfs_readdir+0x67/0x93
[c0479af6] sys_getdents+0x5f/0x9d
[c040405e] sysenter_past_esp+0x5f/0x99
[] 0x

The circular lock seems to be this:

#1:

  sys_mmap2:  down_write(mm-mmap_sem);
  nfs_revalidate_mapping: mutex_lock(inode-i_mutex);


#0:

  vfs_readdir: mutex_lock(inode-i_mutex);
   - during the readdir (filldir64), we take a user fault (missing page?)
and call do_page_fault -
  do_page_fault:   down_read(mm-mmap_sem);


So it does indeed look like a circular locking. Now the question is, is
this a bug?.  Looking like the inode of #1 must be a file or something
else that you can mmap and the inode of #0 seems it must be a directory.
I would say no.

Now if you can readdir on a file or mmap a directory, then this could be
an issue.

Otherwise, I'd love to see someone teach lockdep about this issue! ;-)

-- Steve


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/