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)
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 <[email protected]>
---
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)
This will pop a warning if we count too many good bytes.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
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)
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 <[email protected]>
:::::: CC: Tom Haynes <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
> On Apr 1, 2016, at 11:28 AM, kbuild test robot <[email protected]> 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 <[email protected]>
> :::::: CC: Tom Haynes <[email protected]>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
> <.config.gz>