Re: [PATCH] Fix __d_path for lazy unmounts

[Serge E. Hallyn]

Quoting Andrew Morton (akpm@xxxxxxxxxxxxxxxxxxxx):
> On Sat, 20 Feb 2010 04:27:38 -0800 john.johansen@xxxxxxxxxxxxx wrote:
> 
> > From: John Johansen <john.johansen@xxxxxxxxxxxxx>
> > 
> > When __d_path() hits a lazily unmounted mount point, it tries to prepend
> > the name of the lazily unmounted dentry to the path name.  It gets this wrong,
> > and also overwrites the slash that separates the name from the following
> > pathname component. This patch fixes that; if a process was in directory
> > /foo/bar and /foo got lazily unmounted, the old result was ``foobar'' (note the
> > missing slash), while the new result with this patch is ``/foo/bar''.
> > 
> > Signed-off-by: John Johansen <john.johansen@xxxxxxxxxxxxx>
> > ---
> >  fs/dcache.c |   27 +++++++++++++++++++++++----
> >  1 files changed, 23 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/dcache.c b/fs/dcache.c
> > index 953173a..df49666 100644
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -1922,11 +1922,9 @@ char *__d_path(const struct path *path, struct path *root,
> >  	retval = end-1;
> >  	*retval = '/';
> >  
> > -	for (;;) {
> > +	while(dentry != root->dentry || vfsmnt != root->mnt) {
> 
> Please put a space between the `while' and the `('.
> 
> >  		struct dentry * parent;
> >  
> > -		if (dentry == root->dentry && vfsmnt == root->mnt)
> > -			break;
> >  		if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
> >  			/* Global root? */
> >  			if (vfsmnt->mnt_parent == vfsmnt) {
> > @@ -1950,9 +1948,30 @@ out:
> >  	return retval;
> >  
> >  global_root:
> > -	retval += 1;	/* hit the slash */
> > +	/*
> > +	 * We went past the (vfsmount, dentry) we were looking for and have
> > +	 * either hit a root dentry, a lazily unmounted dentry, an
> > +	 * unconnected dentry, or the file is on a pseudo filesystem.
> > +	 */
> > +	if ((dentry->d_sb->s_flags & MS_NOUSER) ||
> > +	    (dentry->d_name.len = 1 && *dentry->d_name.name == '/')) {
> 
> Did you really mean to assign 1 to dentry->d_name.len here?  Was `=='
> intended?  I hope so, because modifying the dentry in d_path() would be odd.
> 
> If this was a mistake then why did the patch pass testing?

Jinkeys!  Well I guess it must've passed testing bc that will only be
done for MS_NOUSER filesystems, and we won't later be doing any 'ls'
so the d_name.len isn't really used?

> > +		/*
> > +		 * Historically, we also glue together the root dentry and
> > +		 * remaining name for pseudo filesystems like pipefs, which
> > +		 * have the MS_NOUSER flag set. This results in pathnames
> > +		 * like "pipe:[439336]".
> > +		 */
> > +		retval += 1;	/* overwrite the slash */
> > +		buflen++;
> > +	}
> >  	if (prepend_name(&retval, &buflen, &dentry->d_name) != 0)
> >  		goto Elong;
> > +
> > +	/* connect lazily unmounted mount point */
> > +	if (*retval != '/' && !(dentry->d_sb->s_flags & MS_NOUSER) &&
> > +	    prepend(&retval, &buflen, "/", 1) != 0)
> > +		goto Elong;
> > +
> >  	root->mnt = vfsmnt;
> >  	root->dentry = dentry;
> >  	goto out;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/