| Message ID | 1395537141-10389-23-git-send-email-bfields@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Ah, here we got the helper for most of the error handling I asked for earlier. Might be worth mentioning in that patch that this gets further improvements soons. > if (nfserr) { > - xdr->p -= 2; > - xdr->iov->iov_len -= 8; > + xdr->buf->page_len = 0; > + xdr_truncate_encode(xdr, starting_len); Why do we still need to manually set the page_len? -- 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
On Sat, Mar 22, 2014 at 11:50:05PM -0700, Christoph Hellwig wrote: > Ah, here we got the helper for most of the error handling I asked for > earlier. Might be worth mentioning in that patch that this gets further > improvements soons. OK, will do. > > if (nfserr) { > > - xdr->p -= 2; > > - xdr->iov->iov_len -= 8; > > + xdr->buf->page_len = 0; > > + xdr_truncate_encode(xdr, starting_len); > > Why do we still need to manually set the page_len? In the splice case read increments page_len as it goes. This leaves the xdr_buf in somewhat of an inconsistent state and may confuse a later xdr_truncate_encode. It's a little ugly, I'm not sure what to do about it. Currently I'm thinking of just telling nfsd's splice callback to leave page_len alone. That'd mean taking another look at the splice callback and at the v2/v3 read code. --b. -- 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
On Sun, Mar 23, 2014 at 11:07:22AM -0400, J. Bruce Fields wrote: > In the splice case read increments page_len as it goes. This leaves the > xdr_buf in somewhat of an inconsistent state and may confuse a later > xdr_truncate_encode. > > It's a little ugly, I'm not sure what to do about it. Currently I'm > thinking of just telling nfsd's splice callback to leave page_len alone. > That'd mean taking another look at the splice callback and at the v2/v3 > read code. How a about just adding a comment explaing this for now? -- 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
On Tue, Mar 25, 2014 at 08:36:32AM -0700, Christoph Hellwig wrote: > On Sun, Mar 23, 2014 at 11:07:22AM -0400, J. Bruce Fields wrote: > > In the splice case read increments page_len as it goes. This leaves the > > xdr_buf in somewhat of an inconsistent state and may confuse a later > > xdr_truncate_encode. > > > > It's a little ugly, I'm not sure what to do about it. Currently I'm > > thinking of just telling nfsd's splice callback to leave page_len alone. > > That'd mean taking another look at the splice callback and at the v2/v3 > > read code. > > How a about just adding a comment explaing this for now? OK, done.--b. -- 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
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index a3dce3c..e5cc354 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -2043,7 +2043,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp, struct svc_export struct svc_fh *tempfh = NULL; struct kstatfs statfs; __be32 *p; - __be32 *start = xdr->p; + int starting_len = xdr->buf->len; __be32 *attrlenp; u32 dummy; u64 dummy64; @@ -2534,13 +2534,8 @@ out: kfree(acl); if (tempfh) fh_put(tempfh); - if (status) { - int nbytes = (char *)xdr->p - (char *)start; - /* open code what *should* be xdr_truncate(xdr, len); */ - xdr->iov->iov_len -= nbytes; - xdr->buf->len -= nbytes; - xdr->p = start; - } + if (status) + xdr_truncate_encode(xdr, starting_len); return status; out_nfserr: status = nfserrno(err); @@ -3005,6 +3000,7 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr, struct page *page; unsigned long maxcount; struct xdr_stream *xdr = &resp->xdr; + int starting_len = xdr->buf->len; long len; __be32 *p; @@ -3041,8 +3037,8 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr, &maxcount); if (nfserr) { - xdr->p -= 2; - xdr->iov->iov_len -= 8; + xdr->buf->page_len = 0; + xdr_truncate_encode(xdr, starting_len); return nfserr; } eof = (read->rd_offset + maxcount >= @@ -3077,6 +3073,7 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd int maxcount; struct xdr_stream *xdr = &resp->xdr; char *page; + int length_offset = xdr->buf->len; __be32 *p; if (nfserr) @@ -3101,8 +3098,7 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd if (nfserr == nfserr_isdir) nfserr = nfserr_inval; if (nfserr) { - xdr->p--; - xdr->iov->iov_len -= 4; + xdr_truncate_encode(xdr, length_offset); return nfserr; } @@ -3133,7 +3129,8 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4 int maxcount; loff_t offset; struct xdr_stream *xdr = &resp->xdr; - __be32 *page, *savep, *tailbase; + int starting_len = xdr->buf->len; + __be32 *page, *tailbase; __be32 *p; if (nfserr) @@ -3144,7 +3141,6 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4 return nfserr_resource; RESERVE_SPACE(NFS4_VERIFIER_SIZE); - savep = p; /* XXX: Following NFSv3, we ignore the READDIR verifier for now. */ WRITE32(0); @@ -3206,9 +3202,7 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4 return 0; err_no_verf: - xdr->p = savep; - xdr->iov->iov_len = ((char *)resp->xdr.p) - - (char *)resp->xdr.buf->head[0].iov_base; + xdr_truncate_encode(xdr, starting_len); return nfserr; }