Re: [autofs] Re: [PATCH] autofs4 needs to force fail returnrevalidate

From: Ian Kent
Date: Mon Jul 10 2006 - 12:22:17 EST


On Mon, 2006-07-10 at 03:24 -0700, Andrew Morton wrote:
> btw, this patch is presently in a not-going-anywhere state because Al
> expressed some reservations. But then it all went quiet?

Ya. Thought that might be the case.
This is in a sensitive place in the VFS.

Al, please your swift and sure guidance would be appreciated.

>
>
> From: Ian Kent <raven@xxxxxxxxxx>
>
> For a long time now I have had a problem with not being able to return a
> lookup failure on an existsing directory. In autofs this corresponds to a
> mount failure on a autofs managed mount entry that is browsable (and so the
> mount point directory exists).
>
> While this problem has been present for a long time I've avoided resolving
> it because it was not very visible. But now that autofs v5 has "mount and
> expire on demand" of nested multiple mounts, such as is found when mounting
> an export list from a server, solving the problem cannot be avoided any
> longer.
>
> I've tried very hard to find a way to do this entirely within the autofs4
> module but have not been able to find a satisfactory way to achieve it.
>
> So, I need to propose a change to the VFS.
>
> Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
> ---
>
> fs/autofs4/root.c | 38 ++++++++++++++++++++++++++-------
> fs/namei.c | 50 ++++++++++++++++++++++++++++++--------------
> linux/dcache.h | 0
> 3 files changed, 65 insertions(+), 23 deletions(-)
>
> diff -puN fs/autofs4/root.c~autofs4-needs-to-force-fail-return-revalidate fs/autofs4/root.c
> --- a/fs/autofs4/root.c~autofs4-needs-to-force-fail-return-revalidate
> +++ a/fs/autofs4/root.c
> @@ -137,7 +137,9 @@ static int autofs4_dir_open(struct inode
> nd.flags = LOOKUP_DIRECTORY;
> ret = (dentry->d_op->d_revalidate)(dentry, &nd);
>
> - if (!ret) {
> + if (ret <= 0) {
> + if (ret < 0)
> + status = ret;
> dcache_dir_close(inode, file);
> goto out;
> }
> @@ -400,13 +402,23 @@ static int autofs4_revalidate(struct den
> struct autofs_sb_info *sbi = autofs4_sbi(dir->i_sb);
> int oz_mode = autofs4_oz_mode(sbi);
> int flags = nd ? nd->flags : 0;
> - int status = 0;
> + int status = 1;
>
> /* Pending dentry */
> if (autofs4_ispending(dentry)) {
> - if (!oz_mode)
> - status = try_to_fill_dentry(dentry, flags);
> - return !status;
> + /* The daemon never causes a mount to trigger */
> + if (oz_mode)
> + return 1;
> +
> + /*
> + * A zero status is success otherwise we have a
> + * negative error code.
> + */
> + status = try_to_fill_dentry(dentry, flags);
> + if (status == 0)
> + return 1;
> +
> + return status;
> }
>
> /* Negative dentry.. invalidate if "old" */
> @@ -421,9 +433,19 @@ static int autofs4_revalidate(struct den
> DPRINTK("dentry=%p %.*s, emptydir",
> dentry, dentry->d_name.len, dentry->d_name.name);
> spin_unlock(&dcache_lock);
> - if (!oz_mode)
> - status = try_to_fill_dentry(dentry, flags);
> - return !status;
> + /* The daemon never causes a mount to trigger */
> + if (oz_mode)
> + return 1;
> +
> + /*
> + * A zero status is success otherwise we have a
> + * negative error code.
> + */
> + status = try_to_fill_dentry(dentry, flags);
> + if (status == 0)
> + return 1;
> +
> + return status;
> }
> spin_unlock(&dcache_lock);
>
> diff -puN fs/namei.c~autofs4-needs-to-force-fail-return-revalidate fs/namei.c
> --- a/fs/namei.c~autofs4-needs-to-force-fail-return-revalidate
> +++ a/fs/namei.c
> @@ -365,6 +365,30 @@ void release_open_intent(struct nameidat
> fput(nd->intent.open.file);
> }
>
> +static inline struct dentry *
> +do_revalidate(struct dentry *dentry, struct nameidata *nd)
> +{
> + int status = dentry->d_op->d_revalidate(dentry, nd);
> + if (unlikely(status <= 0)) {
> + /*
> + * The dentry failed validation.
> + * If d_revalidate returned 0 attempt to invalidate
> + * the dentry otherwise d_revalidate is asking us
> + * to return a fail status.
> + */
> + if (!status) {
> + if (!d_invalidate(dentry)) {
> + dput(dentry);
> + dentry = NULL;
> + }
> + } else {
> + dput(dentry);
> + dentry = ERR_PTR(status);
> + }
> + }
> + return dentry;
> +}
> +
> /*
> * Internal lookup() using the new generic dcache.
> * SMP-safe
> @@ -379,12 +403,9 @@ static struct dentry * cached_lookup(str
> if (!dentry)
> dentry = d_lookup(parent, name);
>
> - if (dentry && dentry->d_op && dentry->d_op->d_revalidate) {
> - if (!dentry->d_op->d_revalidate(dentry, nd) && !d_invalidate(dentry)) {
> - dput(dentry);
> - dentry = NULL;
> - }
> - }
> + if (dentry && dentry->d_op && dentry->d_op->d_revalidate)
> + dentry = do_revalidate(dentry, nd);
> +
> return dentry;
> }
>
> @@ -477,10 +498,9 @@ static struct dentry * real_lookup(struc
> */
> mutex_unlock(&dir->i_mutex);
> if (result->d_op && result->d_op->d_revalidate) {
> - if (!result->d_op->d_revalidate(result, nd) && !d_invalidate(result)) {
> - dput(result);
> + result = do_revalidate(result, nd);
> + if (!result)
> result = ERR_PTR(-ENOENT);
> - }
> }
> return result;
> }
> @@ -760,12 +780,12 @@ need_lookup:
> goto done;
>
> need_revalidate:
> - if (dentry->d_op->d_revalidate(dentry, nd))
> - goto done;
> - if (d_invalidate(dentry))
> - goto done;
> - dput(dentry);
> - goto need_lookup;
> + dentry = do_revalidate(dentry, nd);
> + if (!dentry)
> + goto need_lookup;
> + if (IS_ERR(dentry))
> + goto fail;
> + goto done;
>
> fail:
> return PTR_ERR(dentry);
> diff -puN include/linux/dcache.h~autofs4-needs-to-force-fail-return-revalidate include/linux/dcache.h
> _
>
> _______________________________________________
> autofs mailing list
> autofs@xxxxxxxxxxxxxxxx
> http://linux.kernel.org/mailman/listinfo/autofs
-
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/