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 13:43:36 +0100 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <20031026124336.GK1465@vagabond> References: <20031026094446.GI1465@vagabond> 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]:29158 "EHLO vagabond.light.src") by vger.kernel.org with ESMTP id S263083AbTJZMnu (ORCPT ); Sun, 26 Oct 2003 07:43:50 -0500 To: Ian Kent Content-Disposition: inline In-Reply-To: List-Id: linux-fsdevel.vger.kernel.org On Sun, Oct 26, 2003 at 19:27:12 +0800, Ian Kent wrote: > On Sun, 26 Oct 2003, Jan Hudec wrote: > > However, use of BKL should be avoided. > Will do. I see that in 2.6 the lock_kernel has mostly been pushed into the > autofs4 module, probably to maintain the status quo, rather than cause > it's needed. Yes. It's just because noone has checked that locking inside autofs4 is correct. > > > 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). > > Been there, will look again. > > I seem to remember that the path lookup has a nameidata struct with this > information in it. That is not available when the filesystem inode lookup > and the dentry revalidate operations are called. I can't think of a way to > get hold of it. Basically, I need to calculate the path from a dentry to > the root of it's mounted filesystem. I currently use for loops and walk up > the d_parent links, but think it would be better to use d_path. Maybe the > existing way is the way to continue to to do it. In 2.6, nameidata should be available to ->lookup method. Just looked up on lxr.linux.no -- ->d_revalidate has them available too. So for 2.6 you can look there; for older kernels a loop over ->d_parent probably has to suffice. > > > > > 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... > > I've checked the kernel docs but I think I missed that. I'll have a look. Documentation/filesystems/Locking and Documentation/filesystems/vfs.txt are exact paths in kernel tree in 2.4.x. lxr.linux.no says, that they exist also in 2.6.0-test* with the same name. There is also Documentation/filesystems/porting in 2.6.0-test*, which sumarises what changed since 2.4. > > Generaly speaking, operations on lists (dentry hash, dentry tree, inode > > hash) are protected with respective spinlocks (dcache_lock, inode_lock). > > I see that. I should know that and I guess I do but the obvious is never > clear to me. I'll just have to keep asking and being told and reading. One > day, soon I hope it will sink in. > > > 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. > > This is a source of confusion for me. > > So should I always hold the dcache_lock when testing a dentry, even for > such simple things as d_mountpoint or dentry->d_inode == NULL? No. It is forbiden to turn dentry to negative one unless you are the only holder (it has refcount 1). See d_delete(). Semanticaly, dentries never change. So once they are filled and published, you can access them without any locks. However, you must have a counted reference. Technicaly, if the dentry has refcount 1, it is reused. But that is just a shortcut for droping it (it would be immediately freed) and reallocating it. It can never cross your path as long as you refcount correctly. > I expect that I need to hold the lock for compound tests to be valid. No. You only need to hold a reference. You only need to hold the lock when you check whether the dentry is hashed. And you need to use the atomic_* functions to access the refcount. > > When modifying an inode, you need to hold it's semaphore. Renames and > > removes have special additional locking (not of interest outside the > > two). > > I see that the inode semaphore is taken by the readdir call. Yes. It serializes against creating new entries. > I have used it to serialise access to an info struct attached to > the corresponding dentry in the supporting open and close > directory operationss. Is this a semsible approach. I realise that > I need to be carefull that I don't trigger a call that also needs it. Or > do you think I should declare my own semaphore within the info struct. There is the "Locking" file, that tells you what state do various methods expect the locks to be. As long as you can satisfy that expectations, it's a sensible approach. Note: Make sure you understand locking scheme of d_release and d_iput. It can get nasty if your info is accessible by other means than from the dentry. ------------------------------------------------------------------------------- Jan 'Bulb' Hudec