* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.