Re: Some questions about memory allocation and BKL

[Jan Hudec <bulb@ucw.cz>]

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 <bulb@ucw.cz>