* [PATCH 0/2] FlexFiles: too many bytes w/ direct+aio retried read @ 2016-04-01 14:29 Weston Andros Adamson 2016-04-01 14:29 ` [PATCH 1/2] pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs Weston Andros Adamson 2016-04-01 14:29 ` [PATCH 2/2] nfs: add debug to directio "good_bytes" counting Weston Andros Adamson 0 siblings, 2 replies; 5+ messages in thread From: Weston Andros Adamson @ 2016-04-01 14:29 UTC (permalink / raw) To: trond.myklebust; +Cc: linux-nfs, Weston Andros Adamson Patch 1 fixes an issue seen when using libaio with directio where if a read is retried (ie due to recalled layout) we return too many bytes. Specifically, we returned a multiple of the read size. To reproduce, run fio with ioengine=libaio and direct=1, then revoke the active layout. This should hit pretty easily. Patch 2 should help notice similar issues elsewhere. Weston Andros Adamson (2): pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs nfs: add debug to directio "good_bytes" counting fs/nfs/direct.c | 7 +++++-- fs/nfs/pnfs.c | 13 ++++++++----- fs/nfs/pnfs.h | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-) -- 2.6.4 (Apple Git-63) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs 2016-04-01 14:29 [PATCH 0/2] FlexFiles: too many bytes w/ direct+aio retried read Weston Andros Adamson @ 2016-04-01 14:29 ` Weston Andros Adamson 2016-04-01 15:28 ` kbuild test robot 2016-04-01 14:29 ` [PATCH 2/2] nfs: add debug to directio "good_bytes" counting Weston Andros Adamson 1 sibling, 1 reply; 5+ messages in thread From: Weston Andros Adamson @ 2016-04-01 14:29 UTC (permalink / raw) To: trond.myklebust; +Cc: linux-nfs, Weston Andros Adamson Like other resend paths, mark the (old) hdr as NFS_IOHDR_REDO. This ensures the hdr completion function will not count the (old) hdr as good bytes. Also, vector the error back through the hdr->task.tk_status like other retry calls. This fixes a bug with the FlexFiles layout where libaio was reporting more bytes read than requested. Signed-off-by: Weston Andros Adamson <dros@primarydata.com> --- fs/nfs/pnfs.c | 13 ++++++++----- fs/nfs/pnfs.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index 2fa483e6dbe2..da90d665b01f 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -2143,12 +2143,15 @@ pnfs_try_to_read_data(struct nfs_pgio_header *hdr, } /* Resend all requests through pnfs. */ -int pnfs_read_resend_pnfs(struct nfs_pgio_header *hdr) +void pnfs_read_resend_pnfs(struct nfs_pgio_header *hdr) { struct nfs_pageio_descriptor pgio; - nfs_pageio_init_read(&pgio, hdr->inode, false, hdr->completion_ops); - return nfs_pageio_resend(&pgio, hdr); + if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) { + nfs_pageio_init_read(&pgio, hdr->inode, false, + hdr->completion_ops); + hdr->task.tk_status = nfs_pageio_resend(&pgio, hdr); + } } EXPORT_SYMBOL_GPL(pnfs_read_resend_pnfs); @@ -2162,8 +2165,8 @@ pnfs_do_read(struct nfs_pageio_descriptor *desc, struct nfs_pgio_header *hdr) trypnfs = pnfs_try_to_read_data(hdr, call_ops, lseg); if (trypnfs == PNFS_TRY_AGAIN) - err = pnfs_read_resend_pnfs(hdr); - if (trypnfs == PNFS_NOT_ATTEMPTED || err) + pnfs_read_resend_pnfs(hdr); + if (trypnfs == PNFS_NOT_ATTEMPTED || hdr->task.tk_status) pnfs_read_through_mds(desc, hdr); } diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h index 1ac1db5f6dad..7222d3a35439 100644 --- a/fs/nfs/pnfs.h +++ b/fs/nfs/pnfs.h @@ -282,7 +282,7 @@ int _pnfs_return_layout(struct inode *); int pnfs_commit_and_return_layout(struct inode *); void pnfs_ld_write_done(struct nfs_pgio_header *); void pnfs_ld_read_done(struct nfs_pgio_header *); -int pnfs_read_resend_pnfs(struct nfs_pgio_header *); +void pnfs_read_resend_pnfs(struct nfs_pgio_header *); struct pnfs_layout_segment *pnfs_update_layout(struct inode *ino, struct nfs_open_context *ctx, loff_t pos, -- 2.6.4 (Apple Git-63) ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs 2016-04-01 14:29 ` [PATCH 1/2] pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs Weston Andros Adamson @ 2016-04-01 15:28 ` kbuild test robot 2016-04-01 15:43 ` Weston Andros Adamson 0 siblings, 1 reply; 5+ messages in thread From: kbuild test robot @ 2016-04-01 15:28 UTC (permalink / raw) To: Weston Andros Adamson Cc: kbuild-all, trond.myklebust, linux-nfs, Weston Andros Adamson [-- Attachment #1: Type: text/plain, Size: 3071 bytes --] Hi Weston, [auto build test WARNING on v4.6-rc1] [also build test WARNING on next-20160401] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Weston-Andros-Adamson/pnfs-set-NFS_IOHDR_REDO-in-pnfs_read_resend_pnfs/20160401-223208 config: i386-allmodconfig (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): fs/nfs/pnfs.c: In function 'pnfs_do_read': >> fs/nfs/pnfs.c:2164:6: warning: unused variable 'err' [-Wunused-variable] int err = 0; ^ vim +/err +2164 fs/nfs/pnfs.c ceb11e13 Peng Tao 2014-11-10 2148 struct nfs_pageio_descriptor pgio; ceb11e13 Peng Tao 2014-11-10 2149 a5073dc2 Weston Andros Adamson 2016-04-01 2150 if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) { a5073dc2 Weston Andros Adamson 2016-04-01 2151 nfs_pageio_init_read(&pgio, hdr->inode, false, a5073dc2 Weston Andros Adamson 2016-04-01 2152 hdr->completion_ops); a5073dc2 Weston Andros Adamson 2016-04-01 2153 hdr->task.tk_status = nfs_pageio_resend(&pgio, hdr); a5073dc2 Weston Andros Adamson 2016-04-01 2154 } ceb11e13 Peng Tao 2014-11-10 2155 } ceb11e13 Peng Tao 2014-11-10 2156 EXPORT_SYMBOL_GPL(pnfs_read_resend_pnfs); ceb11e13 Peng Tao 2014-11-10 2157 493292dd Trond Myklebust 2011-07-13 2158 static void 7f714720 Weston Andros Adamson 2014-05-15 2159 pnfs_do_read(struct nfs_pageio_descriptor *desc, struct nfs_pgio_header *hdr) 493292dd Trond Myklebust 2011-07-13 2160 { 493292dd Trond Myklebust 2011-07-13 2161 const struct rpc_call_ops *call_ops = desc->pg_rpc_callops; 493292dd Trond Myklebust 2011-07-13 2162 struct pnfs_layout_segment *lseg = desc->pg_lseg; 493292dd Trond Myklebust 2011-07-13 2163 enum pnfs_try_status trypnfs; ceb11e13 Peng Tao 2014-11-10 @2164 int err = 0; 493292dd Trond Myklebust 2011-07-13 2165 d45f60c6 Weston Andros Adamson 2014-06-09 2166 trypnfs = pnfs_try_to_read_data(hdr, call_ops, lseg); ceb11e13 Peng Tao 2014-11-10 2167 if (trypnfs == PNFS_TRY_AGAIN) a5073dc2 Weston Andros Adamson 2016-04-01 2168 pnfs_read_resend_pnfs(hdr); a5073dc2 Weston Andros Adamson 2016-04-01 2169 if (trypnfs == PNFS_NOT_ATTEMPTED || hdr->task.tk_status) d45f60c6 Weston Andros Adamson 2014-06-09 2170 pnfs_read_through_mds(desc, hdr); 493292dd Trond Myklebust 2011-07-13 2171 } 493292dd Trond Myklebust 2011-07-13 2172 :::::: The code at line 2164 was first introduced by commit :::::: ceb11e13df3e78b450730c615037133c57b90c3b pnfs: allow LD to ask to resend read through pnfs :::::: TO: Peng Tao <tao.peng@primarydata.com> :::::: CC: Tom Haynes <loghyr@primarydata.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/octet-stream, Size: 54415 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs 2016-04-01 15:28 ` kbuild test robot @ 2016-04-01 15:43 ` Weston Andros Adamson 0 siblings, 0 replies; 5+ messages in thread From: Weston Andros Adamson @ 2016-04-01 15:43 UTC (permalink / raw) To: kbuild test robot; +Cc: kbuild-all, Trond Myklebust, linux-nfs list > On Apr 1, 2016, at 11:28 AM, kbuild test robot <lkp@intel.com> wrote: > > Hi Weston, > > [auto build test WARNING on v4.6-rc1] > [also build test WARNING on next-20160401] > [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] > > url: https://github.com/0day-ci/linux/commits/Weston-Andros-Adamson/pnfs-set-NFS_IOHDR_REDO-in-pnfs_read_resend_pnfs/20160401-223208 > config: i386-allmodconfig (attached as .config) > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All warnings (new ones prefixed by >>): > > fs/nfs/pnfs.c: In function 'pnfs_do_read': >>> fs/nfs/pnfs.c:2164:6: warning: unused variable 'err' [-Wunused-variable] > int err = 0; > ^ Thanks bot! Fixed and reposted. -dros > > vim +/err +2164 fs/nfs/pnfs.c > > ceb11e13 Peng Tao 2014-11-10 2148 struct nfs_pageio_descriptor pgio; > ceb11e13 Peng Tao 2014-11-10 2149 > a5073dc2 Weston Andros Adamson 2016-04-01 2150 if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) { > a5073dc2 Weston Andros Adamson 2016-04-01 2151 nfs_pageio_init_read(&pgio, hdr->inode, false, > a5073dc2 Weston Andros Adamson 2016-04-01 2152 hdr->completion_ops); > a5073dc2 Weston Andros Adamson 2016-04-01 2153 hdr->task.tk_status = nfs_pageio_resend(&pgio, hdr); > a5073dc2 Weston Andros Adamson 2016-04-01 2154 } > ceb11e13 Peng Tao 2014-11-10 2155 } > ceb11e13 Peng Tao 2014-11-10 2156 EXPORT_SYMBOL_GPL(pnfs_read_resend_pnfs); > ceb11e13 Peng Tao 2014-11-10 2157 > 493292dd Trond Myklebust 2011-07-13 2158 static void > 7f714720 Weston Andros Adamson 2014-05-15 2159 pnfs_do_read(struct nfs_pageio_descriptor *desc, struct nfs_pgio_header *hdr) > 493292dd Trond Myklebust 2011-07-13 2160 { > 493292dd Trond Myklebust 2011-07-13 2161 const struct rpc_call_ops *call_ops = desc->pg_rpc_callops; > 493292dd Trond Myklebust 2011-07-13 2162 struct pnfs_layout_segment *lseg = desc->pg_lseg; > 493292dd Trond Myklebust 2011-07-13 2163 enum pnfs_try_status trypnfs; > ceb11e13 Peng Tao 2014-11-10 @2164 int err = 0; > 493292dd Trond Myklebust 2011-07-13 2165 > d45f60c6 Weston Andros Adamson 2014-06-09 2166 trypnfs = pnfs_try_to_read_data(hdr, call_ops, lseg); > ceb11e13 Peng Tao 2014-11-10 2167 if (trypnfs == PNFS_TRY_AGAIN) > a5073dc2 Weston Andros Adamson 2016-04-01 2168 pnfs_read_resend_pnfs(hdr); > a5073dc2 Weston Andros Adamson 2016-04-01 2169 if (trypnfs == PNFS_NOT_ATTEMPTED || hdr->task.tk_status) > d45f60c6 Weston Andros Adamson 2014-06-09 2170 pnfs_read_through_mds(desc, hdr); > 493292dd Trond Myklebust 2011-07-13 2171 } > 493292dd Trond Myklebust 2011-07-13 2172 > > :::::: The code at line 2164 was first introduced by commit > :::::: ceb11e13df3e78b450730c615037133c57b90c3b pnfs: allow LD to ask to resend read through pnfs > > :::::: TO: Peng Tao <tao.peng@primarydata.com> > :::::: CC: Tom Haynes <loghyr@primarydata.com> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation > <.config.gz> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] nfs: add debug to directio "good_bytes" counting 2016-04-01 14:29 [PATCH 0/2] FlexFiles: too many bytes w/ direct+aio retried read Weston Andros Adamson 2016-04-01 14:29 ` [PATCH 1/2] pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs Weston Andros Adamson @ 2016-04-01 14:29 ` Weston Andros Adamson 1 sibling, 0 replies; 5+ messages in thread From: Weston Andros Adamson @ 2016-04-01 14:29 UTC (permalink / raw) To: trond.myklebust; +Cc: linux-nfs, Weston Andros Adamson This will pop a warning if we count too many good bytes. Signed-off-by: Weston Andros Adamson <dros@primarydata.com> --- fs/nfs/direct.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c index 7a0cfd3266e5..5a31f2928530 100644 --- a/fs/nfs/direct.c +++ b/fs/nfs/direct.c @@ -87,6 +87,7 @@ struct nfs_direct_req { int mirror_count; ssize_t count, /* bytes actually processed */ + max_count, /* max expected count */ bytes_left, /* bytes left to be sent */ io_start, /* start of IO */ error; /* any reported error */ @@ -123,6 +124,8 @@ nfs_direct_good_bytes(struct nfs_direct_req *dreq, struct nfs_pgio_header *hdr) int i; ssize_t count; + WARN_ON_ONCE(dreq->count >= dreq->max_count); + if (dreq->mirror_count == 1) { dreq->mirrors[hdr->pgio_mirror_idx].count += hdr->good_bytes; dreq->count += hdr->good_bytes; @@ -593,7 +596,7 @@ ssize_t nfs_file_direct_read(struct kiocb *iocb, struct iov_iter *iter, goto out_unlock; dreq->inode = inode; - dreq->bytes_left = count; + dreq->bytes_left = dreq->max_count = count; dreq->io_start = pos; dreq->ctx = get_nfs_open_context(nfs_file_open_context(iocb->ki_filp)); l_ctx = nfs_get_lock_context(dreq->ctx); @@ -1026,7 +1029,7 @@ ssize_t nfs_file_direct_write(struct kiocb *iocb, struct iov_iter *iter) goto out_unlock; dreq->inode = inode; - dreq->bytes_left = iov_iter_count(iter); + dreq->bytes_left = dreq->max_count = iov_iter_count(iter); dreq->io_start = pos; dreq->ctx = get_nfs_open_context(nfs_file_open_context(iocb->ki_filp)); l_ctx = nfs_get_lock_context(dreq->ctx); -- 2.6.4 (Apple Git-63) ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-04-01 15:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-01 14:29 [PATCH 0/2] FlexFiles: too many bytes w/ direct+aio retried read Weston Andros Adamson 2016-04-01 14:29 ` [PATCH 1/2] pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs Weston Andros Adamson 2016-04-01 15:28 ` kbuild test robot 2016-04-01 15:43 ` Weston Andros Adamson 2016-04-01 14:29 ` [PATCH 2/2] nfs: add debug to directio "good_bytes" counting Weston Andros Adamson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).