Re: [PATCH 04/16] vfs: do_last(): use inode variable

[Nick Piggin]

On 25 April 2012 22:44, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> From: Miklos Szeredi <mszeredi@xxxxxxx>
>
> Use helper variable instead of path->dentry->d_inode before complete_walk().
> This will allow this code to be used in RCU mode.

What do you mean, allow it to be used?

>
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxx>
> ---
>  fs/namei.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 46d4bf6..f21ddb3 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2360,15 +2360,16 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
>        if (error)
>                nd->flags |= LOOKUP_JUMPED;
>
> +       inode = path->dentry->d_inode;
>        error = -ENOENT;
> -       if (!path->dentry->d_inode)
> +       if (!inode)
>                goto exit_dput;
>
> -       if (path->dentry->d_inode->i_op->follow_link)
> +       if (inode->i_op->follow_link)
>                return NULL;
>
>        path_to_nameidata(path, nd);
> -       nd->inode = path->dentry->d_inode;
> +       nd->inode = inode;
>        /* Why this, you ask?  _Now_ we might have grown LOOKUP_JUMPED... */
>        error = complete_walk(nd);
>        if (error)

In rcu-walk mode, dentry->d_inode should not be accessed at all,
outside of the core lookup code that (should) have the correct
barriers and sequence locks.

That logic should not escape into here, so I'm just not sure what
you're doing here.

This code runs in rcu-walk mode, then at the very least you need
ACCESS_ONCE to load the inode, because ->d_inode can
become NULL right after you test it for NULL (but that would likely
be a bandaid, I'm just pointing out there's raft of potential
problems).
--
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/