FlexFiles: too many bytes w/ direct+aio retried read
Subsystems (2)
cover FlexFiles: too many bytes w/ direct+aio retried read
vers 2 fixes unused var found by buildbot.
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 | 14 ++++++++------ fs/nfs/pnfs.h | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-)
Signature · 1 line
2.6.4 (Apple Git-63)
PATCH 1/2 pnfs: set NFS_IOHDR_REDO in pnfs_read_resend_pnfs
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.
--- fs/nfs/pnfs.c | 14 ++++++++------ fs/nfs/pnfs.h | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-)
fs/nfs/pnfs.c +8 -6
@@ -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);
@@ -2158,12 +2161,11 @@ pnfs_do_read(struct nfs_pageio_descriptor *desc, struct nfs_pgio_header *hdr)const struct rpc_call_ops *call_ops = desc->pg_rpc_callops;struct pnfs_layout_segment *lseg = desc->pg_lseg;enum pnfs_try_status trypnfs; -int err =0 ; 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); }
fs/nfs/pnfs.h +1 -1
@@ -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,
Signature · 1 line
2.6.4 (Apple Git-63)
PATCH 2/2 nfs: add debug to directio "good_bytes" counting
In reply to Weston Andros AdamsonThis will pop a warning if we count too many good bytes.
--- fs/nfs/direct.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
fs/nfs/direct.c +5 -2
@@ -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);
Signature · 1 line
2.6.4 (Apple Git-63)