[OpenAFS-devel] Linux 2.6.12 kernel BUG at fs/namei.c:1189

chas williams - CONTRACTOR chas@cmf.nrl.navy.mil
Tue, 31 Jan 2006 10:41:49 -0500


In message <43DF236D.7010200@pclella.cern.ch>,Rainer Toebbicke writes:
>I'm admittedly no kernel freak but from what I read in slab.c and 
>page_alloc.c GFP_NOFS does *not* protect you from sleeping and waiting 
>for kswapd when you run out of free pages. Nor does GFP_KERNEL for 
>that matter. And then you're in trouble if you hold the global lock as 
>we know already that through put_inode kswapd might re-enter AFS and 
>deadlock.

i am not an expert by any means, but as i understand things, if you use
GFP_NOFS, then kswapd isnt going to call into the filesystem to try to
get memory.  this is why i would like to see som call traces from your
deadlock.  i can't see why the allocator would be entering the filesystem.

>You've got a point however in that osi_linux_alloc() is obviously 
>prepared to drop the global lock - however this is only attempted 
>*after* a kmalloc() failed!

actually, i dont think its a good idea for osi_linux_alloc() to
drop the lock.  the other implementation dont seem to need this
but linux is different so perhaps it is necessary.  always dropping
the lock though can lead to races in the afs code since some of
it is written assuming that AFS_GLOCK does not need to be dropped.

>Or use GFP_ATOMIC if we hold the global lock and GFP_NOFS otherwise - 
>is that starting to become a matter of taste? What do the "penguins" say?

maybe.  as i said before i would like to see some traces from the
deadlock.

>+++ openafs/src/afs/LINUX/osi_alloc.c	2006-01-31 09:30:07.000000000 +0100
>@@ -87,6 +87,7 @@
> 
>     /*  if we can use kmalloc use it to allocate the required memory. */
>     while (!new && max_retry) {
>+	if (drop_glock && haveGlock) AFS_GUNLOCK();	/* kmalloc may sleep */
> 	if (asize <= MAX_KMALLOC_SIZE) {
> 	    new = (void *)(unsigned long)kmalloc(asize,
> #ifdef GFP_NOFS
>@@ -102,6 +103,7 @@
> 	    if (new)		/* piggy back alloc type */
> 		new = (void *)(VM_TYPE | (unsigned long)new);
> 	}
>+	if (drop_glock && haveGlock) AFS_GUNLOCK();

AFS_GLOCK() here?