From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Hudec Subject: Re: Some questions about memory allocation and BKL Date: Sun, 26 Oct 2003 10:44:46 +0100 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <20031026094446.GI1465@vagabond> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel Return-path: Received: from cimice4.lam.cz ([212.71.168.94]:2790 "EHLO vagabond.light.src") by vger.kernel.org with ESMTP id S262991AbTJZJo7 (ORCPT ); Sun, 26 Oct 2003 04:44:59 -0500 To: Ian Kent Content-Disposition: inline In-Reply-To: List-Id: linux-fsdevel.vger.kernel.org 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