Return-Path: Received: from fieldses.org ([173.255.197.46]:38487 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752886AbbCQT4e (ORCPT ); Tue, 17 Mar 2015 15:56:34 -0400 Date: Tue, 17 Mar 2015 15:56:33 -0400 From: "J. Bruce Fields" To: Anna Schumaker Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH v3 3/3] NFSD: Add support for encoding multiple segments Message-ID: <20150317195633.GC29843@fieldses.org> References: <1426540688-32095-1-git-send-email-Anna.Schumaker@Netapp.com> <1426540688-32095-4-git-send-email-Anna.Schumaker@Netapp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1426540688-32095-4-git-send-email-Anna.Schumaker@Netapp.com> Sender: linux-nfs-owner@vger.kernel.org List-ID: On Mon, Mar 16, 2015 at 05:18:08PM -0400, Anna Schumaker wrote: > This patch implements sending an array of segments back to the client. > Clients should be prepared to handle multiple segment reads to make this > useful. We try to splice the first data segment into the XDR result, > and remaining segments are encoded directly. I'm still interested in what would happen if we started with an implementation like: - if the entire requested range falls within a hole, return that single hole. - otherwise, just treat the thing as one big data segment. That would provide a benefit in the case there are large-ish holes with minimal impact otherwise. (Though patches for full support are still useful even if only for client-testing purposes.) --b. > > Signed-off-by: Anna Schumaker > --- > fs/nfsd/nfs4proc.c | 4 ++-- > fs/nfsd/nfs4xdr.c | 35 ++++++++++++++++++++++++----------- > 2 files changed, 26 insertions(+), 13 deletions(-) > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index e9f4d8f..6801973 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -1862,8 +1862,8 @@ static inline u32 nfsd4_read_plus_rsize(struct svc_rqst *rqstp, struct nfsd4_op > { > u32 maxcount = svc_max_payload(rqstp); > u32 rlen = min(op->u.read.rd_length, maxcount); > - /* enough extra xdr space for encoding either a hole or data segment. */ > - u32 xdr = 5; > + /* Extra xdr padding for encoding multiple segments. */ > + u32 xdr = 20; > > return (op_encode_hdr_size + 2 + xdr + XDR_QUADLEN(rlen)) * sizeof(__be32); > } > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index 799d52c..5eaecd2 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -4117,7 +4117,7 @@ nfsd4_encode_layoutreturn(struct nfsd4_compoundres *resp, __be32 nfserr, > > static __be32 > nfsd4_encode_read_plus_data(struct nfsd4_compoundres *resp, struct nfsd4_read *read, > - struct file *file) > + struct file *file, loff_t hole_pos) > { > __be32 *p, err; > unsigned long maxcount; > @@ -4128,20 +4128,26 @@ nfsd4_encode_read_plus_data(struct nfsd4_compoundres *resp, struct nfsd4_read *r > return nfserr_resource; > xdr_commit_encode(xdr); > > + if (hole_pos <= read->rd_offset) > + hole_pos = i_size_read(file_inode(file)); > + > maxcount = svc_max_payload(resp->rqstp); > maxcount = min_t(unsigned long, maxcount, (xdr->buf->buflen - xdr->buf->len)); > maxcount = min_t(unsigned long, maxcount, read->rd_length); > + maxcount = min_t(unsigned long, maxcount, hole_pos - read->rd_offset); > > if (file->f_op->splice_read && test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags)) > err = nfsd4_encode_splice_read(resp, read, file, &maxcount); > else > err = nfsd4_encode_readv(resp, read, file, &maxcount); > + clear_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags); > > *p++ = cpu_to_be32(NFS4_CONTENT_DATA); > p = xdr_encode_hyper(p, read->rd_offset); > *p++ = cpu_to_be32(maxcount); > > read->rd_offset += maxcount; > + read->rd_length -= maxcount; > return err; > } > > @@ -4156,7 +4162,7 @@ nfsd4_encode_read_plus_hole(struct nfsd4_compoundres *resp, struct nfsd4_read *r > if (data_pos == -ENXIO) > data_pos = i_size_read(file_inode(file)); > if (data_pos <= read->rd_offset) > - return nfsd4_encode_read_plus_data(resp, read, file); > + return nfsd4_encode_read_plus_data(resp, read, file, 0); > > maxcount = data_pos - read->rd_offset; > p = xdr_reserve_space(&resp->xdr, 4 + 8 + 8); > @@ -4165,6 +4171,10 @@ nfsd4_encode_read_plus_hole(struct nfsd4_compoundres *resp, struct nfsd4_read *r > p = xdr_encode_hyper(p, maxcount); > > read->rd_offset += maxcount; > + if (maxcount > read->rd_length) > + read->rd_length = 0; > + else > + read->rd_length -= maxcount; > return nfs_ok; > } > > @@ -4197,17 +4207,20 @@ nfsd4_encode_read_plus(struct nfsd4_compoundres *resp, __be32 nfserr, > goto err_truncate; > } > > - hole_pos = vfs_llseek(file, read->rd_offset, SEEK_HOLE); > - if (hole_pos == -ENXIO) > - goto out_encode; > + do { > + hole_pos = vfs_llseek(file, read->rd_offset, SEEK_HOLE); > + if (hole_pos == -ENXIO) > + break; > > - if (hole_pos == read->rd_offset) > - err = nfsd4_encode_read_plus_hole(resp, read, file); > - else > - err = nfsd4_encode_read_plus_data(resp, read, file); > - segments++; > + if (hole_pos == read->rd_offset) > + err = nfsd4_encode_read_plus_hole(resp, read, file); > + else > + err = nfsd4_encode_read_plus_data(resp, read, file, hole_pos); > + if (err) > + break; > + segments++; > + } while (read->rd_length > 0); > > -out_encode: > eof = (read->rd_offset >= i_size_read(file_inode(file))); > *p++ = cpu_to_be32(eof); > *p++ = cpu_to_be32(segments); > -- > 2.3.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