On Mon, 28 Dec 2015 11:27:59 -0500
Benjamin Coddington <bcodding@xxxxxxxxxx> wrote:
> We only use the file struct pointer to obtain the nfs_open_context (or the
> inode for v2/v3), so just pass that instead. This allows us to use the
> lock procedure without a reference to a struct file which may not be
> available while releasing locks after a file close.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
Hmm...
So what pins the nfs_open_context in place after the filp is gone?
AFAICT, that gets zapped when the struct file goes away. Is there any
chance that the ctx pointers you're passing around here might outlive
the structure to which they point?
> ---
> fs/nfs/file.c | 9 ++++++---
> fs/nfs/nfs3proc.c | 4 ++--
> fs/nfs/nfs4proc.c | 4 +---
> fs/nfs/proc.c | 4 ++--
> include/linux/nfs_xdr.h | 2 +-
> 5 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 1e4804b..5dec0fb 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -711,6 +711,7 @@ static int
> do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
> {
> struct inode *inode = filp->f_mapping->host;
> + struct nfs_open_context *ctx = nfs_file_open_context(filp);
> int status = 0;
> unsigned int saved_type = fl->fl_type;
>
> @@ -728,7 +729,7 @@ do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
> if (is_local)
> goto out_noconflict;
>
> - status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> + status = NFS_PROTO(inode)->lock(ctx, cmd, fl);
> out:
> return status;
> out_noconflict:
> @@ -745,6 +746,7 @@ static int
> do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
> {
> struct inode *inode = filp->f_mapping->host;
> + struct nfs_open_context *ctx = nfs_file_open_context(filp);
> struct nfs_lock_context *l_ctx;
> int status;
>
> @@ -771,7 +773,7 @@ do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
> * "-olocal_lock="
> */
> if (!is_local)
> - status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> + status = NFS_PROTO(inode)->lock(ctx, cmd, fl);
> else
> status = do_vfs_lock(filp, fl);
> return status;
> @@ -786,6 +788,7 @@ static int
> do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
> {
> struct inode *inode = filp->f_mapping->host;
> + struct nfs_open_context *ctx = nfs_file_open_context(filp);
> int status;
>
> /*
> @@ -801,7 +804,7 @@ do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
> * "-olocal_lock="
> */
> if (!is_local)
> - status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> + status = NFS_PROTO(inode)->lock(ctx, cmd, fl);
> else
> status = do_vfs_lock(filp, fl);
> if (status < 0)
> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> index cb28cce..a10e147 100644
> --- a/fs/nfs/nfs3proc.c
> +++ b/fs/nfs/nfs3proc.c
> @@ -866,9 +866,9 @@ static void nfs3_proc_commit_setup(struct nfs_commit_data *data, struct rpc_mess
> }
>
> static int
> -nfs3_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
> +nfs3_proc_lock(struct nfs_open_context *ctx, int cmd, struct file_lock *fl)
> {
> - struct inode *inode = file_inode(filp);
> + struct inode *inode = d_inode(ctx->dentry);
>
> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
> }
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 05ea1e1..784ba4e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -6097,15 +6097,13 @@ static int nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *
> }
>
> static int
> -nfs4_proc_lock(struct file *filp, int cmd, struct file_lock *request)
> +nfs4_proc_lock(struct nfs_open_context *ctx, int cmd, struct file_lock *request)
> {
> - struct nfs_open_context *ctx;
> struct nfs4_state *state;
> unsigned long timeout = NFS4_LOCK_MINTIMEOUT;
> int status;
>
> /* verify open state */
> - ctx = nfs_file_open_context(filp);
> state = ctx->state;
>
> if (IS_GETLK(cmd)) {
> diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
> index b417bbc..185deb9 100644
> --- a/fs/nfs/proc.c
> +++ b/fs/nfs/proc.c
> @@ -634,9 +634,9 @@ nfs_proc_commit_setup(struct nfs_commit_data *data, struct rpc_message *msg)
> }
>
> static int
> -nfs_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
> +nfs_proc_lock(struct nfs_open_context *ctx, int cmd, struct file_lock *fl)
> {
> - struct inode *inode = file_inode(filp);
> + struct inode *inode = d_inode(ctx->dentry);
>
> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
> }
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 11bbae4..58c4682 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1555,7 +1555,7 @@ struct nfs_rpc_ops {
> void (*commit_setup) (struct nfs_commit_data *, struct rpc_message *);
> void (*commit_rpc_prepare)(struct rpc_task *, struct nfs_commit_data *);
> int (*commit_done) (struct rpc_task *, struct nfs_commit_data *);
> - int (*lock)(struct file *, int, struct file_lock *);
> + int (*lock)(struct nfs_open_context *, int, struct file_lock *);
> int (*lock_check_bounds)(const struct file_lock *);
> void (*clear_acl_cache)(struct inode *);
> void (*close_context)(struct nfs_open_context *ctx, int);
--
Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
