Re: Some questions about memory allocation and BKL

[Jan Hudec <bulb@ucw.cz>]

On Sun, Oct 26, 2003 at 12:42:01 +0800, Ian Kent wrote:
> The BKL is widely used in both the autofs and autofs4 modules. Both of 
> these modules call kmalloc under the BKL. Since the BKL is a spinlock and 
> kmalloc can sleep this is bad. 

BKL is not a (normal) spinlock. It is a magic lock that makes UP from
SMP. You can sleep under BKL -- it is magicaly unlocked when you
schedule() and is locked again before schedule() returns.

However, use of BKL should be avoided.

> Worse, I have used kmalloc with GFP_ATOMIC 
> inside the dcache_lock, which I suspect may be failing on after about a 
> week of heavy activity.

That's a different story -- atomic kmalloc might fail under heavy
activity. You can allocate the buffer before you take the spinlock and
release it if something fails during work under the lock.

> This has lead me to the following questions. The 
> situation here is that the memory is used to communicate requests to the 
> userspace daemon on behalf of processes.

If the allocation unit is at least a page, it might be better to use
vmalloc.

> 1) Since the BKL is so widely used is it really worth eliminating it from 
>    code that you work on?

Eliminating BKL is always good. BKL is a relict from times when kernel
was not SMP.

> 2) How should smallish memory chunks be allocated if under the BKL if at 
>    all? How else should it be done? What about other spinlocks like 
>    dcache_lock?

BKL does not matter. It's only efect is that it kills performance on
SMP. You CAN sleep under BKL.

Under spinlock, you need allocate with GFP_ATOMIC flag. That can
unfortunately fail too often, so you need to handle that case.

So you either allocate before you take the spinlock, or take the
spinlock, try to allocate and if it fails, drop it, allocate and restart
the operation.

> 3) How do I get hold of the vfsmount struct that corresponds to a dentry 
>    from within revalidate and lookup calls?

Perhaps try looking at how .. is followed in lookup (follow_dotdot or
some such).

> 4) How does one decide when locking is actually needed? For example, what 
>    is the 'usual' locking need for the dentry release operation?

I wonder if the Documentation/filesystems/Locking file is up to date...

Generaly speaking, operations on lists (dentry hash, dentry tree, inode
hash) are protected with respective spinlocks (dcache_lock, inode_lock).

Dentries shouldn't be modified once constructed (at least not directly
-- helper functions take enough care). Reading them is only protected by
properly holding a reference.

When modifying an inode, you need to hold it's semaphore. Renames and
removes have special additional locking (not of interest outside the
two).

To your example, if you mean d_release dentry operation, that one needs
no locking at all. It gets a last reference and the dentry is already
unhashed, so noone else can get it. 

Generaly in methods (functions called via *_operations structs), you
already have some locks and usualy they are sufficient.
It is described in Documentation/filesystems/Locking and in .../vfs.txt

-------------------------------------------------------------------------------
						 Jan 'Bulb' Hudec <bulb@ucw.cz>