Return-Path: Received: from mail-yk0-f180.google.com ([209.85.160.180]:36741 "EHLO mail-yk0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752604AbbHULXs (ORCPT ); Fri, 21 Aug 2015 07:23:48 -0400 Received: by ykfw73 with SMTP id w73so66703635ykf.3 for ; Fri, 21 Aug 2015 04:23:47 -0700 (PDT) Date: Fri, 21 Aug 2015 07:23:44 -0400 From: Jeff Layton To: Peng Tao Cc: Bruce Fields , Linux NFS Mailing List , Christoph Hellwig , Kinglong Mee Subject: Re: [PATCH v3 19/20] nfsd: hook up nfs4_preprocess_stateid_op to the nfsd_file cache Message-ID: <20150821072344.25b70af6@tlielax.poochiereds.net> In-Reply-To: References: <1440069440-27454-1-git-send-email-jeff.layton@primarydata.com> <1440069440-27454-20-git-send-email-jeff.layton@primarydata.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: On Thu, 20 Aug 2015 18:28:05 -0700 Peng Tao wrote: > On Thu, Aug 20, 2015 at 4:17 AM, Jeff Layton wrote: > > Have nfs4_preprocess_stateid_op pass back a nfsd_file instead of a filp. > > Since we now presume that the struct file will be persistent in most > > cases, we can stop fiddling with the raparms in the read code. This > > also means that we don't really care about the rd_tmp_file field > > anymore. > > > > Signed-off-by: Jeff Layton > > --- > > fs/nfsd/nfs4proc.c | 32 ++++++++++++++++---------------- > > fs/nfsd/nfs4state.c | 20 +++++++------------- > > fs/nfsd/nfs4xdr.c | 16 +++++----------- > > fs/nfsd/state.h | 2 +- > > fs/nfsd/xdr4.h | 15 +++++++-------- > > 5 files changed, 36 insertions(+), 49 deletions(-) > > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > > index b9681ee0ed19..42a3f8b50814 100644 > > --- a/fs/nfsd/nfs4proc.c > > +++ b/fs/nfsd/nfs4proc.c > > @@ -758,7 +758,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > { > > __be32 status; > > > > - read->rd_filp = NULL; > > + read->rd_nf = NULL; > > if (read->rd_offset >= OFFSET_MAX) > > return nfserr_inval; > > > > @@ -775,7 +775,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > > > /* check stateid */ > > status = nfs4_preprocess_stateid_op(rqstp, cstate, &read->rd_stateid, > > - RD_STATE, &read->rd_filp, &read->rd_tmp_file); > > + RD_STATE, &read->rd_nf); > > if (status) { > > dprintk("NFSD: nfsd4_read: couldn't process stateid!\n"); > > goto out; > > @@ -921,7 +921,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > > > if (setattr->sa_iattr.ia_valid & ATTR_SIZE) { > > status = nfs4_preprocess_stateid_op(rqstp, cstate, > > - &setattr->sa_stateid, WR_STATE, NULL, NULL); > > + &setattr->sa_stateid, WR_STATE, NULL); > > if (status) { > > dprintk("NFSD: nfsd4_setattr: couldn't process stateid!\n"); > > return status; > > @@ -977,7 +977,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > struct nfsd4_write *write) > > { > > stateid_t *stateid = &write->wr_stateid; > > - struct file *filp = NULL; > > + struct nfsd_file *nf = NULL; > > __be32 status = nfs_ok; > > unsigned long cnt; > > int nvecs; > > @@ -986,7 +986,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > return nfserr_inval; > > > > status = nfs4_preprocess_stateid_op(rqstp, cstate, stateid, WR_STATE, > > - &filp, NULL); > > + &nf); > > if (status) { > > dprintk("NFSD: nfsd4_write: couldn't process stateid!\n"); > > return status; > > @@ -999,10 +999,10 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > nvecs = fill_in_write_vector(rqstp->rq_vec, write); > > WARN_ON_ONCE(nvecs > ARRAY_SIZE(rqstp->rq_vec)); > > > > - status = nfsd_vfs_write(rqstp, &cstate->current_fh, filp, > > + status = nfsd_vfs_write(rqstp, &cstate->current_fh, nf->nf_file, > > write->wr_offset, rqstp->rq_vec, nvecs, &cnt, > > &write->wr_how_written); > > - fput(filp); > > + nfsd_file_put(nf); > > > > write->wr_bytes_written = cnt; > > > > @@ -1014,21 +1014,21 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > struct nfsd4_fallocate *fallocate, int flags) > > { > > __be32 status = nfserr_notsupp; > > - struct file *file; > > + struct nfsd_file *nf; > > > > status = nfs4_preprocess_stateid_op(rqstp, cstate, > > &fallocate->falloc_stateid, > > - WR_STATE, &file, NULL); > > + WR_STATE, &nf); > > if (status != nfs_ok) { > > dprintk("NFSD: nfsd4_fallocate: couldn't process stateid!\n"); > > return status; > > } > > > > - status = nfsd4_vfs_fallocate(rqstp, &cstate->current_fh, file, > > + status = nfsd4_vfs_fallocate(rqstp, &cstate->current_fh, nf->nf_file, > > fallocate->falloc_offset, > > fallocate->falloc_length, > > flags); > > - fput(file); > > + nfsd_file_put(nf); > > return status; > > } > > > > @@ -1053,11 +1053,11 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > { > > int whence; > > __be32 status; > > - struct file *file; > > + struct nfsd_file *nf; > > > > status = nfs4_preprocess_stateid_op(rqstp, cstate, > > &seek->seek_stateid, > > - RD_STATE, &file, NULL); > > + RD_STATE, &nf); > > if (status) { > > dprintk("NFSD: nfsd4_seek: couldn't process stateid!\n"); > > return status; > > @@ -1079,14 +1079,14 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > * Note: This call does change file->f_pos, but nothing in NFSD > > * should ever file->f_pos. > > */ > > - seek->seek_pos = vfs_llseek(file, seek->seek_offset, whence); > > + seek->seek_pos = vfs_llseek(nf->nf_file, seek->seek_offset, whence); > > if (seek->seek_pos < 0) > > status = nfserrno(seek->seek_pos); > > - else if (seek->seek_pos >= i_size_read(file_inode(file))) > > + else if (seek->seek_pos >= i_size_read(file_inode(nf->nf_file))) > > seek->seek_eof = true; > > > > out: > > - fput(file); > > + nfsd_file_put(nf); > > return status; > > } > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index f8394a4cd126..c626358c2bad 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -4615,7 +4615,7 @@ nfs4_check_olstateid(struct svc_fh *fhp, struct nfs4_ol_stateid *ols, int flags) > > > > static __be32 > > nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s, > > - struct file **filpp, bool *tmp_file, int flags) > > + struct nfsd_file **nfp, int flags) > > { > > int acc = (flags & RD_STATE) ? NFSD_MAY_READ : NFSD_MAY_WRITE; > > struct nfsd_file *nf; > > @@ -4631,14 +4631,10 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s, > > status = nfsd_file_acquire(rqstp, fhp, acc, &nf); > > if (status) > > return status; > > - > > - if (tmp_file) > > - *tmp_file = true; > > } > > > > - *filpp = get_file(nf->nf_file); > > + *nfp = nf; > > out: > > - nfsd_file_put(nf); > If nfsd_permission() fails, nf is leaked. Previous patch has: > > @@ -4614,21 +4618,17 @@ nfs4_check_file(struct svc_rqst *rqstp, struct > svc_fh *fhp, struct nfs4_stid *s, > struct file **filpp, bool *tmp_file, int flags) > { > int acc = (flags & RD_STATE) ? NFSD_MAY_READ : NFSD_MAY_WRITE; > - struct file *file; > + struct nfsd_file *nf; > __be32 status; > > - file = nfs4_find_file(s, flags); > - if (file) { > + nf = nfs4_find_file(s, flags); > + if (nf) { > status = nfsd_permission(rqstp, fhp->fh_export, fhp->fh_dentry, > acc | NFSD_MAY_OWNER_OVERRIDE); > - if (status) { > - fput(file); > - return status; > - } > - > - *filpp = file; > + if (status) > + goto out; > } else { > - status = nfsd_open(rqstp, fhp, S_IFREG, acc, filpp); > + status = nfsd_file_acquire(rqstp, fhp, acc, &nf); > if (status) > return status; > > Cheers, > Tao > Good catch. I'll fix that. Thanks, Jeff > > return status; > > } > > > > @@ -4648,7 +4644,7 @@ out: > > __be32 > > nfs4_preprocess_stateid_op(struct svc_rqst *rqstp, > > struct nfsd4_compound_state *cstate, stateid_t *stateid, > > - int flags, struct file **filpp, bool *tmp_file) > > + int flags, struct nfsd_file **nfp) > > { > > struct svc_fh *fhp = &cstate->current_fh; > > struct inode *ino = d_inode(fhp->fh_dentry); > > @@ -4657,10 +4653,8 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp, > > struct nfs4_stid *s = NULL; > > __be32 status; > > > > - if (filpp) > > - *filpp = NULL; > > - if (tmp_file) > > - *tmp_file = false; > > + if (nfp) > > + *nfp = NULL; > > > > if (grace_disallows_io(net, ino)) > > return nfserr_grace; > > @@ -4697,8 +4691,8 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp, > > status = nfs4_check_fh(fhp, s); > > > > done: > > - if (!status && filpp) > > - status = nfs4_check_file(rqstp, fhp, s, filpp, tmp_file, flags); > > + if (status == nfs_ok && nfp) > > + status = nfs4_check_file(rqstp, fhp, s, nfp, flags); > > out: > > if (s) > > nfs4_put_stid(s); > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > > index 75e0563c09d1..7e25a31f8e60 100644 > > --- a/fs/nfsd/nfs4xdr.c > > +++ b/fs/nfsd/nfs4xdr.c > > @@ -49,6 +49,7 @@ > > #include "cache.h" > > #include "netns.h" > > #include "pnfs.h" > > +#include "filecache.h" > > > > #ifdef CONFIG_NFSD_V4_SECURITY_LABEL > > #include > > @@ -3418,14 +3419,14 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr, > > { > > unsigned long maxcount; > > struct xdr_stream *xdr = &resp->xdr; > > - struct file *file = read->rd_filp; > > + struct file *file; > > int starting_len = xdr->buf->len; > > - struct raparms *ra = NULL; > > __be32 *p; > > > > if (nfserr) > > goto out; > > > > + file = read->rd_nf->nf_file; > > p = xdr_reserve_space(xdr, 8); /* eof flag and byte count */ > > if (!p) { > > WARN_ON_ONCE(test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags)); > > @@ -3445,24 +3446,17 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr, > > (xdr->buf->buflen - xdr->buf->len)); > > maxcount = min_t(unsigned long, maxcount, read->rd_length); > > > > - if (read->rd_tmp_file) > > - ra = nfsd_init_raparms(file); > > - > > if (file->f_op->splice_read && > > test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags)) > > nfserr = nfsd4_encode_splice_read(resp, read, file, maxcount); > > else > > nfserr = nfsd4_encode_readv(resp, read, file, maxcount); > > > > - if (ra) > > - nfsd_put_raparams(file, ra); > > - > > if (nfserr) > > xdr_truncate_encode(xdr, starting_len); > > - > > out: > > - if (file) > > - fput(file); > > + if (read->rd_nf) > > + nfsd_file_put(read->rd_nf); > > return nfserr; > > } > > > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > > index 8a317de773b9..cf7e27199507 100644 > > --- a/fs/nfsd/state.h > > +++ b/fs/nfsd/state.h > > @@ -585,7 +585,7 @@ struct nfsd_net; > > > > extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp, > > struct nfsd4_compound_state *cstate, stateid_t *stateid, > > - int flags, struct file **filp, bool *tmp_file); > > + int flags, struct nfsd_file **filp); > > __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate, > > stateid_t *stateid, unsigned char typemask, > > struct nfs4_stid **s, struct nfsd_net *nn); > > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h > > index 9f991007a578..ea016fb24675 100644 > > --- a/fs/nfsd/xdr4.h > > +++ b/fs/nfsd/xdr4.h > > @@ -268,15 +268,14 @@ struct nfsd4_open_downgrade { > > > > > > struct nfsd4_read { > > - stateid_t rd_stateid; /* request */ > > - u64 rd_offset; /* request */ > > - u32 rd_length; /* request */ > > - int rd_vlen; > > - struct file *rd_filp; > > - bool rd_tmp_file; > > + stateid_t rd_stateid; /* request */ > > + u64 rd_offset; /* request */ > > + u32 rd_length; /* request */ > > + int rd_vlen; > > + struct nfsd_file *rd_nf; > > > > - struct svc_rqst *rd_rqstp; /* response */ > > - struct svc_fh * rd_fhp; /* response */ > > + struct svc_rqst *rd_rqstp; /* response */ > > + struct svc_fh *rd_fhp; /* response */ > > }; > > > > struct nfsd4_readdir { > > -- > > 2.4.3 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Jeff Layton