Hi Trond,
Thank you so much for looking into this further!
I duplicated the original problem using the unpatched 2.6.12-rc6. I then
applied your patch to 2.6.12-rc6 and retested. The original problem was
fixed by your patch.
I now need to backfit your patch to the older kernel that I am working
with.
Thanks again very much for your help!
Cheers!
Linda
On Tue, 2005-06-07 at 09:41, Trond Myklebust wrote:
> må den 06.06.2005 Klokka 22:29 (-0400) skreiv Linda Dunaphant:
> > I changed the nfs_is_exclusive_create() to use the LOOKUP_PARENT
> > flag instead of the LOOKUP_CONTINUE flag. I found that LOOKUP_PARENT
> > stays set until you get to the very last pathname level, which is
> > the correct point for it to check whether it is an exclusive create.
>
> The intent information as it stands today should really only be applied
> if we're looking up the very last path component of an open()/create()
> or access() call. When we're walking the path doing lookups, that rule
> basically boils down to: never apply the intent if either
> LOOKUP_CONTINUE or LOOKUP_PARENT are set.
>
> It therefore turns out that your patch is in fact a valid fix in the
> case of nfs_is_exclusive_create() since the VFS is guaranteed to always
> call LOOKUP_PARENT first (my apologies).
> However, when hunting through the NFS lookup code, I found several other
> cases where we check for an intent and where we do need the more general
> test. To avoid confusion, I'd therefore prefer to introduce a helper
> that _always_ returns the correct intent information for the component
> that is being looked up.
>
> Could you therefore check if this patch works for you?
>
> Cheers,
> Trond
>
>
> ______________________________________________________________________
> [NFS] Fix lookup intent handling
>
> We should never apply a lookup intent to the path component of a
> LOOKUP_PARENT call.
> Introduce nd_lookup_intent() which always returns zero if LOOKUP_CONTINUE
> or LOOKUP_PARENT is set, and otherwise returns the intent flags.
>
> Problem noticed by Linda Dunaphant <[email protected]>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> dir.c | 49 +++++++++++++++++++++++++++++++++++--------------
> 1 files changed, 35 insertions(+), 14 deletions(-)
>
> Index: linux-2.6.12-rc6/fs/nfs/dir.c
> ===================================================================
> --- linux-2.6.12-rc6.orig/fs/nfs/dir.c
> +++ linux-2.6.12-rc6/fs/nfs/dir.c
> @@ -528,19 +528,39 @@ static inline void nfs_renew_times(struc
> dentry->d_time = jiffies;
> }
>
> +/*
> + * Return the intent data that applies to this particular path component
> + *
> + * Note that the current set of intents only apply to the very last
> + * component of the path.
> + * We check for this using LOOKUP_CONTINUE and LOOKUP_PARENT.
> + */
> +static inline unsigned int lookup_check_intent(struct nameidata *nd, unsigned int mask)
> +{
> + if (nd->flags & (LOOKUP_CONTINUE|LOOKUP_PARENT))
> + return 0;
> + return nd->flags & mask;
> +}
> +
> +/*
> + * Inode and filehandle revalidation for lookups.
> + *
> + * We force revalidation in the cases where the VFS sets LOOKUP_REVAL,
> + * or if the intent information indicates that we're about to open this
> + * particular file and the "nocto" mount flag is not set.
> + *
> + */
> static inline
> int nfs_lookup_verify_inode(struct inode *inode, struct nameidata *nd)
> {
> struct nfs_server *server = NFS_SERVER(inode);
>
> if (nd != NULL) {
> - int ndflags = nd->flags;
> /* VFS wants an on-the-wire revalidation */
> - if (ndflags & LOOKUP_REVAL)
> + if (nd->flags & LOOKUP_REVAL)
> goto out_force;
> /* This is an open(2) */
> - if ((ndflags & LOOKUP_OPEN) &&
> - !(ndflags & LOOKUP_CONTINUE) &&
> + if (lookup_check_intent(nd, LOOKUP_OPEN) != 0 &&
> !(server->flags & NFS_MOUNT_NOCTO))
> goto out_force;
> }
> @@ -560,12 +580,8 @@ static inline
> int nfs_neg_need_reval(struct inode *dir, struct dentry *dentry,
> struct nameidata *nd)
> {
> - int ndflags = 0;
> -
> - if (nd)
> - ndflags = nd->flags;
> /* Don't revalidate a negative dentry if we're creating a new file */
> - if ((ndflags & LOOKUP_CREATE) && !(ndflags & LOOKUP_CONTINUE))
> + if (nd != NULL && lookup_check_intent(nd, LOOKUP_CREATE) != 0)
> return 0;
> return !nfs_check_verifier(dir, dentry);
> }
> @@ -700,12 +716,16 @@ struct dentry_operations nfs_dentry_oper
> .d_iput = nfs_dentry_iput,
> };
>
> +/*
> + * Use intent information to check whether or not we're going to do
> + * an O_EXCL create using this path component.
> + */
> static inline
> int nfs_is_exclusive_create(struct inode *dir, struct nameidata *nd)
> {
> if (NFS_PROTO(dir)->version == 2)
> return 0;
> - if (!nd || (nd->flags & LOOKUP_CONTINUE) || !(nd->flags & LOOKUP_CREATE))
> + if (nd == NULL || lookup_check_intent(nd, LOOKUP_CREATE) == 0)
> return 0;
> return (nd->intent.open.flags & O_EXCL) != 0;
> }
> @@ -772,12 +792,13 @@ struct dentry_operations nfs4_dentry_ope
> .d_iput = nfs_dentry_iput,
> };
>
> +/*
> + * Use intent information to determine whether we need to substitute
> + * the NFSv4-style stateful OPEN for the LOOKUP call
> + */
> static int is_atomic_open(struct inode *dir, struct nameidata *nd)
> {
> - if (!nd)
> - return 0;
> - /* Check that we are indeed trying to open this file */
> - if ((nd->flags & LOOKUP_CONTINUE) || !(nd->flags & LOOKUP_OPEN))
> + if (nd == NULL || lookup_check_intent(nd, LOOKUP_OPEN) == 0)
> return 0;
> /* NFS does not (yet) have a stateful open for directories */
> if (nd->flags & LOOKUP_DIRECTORY)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
