Re: VFS locking & HFS problems (2.4.6pre6)

Re: VFS locking & HFS problems (2.4.6pre6)

[Benjamin Herrenschmidt]

Alan Cox wrote:

>Holding a spinlock while sleeping is an offence punishable by deadlock..

Right, and it's indeed the problem. But I'm still concerned about
locking since by using that spinlock, the guy who wrote it did
not expect beeing re-entered at this point, and just "cleaning" it
may not be enough.

>You might also look for memory allocations that are not GFP_ATOMIC made with
>the lock held

Yup. It's the problem. It locks, then calls some alloc routines, which
fills a cache and uses kmalloc with GFP_KERNEL.

Turning it into GFP_ATOMIC might not be the best idea as the HFS
filesystem currently shares a single hfs_malloc() for everybody and
turning it into GFP_ATOMIC would cause all of HFS allocs to be atomic.

I can change this single routine (and any other doing the same thing),
but  I'd rather fix it by making sure HFS can safely sleep at this
point and still use GFP_KERNEL.

I just found Documentations/filesystems/Locking document, I bet I'll
find all the infos I need there. It's amazing how long it took me
to look for the info where it logically should be ;)

Ben.

Re: VFS locking & HFS problems (2.4.6pre6)

[Alan Cox]

> Yup. It's the problem. It locks, then calls some alloc routines, which
> fills a cache and uses kmalloc with GFP_KERNEL.

Thats quite common. If it can safely sleep there without other deadlocks a
semaphore may be better

> Turning it into GFP_ATOMIC might not be the best idea as the HFS
> filesystem currently shares a single hfs_malloc() for everybody and
> turning it into GFP_ATOMIC would cause all of HFS allocs to be atomic.

Another approach is to prealloc the object and pass it in, then free it
if not needed

VFS locking & HFS problems (2.4.6pre6)

[Benjamin Herrenschmidt]

I've had a deadlock twice with 2.4.6pre6 today. It's an SMP kernel
running on an UP box (a PowerBook Pismo).

The deadlock happen in the HFS filesystem in hfs_cat_put(), apparently
(quickly looking at addresses) in spin_lock().

I don't have the complete backtrace at hand right now, but it basically
went up to kswapd without anything evidently getting that spinlock,
I'll try to gather more details.

So my question: Is there any document explaining the various locking
requirements & re-entrency possibilities in a filesystem.

What I think might happen after a quick look is that HFS may be causing
schedule() to be called while holding the spinlock, and gets then
re-entered from another process context. I have to look at it in more
detail (is there an HFS maintainer ?) but some background informations
on VFS locking & reentrancy issues would be helpful.

Ben.

Re: VFS locking & HFS problems (2.4.6pre6)

[Andrew Morton]

Benjamin Herrenschmidt wrote:
> 
> I've had a deadlock twice with 2.4.6pre6 today. It's an SMP kernel
> running on an UP box (a PowerBook Pismo).
> 
> The deadlock happen in the HFS filesystem in hfs_cat_put(), apparently
> (quickly looking at addresses) in spin_lock().
> 

Please test this:

Index: fs/hfs/catalog.c
===================================================================
RCS file: /opt/cvs/lk/fs/hfs/catalog.c,v
retrieving revision 1.2
diff -u -r1.2 catalog.c
--- fs/hfs/catalog.c	2001/02/17 01:44:39	1.2
+++ fs/hfs/catalog.c	2001/06/29 15:54:17
@@ -549,7 +549,7 @@
 		   entry->state &= ~HFS_LOCK;
 		   hfs_wake_up(&entry->wait);
 		}
-
+		spin_lock(&entry_lock);
 		return entry;
 	}
 
@@ -559,7 +559,6 @@
 	if (grow_entries())
 	        goto add_new_entry;
 
-	spin_unlock(&entry_lock);
 	return NULL;
 
 read_fail: 
@@ -570,7 +569,6 @@
 	init_entry(entry);
 	list_add(&entry->list, &entry_unused);
 	entries_stat.nr_free_entries++;
-	spin_unlock(&entry_lock);
 	return NULL;
 }

Re: VFS locking & HFS problems (2.4.6pre6)

[Alexander Viro]

On Fri, 29 Jun 2001, Benjamin Herrenschmidt wrote:

> The deadlock happen in the HFS filesystem in hfs_cat_put(), apparently
> (quickly looking at addresses) in spin_lock().

<looks>
Uh-oh. Looks like hfs_cat_put() grabs some internal spinlock and calls
write_entry(). If it really is what its name implies, you are calling
a blocking function under the spinlock.

> So my question: Is there any document explaining the various locking
> requirements & re-entrency possibilities in a filesystem.

There is, but this bug has nothing fs-specific in it. You should never
block while holding a spinlock.

BTW, looks like 2.2 has the same bug.