Prev: linux-next: Tree for July 22
Next: linux-next: build warning after merge of the nettree
From: Nick Piggin on 22 Jul 2010 00:20
This hasn't had much comments yet.

On Tue, Jul 13, 2010 at 10:55:50PM +0100, David Howells wrote:
>
> Add a dentry op (d_automount) to handle automounting directories rather than
> abusing the follow_link() inode operation.
>
> I've only changed __follow_mount() to handle automount points, but it might be
> necessary to change follow_mount() too. The latter is only used from
> follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
> a child of it.
>
> AFS is made to use this facility so that it can be tested. Other filesystems
> abusing the follow_mount() inode operation will also need to be modified.

How about having a .follow_mount op, and using that instead of
default follow_mount in case mounted is incremented?

Also I would prefer the patch to add this call keep basically the
same API as follow_mount, so if you are going to change that to
return an error and do the NOFOLLOW handling in there, then
could you do that first, as a more trivial patch?

Then your addition of the d_op should not touch outside
*follow_mount.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nick Piggin on 22 Jul 2010 11:00
On Thu, Jul 22, 2010 at 01:36:27PM +0100, David Howells wrote:
> Nick Piggin <npiggin(a)suse.de> wrote:
>
> > > AFS is made to use this facility so that it can be tested. Other
> > > filesystems abusing the follow_mount() inode operation will also need to
> > > be modified.
>
> I meant follow_link() here of course... Too many followy things:-)
>
> > How about having a .follow_mount op, and using that instead of
> > default follow_mount in case mounted is incremented?
>
> But what if d_mounted is not incremented, though?

Nothing?


> That's usually the point
> you'd want to call the automount code.

I think you have it the wrong way around. If you wanted to call
the automount code, you would have incremented d_mounted.


> Why would you want to call into the
> filesystem just to skip over possibly mounted dentries? A dentry may have an
> elevated d_mount on it, but nothing mounted at that {vfsmount,dentry} point I
> suppose, but still jumping into the filesystem just so it can skip an already
> mounted point would seem a waste of time.

Those that don't care wouldn't set ->follow_mount though.
Following a mount is a fairly heavy operation already, it
does take a global lock (before vfs scalability patches,
anyway).

I like the flexibility of doing one's own ->follow_mount,
although Al might object to allowing filesystems to follow
mounts in ways that are not published to the core
namespace structures.


> > Also I would prefer the patch to add this call
>
> Meaning i_op->follow_mount()?

Either one, just make the follow_mount/__follow_mount API
changes in one patch, and add the callback in another.


> > keep basically the same API as follow_mount, so if you are going to change
> > that to return an error and do the NOFOLLOW handling in there, then could
> > you do that first, as a more trivial patch?
>
> Ummm... I'm not sure I follow you. I changed __follow_mount() not
> follow_mount(). I don't think changing the latter is necessary.

I meant __follow_mount.


> > Then your addition of the d_op should not touch outside *follow_mount.
>
> But calling i_op->follow_mount() would, so what does this gain you? And why
> not touch the inside of __follow_mount()?
>
> Are you suggesting doing i_op->follow_mount() instead of or as well as
> d_op->d_automount()? I'm not entirely sure.

Two suggestions. Firstly a d_op->d_follow_mount() (does following
a mount even make sense at the inode level?)

Secondly, just simply to split the patch so you change the
__follow_mount API in namespace first.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: linux-next: Tree for July 22
Next: linux-next: build warning after merge of the nettree