CIFS: Add support for direct I/O read
Subsystems (2)
PATCH v4 1/3 CIFS: Add support for direct I/O read
From: Long Li <[email protected]>
With direct I/O read, we transfer the data directly from transport layer to the user data buffer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common read code to __cifs_readv for direct and non-direct I/O. Retry on direct I/O failure.
--- fs/cifs/cifsfs.h | 1 + fs/cifs/cifsglob.h | 5 ++ fs/cifs/file.c | 219 +++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 186 insertions(+), 39 deletions(-)
fs/cifs/cifsfs.h +1 -0
@@ -102,6 +102,7 @@ extern int cifs_open(struct inode *inode, struct file *file);extern int cifs_close (struct inode *inode,struct file *file);extern int cifs_closedir (struct inode *inode,struct file *file);extern ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to); +extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_strict_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from);extern ssize_t cifs_strict_writev (struct kiocb *iocb,struct iov_iter *from);
fs/cifs/cifsglob.h +5 -0
@@ -1146,6 +1146,11 @@ struct cifs_aio_ctx {unsigned int len;unsigned int total_len;bool should_dirty; +/* +* Indicates if this aio_ctx is for direct_io, +* If yes, iter is a copy of the user passed iov_iter +*/ +bool direct_io; };struct cifs_readdata ;
fs/cifs/file.c +180 -39
324 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH v4 2/3 CIFS: Add support for direct I/O write
From: Long Li <[email protected]>
With direct I/O write, user supplied buffers are pinned to the memory and data are transferred directly from user buffers to the transport layer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common write code to __cifs_writev for direct and non-direct I/O. Retry on direct I/O failure.
--- fs/cifs/cifsfs.h | 1 + fs/cifs/file.c | 194 +++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 154 insertions(+), 41 deletions(-)
fs/cifs/cifsfs.h +1 -0
@@ -105,6 +105,7 @@ extern ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to);extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_strict_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from); +extern ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter *from);extern ssize_t cifs_strict_writev (struct kiocb *iocb,struct iov_iter *from);extern int cifs_lock (struct file *,int ,struct file_lock *);extern int cifs_fsync (struct file *,loff_t ,loff_t ,int );
fs/cifs/file.c +153 -41
267 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 31 окт. 2018 г. в 15:26, Long Li <[email protected]>:
Quoted 312 lines
From: Long Li <[email protected]>
With direct I/O write, user supplied buffers are pinned to the memory and data are transferred directly from user buffers to the transport layer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common write code to__cifs_writevfor direct and non-direct I/O. Retry on direct I/O failure.Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/file.c | 194 +++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 154 insertions(+), 41 deletions(-)fs/cifs/cifsfs.h +1
-0@@ -105,6 +105,7 @@ extern ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to);extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_strict_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from); +extern ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter *from);extern ssize_t cifs_strict_writev (struct kiocb *iocb,struct iov_iter *from);extern int cifs_lock (struct file *,int ,struct file_lock *);extern int cifs_fsync (struct file *,loff_t ,loff_t ,int );fs/cifs/file.c +153
-42@@ -2524,6 +2524,55 @@ wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from, }static int +cifs_resend_wdata (struct cifs_writedata *wdata,struct list_head *wdata_list,struct cifs_aio_ctx *ctx) +{ +int wait_retry =0 ; +unsigned int wsize, credits; +int rc; +struct TCP_Server_Info *server =tlink_tcon (wdata->cfile->tlink)->ses->server; + +/* +* Try to resend this wdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole wdata not in segments +*/ +do { + rc = server->ops->wait_mtu_credits (server, wdata->bytes, &wsize, &credits); + +if (rc) +break ; + +if (wsize < wdata->bytes) { +add_credits_and_wake_if (server, credits,0 ); +msleep (1000 ); + wait_retry++; + } + }while (wsize < wdata->bytes && wait_retry <3 ); + +if (wsize < wdata->bytes) { + rc = -EBUSY ; + goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!wdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (wdata->cfile, false))) + rc = server->ops->async_writev (wdata, + cifs_uncached_writedata_release); + +if (!rc) { +list_add_tail (&wdata->list, wdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, wdata->credits,0 ); +out: +kref_put (&wdata->refcount, cifs_uncached_writedata_release); + +return rc; +} + +static int cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from, struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, struct list_head *wdata_list,@@ -2537,6 +2586,8 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,loff_t saved_offset = offset;pid_t pid;struct TCP_Server_Info *server; +struct page **pagevec; +size_t start;if (cifs_sb->mnt_cifs_flags &CIFS_MOUNT_RWPIDFORWARD ) pid = open_file->pid;@@ -2553,38 +2604,74 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,if (rc)break ; - nr_pages =get_numpages (wsize, len, &cur_len); - wdata =cifs_writedata_alloc (nr_pages, +if (ctx->direct_io) { + cur_len =iov_iter_get_pages_alloc ( + from, &pagevec, wsize, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"direct_writev couldn't get user pages " +"(rc=%zd) iter type %d iov_offset %zd count" +" %zd\n" , + cur_len, from->type, + from->iov_offset, from->count); +dump_stack (); +break ; + } +iov_iter_advance (from, cur_len); + + nr_pages = (cur_len + start +PAGE_SIZE -1 ) /PAGE_SIZE ; + + wdata =cifs_writedata_direct_alloc (pagevec, cifs_uncached_writev_complete); -if (!wdata) { - rc = -ENOMEM ; -add_credits_and_wake_if (server, credits,0 ); -break ; - } +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } - rc =cifs_write_allocate_pages (wdata->pages, nr_pages); -if (rc) { -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } - num_pages = nr_pages; - rc =wdata_fill_from_iovec (wdata, from, &cur_len, &num_pages); -if (rc) { -for (i =0 ; i < nr_pages; i++) -put_page (wdata->pages[i]); -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } + wdata->page_offset = start; + wdata->tailsz = + nr_pages >1 ? + cur_len - (PAGE_SIZE - start) - + (nr_pages -2 ) *PAGE_SIZE : + cur_len; + }else { + nr_pages =get_numpages (wsize, len, &cur_len); + wdata =cifs_writedata_alloc (nr_pages, + cifs_uncached_writev_complete); +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } -/* -* Bring nr_pages down to the number of pages we actually used, -* and free any pages that we didn't use. -*/ -for ( ; nr_pages > num_pages; nr_pages--) -put_page (wdata->pages[nr_pages -1 ]); + rc =cifs_write_allocate_pages (wdata->pages, nr_pages); +if (rc) { +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + + num_pages = nr_pages; + rc =wdata_fill_from_iovec (wdata, from, &cur_len, &num_pages); +if (rc) { +for (i =0 ; i < nr_pages; i++) +put_page (wdata->pages[i]); +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + +/* +* Bring nr_pages down to the number of pages we actually used, +* and free any pages that we didn't use. +*/ +for ( ; nr_pages > num_pages; nr_pages--) +put_page (wdata->pages[nr_pages -1 ]); + + wdata->tailsz = cur_len - ((nr_pages -1 ) *PAGE_SIZE ); + } wdata->sync_mode =WB_SYNC_ALL ; wdata->nr_pages = nr_pages;@@ -2593,7 +2680,6 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from, wdata->pid = pid; wdata->bytes = cur_len; wdata->pagesz =PAGE_SIZE ; - wdata->tailsz = cur_len - ((nr_pages -1 ) *PAGE_SIZE ); wdata->credits = credits; wdata->ctx = ctx;kref_get (&ctx->refcount);@@ -2668,13 +2754,17 @@ static void collect_uncached_write_data(struct cifs_aio_ctx *ctx)INIT_LIST_HEAD (&tmp_list);list_del_init (&wdata->list); -iov_iter_advance (&tmp_from, +if (ctx->direct_io) + rc =cifs_resend_wdata (wdata, &tmp_list, ctx); +else { +iov_iter_advance (&tmp_from, wdata->offset - ctx->pos); - rc =cifs_write_from_iter (wdata->offset, + rc =cifs_write_from_iter (wdata->offset, wdata->bytes, &tmp_from, ctx->cfile, cifs_sb, &tmp_list, ctx); + }list_splice (&tmp_list, &ctx->list);@@ -2687,8 +2777,9 @@ static void collect_uncached_write_data(struct cifs_aio_ctx *ctx)kref_put (&wdata->refcount, cifs_uncached_writedata_release); } -for (i =0 ; i < ctx->npages; i++) -put_page (ctx->bv[i].bv_page); +if (!ctx->direct_io) +for (i =0 ; i < ctx->npages; i++) +put_page (ctx->bv[i].bv_page);cifs_stats_bytes_written (tcon, ctx->total_len);set_bit (CIFS_INO_INVALID_MAPPING , &CIFS_I (dentry->d_inode)->flags);@@ -2703,7 +2794,7 @@ static void collect_uncached_write_data(struct cifs_aio_ctx *ctx)complete (&ctx->done); } -ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) +static ssize_t __cifs_writev (struct kiocb *iocb,struct iov_iter *from,bool direct) {struct file *file = iocb->ki_filp;ssize_t total_written =0 ;@@ -2712,13 +2803,18 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)struct cifs_sb_info *cifs_sb;struct cifs_aio_ctx *ctx;struct iov_iter saved_from = *from; +size_t len =iov_iter_count (from);int rc;/* -* BB - optimize the way when signing is disabled. We can drop this -* extra memory-to-memory copying and use iovec buffers for constructing -* write request. +* iov_iter_get_pages_alloc doesn't work with ITER_KVEC. +* In this case, fall back to non-direct write function. +* this could be improved by getting pages directly in ITER_KVEC */ +if (direct && from->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_writev for kvec I/O\n" ); + direct = false; + } rc =generic_write_checks (iocb, from);if (rc <=0 )@@ -2742,10 +2838,16 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from) ctx->pos = iocb->ki_pos; - rc =setup_aio_ctx_iter (ctx, from,WRITE ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->direct_io = true; + ctx->iter = *from; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, from,WRITE ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } }/* grab a lock here due to read response handlers can access ctx */ @@ -2795,6 +2897,16 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)return total_written; } +ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter *from) +{ +return __cifs_writev (iocb, from, true); +} + +ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) +{ +return __cifs_writev (iocb, from, false); +} +static ssize_t cifs_writev (struct kiocb *iocb,struct iov_iter *from) { --
Quoted 2 lines
2.7.4
Did you measure the performance benefit of using O_DIRECT with your patches for non-RDMA mode?
--
Best regards,
Pavel Shilovsky
RE: [Patch v4 2/3] CIFS: Add support for direct I/O write
Quoted 322 lines
Subject: Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 31 окт. 2018 г. в 15:26, Long Li <[email protected]>:
Quoted 8 lines
From: Long Li <[email protected]>
With direct I/O write, user supplied buffers are pinned to the memory and data are transferred directly from user buffers to the transport layer.
Change in v3: add support for kernel AIO
Quoted 8 lines
Change in v4:
Refactor common write code to__cifs_writevfor direct and non-direct I/O. Retry on direct I/O failure.Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/file.c | 194 +++++++++++++++++++++++++++++++++++++++++++------------
Quoted 24 lines
2 files changed, 154 insertions(+), 41 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-07fba9aa..e9c5103 100644
fs/cifs/cifsfs.h +0
-0@@ -105,6 +105,7 @@ extern ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to); externssize_tcifs_direct_readv(struct kiocb *iocb, struct iov_iter *to); externssize_tcifs_strict_readv(struct kiocb *iocb, struct iov_iter *to); externssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter +*from); extern ssize_t cifs_strict_writev(struct kiocb *iocb, struct iov_iter*from); extern int
cifs_lock(struct file *, int, struct file_lock *); extern intcifs_fsync(struct file *, loff_t, loff_t, int); diff --gita/fs/cifs/file.cb/fs/cifs/file.cindex daab878..1a41c04 100644fs/cifs/file.c +1
-0@@ -2524,6 +2524,55 @@ wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from, }static int +cifs_resend_wdata(struct cifs_writedata *wdata, struct list_headfs/cifs/file.c wdata_fill_from_iovec(struct cifs_writedata · line 2537
Quoted 10 lines
fs/cifs/file.c +10
-0+*wdata_list, struct cifs_aio_ctx *ctx) { +int wait_retry =0 ; +unsigned int wsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (wdata->cfile->tlink)->ses->server; + +/* +* Try to resend this wdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole wdata not in segments
Quoted 44 lines
fs/cifs/file.c +41
-0+*/ +do { + rc = server->ops->wait_mtu_credits (server, + wdata->bytes, &wsize, &credits); + +if (rc) +break ; + +if (wsize < wdata->bytes) { +add_credits_and_wake_if (server, credits,0 ); +msleep (1000 ); + wait_retry++; + } + }while (wsize < wdata->bytes && wait_retry <3 ); + +if (wsize < wdata->bytes) { + rc = -EBUSY ; + goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!wdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (wdata->cfile, false))) + rc = server->ops->async_writev (wdata, + + cifs_uncached_writedata_release); + +if (!rc) { +list_add_tail (&wdata->list, wdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, wdata->credits,0 ); +out: +kref_put (&wdata->refcount, cifs_uncached_writedata_release); + +return rc; +} + +static int cifs_write_from_iter (loff_t offset,size_t len,struct iov_iter *from,struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, struct list_headQuoted 1 line
*wdata_list, @@ -2537,6 +2586,8 @@ cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len,
Quoted 9 lines
loff_t saved_offset = offset; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +2
-0+struct page **pagevec; +size_t start;if (cifs_sb->mnt_cifs_flags &CIFS_MOUNT_RWPIDFORWARD ) pid = open_file->pid;@@ -2553,38 +2604,74 @@ cifs_write_from_iter(loff_t offset, size_t len,struct
iov_iter*from,Quoted 73 lines
fs/cifs/file.c +39
-27if (rc)break ; - nr_pages =get_numpages (wsize, len, &cur_len); - wdata =cifs_writedata_alloc (nr_pages, +if (ctx->direct_io) { + cur_len =iov_iter_get_pages_alloc ( + from, &pagevec, wsize, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"direct_writev couldn't get user pages " +"(rc=%zd) iter type %d iov_offset %zd count" +" %zd\n" , + cur_len, from->type, + from->iov_offset, from->count); +dump_stack (); +break ; + } +iov_iter_advance (from, cur_len); + + nr_pages = (cur_len + start +PAGE_SIZE -1 ) / +PAGE_SIZE ; + + wdata =cifs_writedata_direct_alloc (pagevec, cifs_uncached_writev_complete); -if (!wdata) { - rc = -ENOMEM ; -add_credits_and_wake_if (server, credits,0 ); -break ; - } +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } - rc =cifs_write_allocate_pages (wdata->pages, nr_pages); -if (rc) { -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } - num_pages = nr_pages; - rc =wdata_fill_from_iovec (wdata, from, &cur_len, &num_pages); -if (rc) { -for (i =0 ; i < nr_pages; i++) -put_page (wdata->pages[i]); -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } + wdata->page_offset = start; + wdata->tailsz = + nr_pages >1 ? + cur_len - (PAGE_SIZE - start) - + (nr_pages -2 ) *PAGE_SIZE : + cur_len; + }else { + nr_pages =get_numpages (wsize, len, &cur_len); + wdata =cifs_writedata_alloc (nr_pages, + cifs_uncached_writev_complete); +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } -/* -* Bring nr_pages down to the number of pages we actually used, -* and free any pages that we didn't use. -*/ -for ( ; nr_pages > num_pages; nr_pages--)fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 10 lines
fs/cifs/file.c +9
-1-put_page (wdata->pages[nr_pages -1 ]); + rc =cifs_write_allocate_pages (wdata->pages, nr_pages); +if (rc) { +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + + num_pages = nr_pages; + rc = wdata_fill_from_iovec(wdata, from, &cur_len,
&num_pages);fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 10 lines
fs/cifs/file.c +10
-0+if (rc) { +for (i =0 ; i < nr_pages; i++) +put_page (wdata->pages[i]); +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + +/* +* Bring nr_pages down to the number of pages we actually used,
fs/cifs/file.c static void collect_uncached_write_data(struct
Quoted 41 lines
fs/cifs/file.c +7
-0+* and free any pages that we didn't use. +*/ +for ( ; nr_pages > num_pages; nr_pages--) +put_page (wdata->pages[nr_pages -1 ]); + + wdata->tailsz = cur_len - ((nr_pages -1 ) *PAGE_SIZE ); + } wdata->sync_mode =WB_SYNC_ALL ; wdata->nr_pages = nr_pages; @@ -2593,7 +2680,6 @@cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,wdata->pid = pid; wdata->bytes = cur_len; wdata->pagesz = PAGE_SIZE; - wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE); wdata->credits = credits; wdata->ctx = ctx; kref_get(&ctx->refcount); @@ -2668,13 +2754,17 @@ static void collect_uncached_write_data(struct cifs_aio_ctx *ctx) INIT_LIST_HEAD(&tmp_list); list_del_init(&wdata->list);fs/cifs/file.c +4
-1- iov_iter_advance(&tmp_from, +if (ctx->direct_io) + rc =cifs_resend_wdata (wdata, &tmp_list, ctx); +else { + iov_iter_advance(&tmp_from, wdata->offset -
ctx->pos);fs/cifs/file.c +3
-1- rc =cifs_write_from_iter (wdata->offset, + rc = +cifs_write_from_iter (wdata->offset, wdata->bytes, &tmp_from, ctx->cfile, cifs_sb, &tmp_list, ctx); + }list_splice (&tmp_list, &ctx->list);@@ -2687,8 +2777,9 @@ static void collect_uncached_write_data(struct
cifs_aio_ctx*ctx)Quoted 11 lines
fs/cifs/file.c +3
-2kref_put (&wdata->refcount, cifs_uncached_writedata_release); } -for (i =0 ; i < ctx->npages; i++) -put_page (ctx->bv[i].bv_page); +if (!ctx->direct_io) +for (i =0 ; i < ctx->npages; i++) +put_page (ctx->bv[i].bv_page);cifs_stats_bytes_written (tcon, ctx->total_len); set_bit(CIFS_INO_INVALID_MAPPING,Quoted 1 line
&CIFS_I(dentry->d_inode)->flags); @@ -2703,7 +2794,7 @@ static void
collect_uncached_write_data(struct cifs_aio_ctx *ctx)fs/cifs/file.c ssize_t cifs_user_writev(struct kiocb *iocb,
Quoted 10 lines
complete(&ctx->done);}
fs/cifs/file.c +2
-1-ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) +static ssize_t __cifs_writev (struct kiocb *iocb,struct iov_iter +*from,bool direct) {struct file *file = iocb->ki_filp;ssize_t total_written =0 ;@@ -2712,13 +2803,18 @@ ssize_t cifs_user_writev(struct kiocb *iocb,struct
iov_iter*from)fs/cifs/file.c ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)
Quoted 9 lines
fs/cifs/file.c +1
-2struct cifs_sb_info *cifs_sb;struct cifs_aio_ctx *ctx;struct iov_iter saved_from = *from; +size_t len =iov_iter_count (from);int rc;/* -* BB - optimize the way when signing is disabled. We can drop this -* extra memory-to-memory copying and use iovec buffers for constructing
Quoted 35 lines
fs/cifs/file.c +18
-5-* write request. +* iov_iter_get_pages_alloc doesn't work with ITER_KVEC. +* In this case, fall back to non-direct write function. +* this could be improved by getting pages directly in +ITER_KVEC */ +if (direct && from->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_writev for kvec I/O\n" ); + direct = false; + } rc =generic_write_checks (iocb, from);if (rc <=0 )@@ -2742,10 +2838,16 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from) ctx->pos = iocb->ki_pos; - rc =setup_aio_ctx_iter (ctx, from,WRITE ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->direct_io = true; + ctx->iter = *from; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, from,WRITE ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } }/* grab a lock here due to read response handlers can accessQuoted 1 line
ctx */ @@ -2795,6 +2897,16 @@
ssize_tcifs_user_writev(struct kiocb*iocb, struct
iov_iter*from)Quoted 3 lines
return total_written;}
Quoted 15 lines
Quoted 10 lines
fs/cifs/file.c +7
-1+ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter *from) +{ +return __cifs_writev (iocb, from, true); } + +ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) { +return __cifs_writev (iocb, from, false); } +static ssize_t cifs_writev (struct kiocb *iocb,struct iov_iter *from) { --Quoted 2 lines
2.7.4
Did you measure the performance benefit of using
O_DIRECTwith your patches for non-RDMA mode?
Here are some of the results measured on TCP/IP over IPoIB 40G Infiniband. Please note that IPoIB is done in software so it's slower than a 40G Ethernet NIC.
All servers are running 2 X Intel(R) Xeon(R) CPU E5-2650 v3 @ 2.30GHz. Tested on FIO 64k read or write with queue depths 1 or 16. All the tests are running with 1 FIO job, and with "direct=1".
Without direct I/O:
read 64k d1 113669KB/s
read 64k d16 618428KB/s
write 64k d1 134911KB/s
write 64k d16 457005KB/s
With direct I/O:
read 64k d1 127134KB/s
read 64k d16 714629KB/s
write 64k d1 129268KB/s
write 64k d16 461054KB/s
Direct I/O improvement:
read 64k d1 11%
read 64k d16 15%
write 64k d1 -5%
write 64k d16 1%
Quoted 4 lines
--
Best regards,
Pavel Shilovsky
Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 28 нояб. 2018 г. в 18:20, Long Li <[email protected]>:
Quoted 355 lines
Quoted 323 lines
Subject: Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 31 окт. 2018 г. в 15:26, Long Li <[email protected]>:
Quoted 8 lines
From: Long Li <[email protected]>
With direct I/O write, user supplied buffers are pinned to the memory and data are transferred directly from user buffers to the transport layer.
Change in v3: add support for kernel AIO
Quoted 8 lines
Change in v4:
Refactor common write code to__cifs_writevfor direct and non-direct I/O. Retry on direct I/O failure.Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/file.c | 194 +++++++++++++++++++++++++++++++++++++++++++------------
Quoted 24 lines
2 files changed, 154 insertions(+), 41 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-07fba9aa..e9c5103 100644
fs/cifs/cifsfs.h +0
-0@@ -105,6 +105,7 @@ extern ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to); externssize_tcifs_direct_readv(struct kiocb *iocb, struct iov_iter *to); externssize_tcifs_strict_readv(struct kiocb *iocb, struct iov_iter *to); externssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter +*from); extern ssize_t cifs_strict_writev(struct kiocb *iocb, struct iov_iter*from); extern int
cifs_lock(struct file *, int, struct file_lock *); extern intcifs_fsync(struct file *, loff_t, loff_t, int); diff --gita/fs/cifs/file.cb/fs/cifs/file.cindex daab878..1a41c04 100644fs/cifs/file.c +1
-0@@ -2524,6 +2524,55 @@ wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from, }static int +cifs_resend_wdata(struct cifs_writedata *wdata, struct list_headfs/cifs/file.c wdata_fill_from_iovec(struct cifs_writedata · line 2537
Quoted 10 lines
fs/cifs/file.c +10
-0+*wdata_list, struct cifs_aio_ctx *ctx) { +int wait_retry =0 ; +unsigned int wsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (wdata->cfile->tlink)->ses->server; + +/* +* Try to resend this wdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole wdata not in segments
Quoted 44 lines
fs/cifs/file.c +41
-0+*/ +do { + rc = server->ops->wait_mtu_credits (server, + wdata->bytes, &wsize, &credits); + +if (rc) +break ; + +if (wsize < wdata->bytes) { +add_credits_and_wake_if (server, credits,0 ); +msleep (1000 ); + wait_retry++; + } + }while (wsize < wdata->bytes && wait_retry <3 ); + +if (wsize < wdata->bytes) { + rc = -EBUSY ; + goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!wdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (wdata->cfile, false))) + rc = server->ops->async_writev (wdata, + + cifs_uncached_writedata_release); + +if (!rc) { +list_add_tail (&wdata->list, wdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, wdata->credits,0 ); +out: +kref_put (&wdata->refcount, cifs_uncached_writedata_release); + +return rc; +} + +static int cifs_write_from_iter (loff_t offset,size_t len,struct iov_iter *from,struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, struct list_headQuoted 1 line
*wdata_list, @@ -2537,6 +2586,8 @@ cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len,
Quoted 9 lines
loff_t saved_offset = offset; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +2
-0+struct page **pagevec; +size_t start;if (cifs_sb->mnt_cifs_flags &CIFS_MOUNT_RWPIDFORWARD ) pid = open_file->pid;@@ -2553,38 +2604,74 @@ cifs_write_from_iter(loff_t offset, size_t len,struct
iov_iter*from,Quoted 73 lines
fs/cifs/file.c +39
-27if (rc)break ; - nr_pages =get_numpages (wsize, len, &cur_len); - wdata =cifs_writedata_alloc (nr_pages, +if (ctx->direct_io) { + cur_len =iov_iter_get_pages_alloc ( + from, &pagevec, wsize, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"direct_writev couldn't get user pages " +"(rc=%zd) iter type %d iov_offset %zd count" +" %zd\n" , + cur_len, from->type, + from->iov_offset, from->count); +dump_stack (); +break ; + } +iov_iter_advance (from, cur_len); + + nr_pages = (cur_len + start +PAGE_SIZE -1 ) / +PAGE_SIZE ; + + wdata =cifs_writedata_direct_alloc (pagevec, cifs_uncached_writev_complete); -if (!wdata) { - rc = -ENOMEM ; -add_credits_and_wake_if (server, credits,0 ); -break ; - } +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } - rc =cifs_write_allocate_pages (wdata->pages, nr_pages); -if (rc) { -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } - num_pages = nr_pages; - rc =wdata_fill_from_iovec (wdata, from, &cur_len, &num_pages); -if (rc) { -for (i =0 ; i < nr_pages; i++) -put_page (wdata->pages[i]); -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } + wdata->page_offset = start; + wdata->tailsz = + nr_pages >1 ? + cur_len - (PAGE_SIZE - start) - + (nr_pages -2 ) *PAGE_SIZE : + cur_len; + }else { + nr_pages =get_numpages (wsize, len, &cur_len); + wdata =cifs_writedata_alloc (nr_pages, + cifs_uncached_writev_complete); +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } -/* -* Bring nr_pages down to the number of pages we actually used, -* and free any pages that we didn't use. -*/ -for ( ; nr_pages > num_pages; nr_pages--)fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 10 lines
fs/cifs/file.c +9
-1-put_page (wdata->pages[nr_pages -1 ]); + rc =cifs_write_allocate_pages (wdata->pages, nr_pages); +if (rc) { +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + + num_pages = nr_pages; + rc = wdata_fill_from_iovec(wdata, from, &cur_len,
&num_pages);fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 10 lines
fs/cifs/file.c +10
-0+if (rc) { +for (i =0 ; i < nr_pages; i++) +put_page (wdata->pages[i]); +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + +/* +* Bring nr_pages down to the number of pages we actually used,
fs/cifs/file.c static void collect_uncached_write_data(struct
Quoted 41 lines
fs/cifs/file.c +7
-0+* and free any pages that we didn't use. +*/ +for ( ; nr_pages > num_pages; nr_pages--) +put_page (wdata->pages[nr_pages -1 ]); + + wdata->tailsz = cur_len - ((nr_pages -1 ) *PAGE_SIZE ); + } wdata->sync_mode =WB_SYNC_ALL ; wdata->nr_pages = nr_pages; @@ -2593,7 +2680,6 @@cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,wdata->pid = pid; wdata->bytes = cur_len; wdata->pagesz = PAGE_SIZE; - wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE); wdata->credits = credits; wdata->ctx = ctx; kref_get(&ctx->refcount); @@ -2668,13 +2754,17 @@ static void collect_uncached_write_data(struct cifs_aio_ctx *ctx) INIT_LIST_HEAD(&tmp_list); list_del_init(&wdata->list);fs/cifs/file.c +4
-1- iov_iter_advance(&tmp_from, +if (ctx->direct_io) + rc =cifs_resend_wdata (wdata, &tmp_list, ctx); +else { + iov_iter_advance(&tmp_from, wdata->offset -
ctx->pos);fs/cifs/file.c +3
-1- rc =cifs_write_from_iter (wdata->offset, + rc = +cifs_write_from_iter (wdata->offset, wdata->bytes, &tmp_from, ctx->cfile, cifs_sb, &tmp_list, ctx); + }list_splice (&tmp_list, &ctx->list);@@ -2687,8 +2777,9 @@ static void collect_uncached_write_data(struct
cifs_aio_ctx*ctx)Quoted 11 lines
fs/cifs/file.c +3
-2kref_put (&wdata->refcount, cifs_uncached_writedata_release); } -for (i =0 ; i < ctx->npages; i++) -put_page (ctx->bv[i].bv_page); +if (!ctx->direct_io) +for (i =0 ; i < ctx->npages; i++) +put_page (ctx->bv[i].bv_page);cifs_stats_bytes_written (tcon, ctx->total_len); set_bit(CIFS_INO_INVALID_MAPPING,Quoted 1 line
&CIFS_I(dentry->d_inode)->flags); @@ -2703,7 +2794,7 @@ static void
collect_uncached_write_data(struct cifs_aio_ctx *ctx)fs/cifs/file.c ssize_t cifs_user_writev(struct kiocb *iocb,
Quoted 10 lines
complete(&ctx->done);}
fs/cifs/file.c +2
-1-ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) +static ssize_t __cifs_writev (struct kiocb *iocb,struct iov_iter +*from,bool direct) {struct file *file = iocb->ki_filp;ssize_t total_written =0 ;@@ -2712,13 +2803,18 @@ ssize_t cifs_user_writev(struct kiocb *iocb,struct
iov_iter*from)fs/cifs/file.c ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)
Quoted 9 lines
fs/cifs/file.c +1
-2struct cifs_sb_info *cifs_sb;struct cifs_aio_ctx *ctx;struct iov_iter saved_from = *from; +size_t len =iov_iter_count (from);int rc;/* -* BB - optimize the way when signing is disabled. We can drop this -* extra memory-to-memory copying and use iovec buffers for constructing
Quoted 35 lines
fs/cifs/file.c +18
-5-* write request. +* iov_iter_get_pages_alloc doesn't work with ITER_KVEC. +* In this case, fall back to non-direct write function. +* this could be improved by getting pages directly in +ITER_KVEC */ +if (direct && from->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_writev for kvec I/O\n" ); + direct = false; + } rc =generic_write_checks (iocb, from);if (rc <=0 )@@ -2742,10 +2838,16 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from) ctx->pos = iocb->ki_pos; - rc =setup_aio_ctx_iter (ctx, from,WRITE ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->direct_io = true; + ctx->iter = *from; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, from,WRITE ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } }/* grab a lock here due to read response handlers can accessQuoted 1 line
ctx */ @@ -2795,6 +2897,16 @@
ssize_tcifs_user_writev(struct kiocb*iocb, struct
iov_iter*from)Quoted 3 lines
return total_written;}
Quoted 15 lines
Quoted 10 lines
fs/cifs/file.c +7
-1+ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter *from) +{ +return __cifs_writev (iocb, from, true); } + +ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) { +return __cifs_writev (iocb, from, false); } +static ssize_t cifs_writev (struct kiocb *iocb,struct iov_iter *from) { --Quoted 2 lines
2.7.4
Did you measure the performance benefit of using
O_DIRECTwith your patches for non-RDMA mode?Here are some of the results measured on TCP/IP over IPoIB 40G Infiniband. Please note that IPoIB is done in software so it's slower than a 40G Ethernet NIC.
All servers are running 2 X Intel(R) Xeon(R) CPU E5-2650 v3 @ 2.30GHz. Tested on FIO 64k read or write with queue depths 1 or 16. All the tests are running with 1 FIO job, and with "direct=1".
Without direct I/O:
read 64k d1 113669KB/s read 64k d16 618428KB/s write 64k d1 134911KB/s write 64k d16 457005KB/sWith direct I/O:
read 64k d1 127134KB/s read 64k d16 714629KB/s write 64k d1 129268KB/s write 64k d16 461054KB/s
Quoted 4 lines
Direct I/O improvement:
read 64k d1 11% read 64k d16 15% write 64k d1 -5%
^^^
This is strange. Is it consistent across multiple runs?
Quoted 1 line
write 64k d16 1%
Good read performance improvements overall and it seems write performance results need more investigations.
--
Best regards,
Pavel Shilovsky
RE: [Patch v4 2/3] CIFS: Add support for direct I/O write
Quoted 386 lines
Subject: Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 28 нояб. 2018 г. в 18:20, Long Li <[email protected]>:
Quoted 9 lines
Quoted 9 lines
Subject: Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 31 окт. 2018 г. в 15:26, Long Li <[email protected]>:
Quoted 5 lines
From: Long Li <[email protected]>
With direct I/O write, user supplied buffers are pinned to the memory and data are transferred directly from user buffers to the
transport layer.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Change in v3: add support for kernel AIO
Change in v4:
Refactor common write code to__cifs_writevfor direct and non-directI/O.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Retry on direct I/O failure.
Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/file.c | 194+++++++++++++++++++++++++++++++++++++++++++----
Quoted 141 lines
Quoted 141 lines
--------
Quoted 27 lines
2 files changed, 154 insertions(+), 41 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-07fba9aa..e9c5103 100644
fs/cifs/cifsfs.h +0
-0@@ -105,6 +105,7 @@ extern ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to); extern ssize_tcifs_direct_readv (struct kiocb *iocb,struct iov_iter *to);extern
ssize_tcifs_strict_readv(struct kiocb *iocb, struct iov_iter *to); externssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_writev (struct kiocb *iocb,struct +iov_iter *from); extern ssize_t cifs_strict_writev(struct kiocb *iocb, struct
iov_iter*from); extern intcifs_lock(struct file *, int, struct file_lock *); extern intcifs_fsync(struct file *, loff_t, loff_t, int); diff --gita/fs/cifs/file.cb/fs/cifs/file.cindex daab878..1a41c04 100644fs/cifs/file.c +2
-0@@ -2524,6 +2524,55 @@ wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from, }static int +cifs_resend_wdata (struct cifs_writedata *wdata,struct list_head +*wdata_list,struct cifs_aio_ctx *ctx) {fs/cifs/file.c wdata_fill_from_iovec(struct cifs_writedata · line 2538
Quoted 10 lines
fs/cifs/file.c +10
-0+int wait_retry =0 ; +unsigned int wsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (wdata->cfile->tlink)->ses->server; + +/* +* Try to resend this wdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole wdata not +in segments
Quoted 45 lines
fs/cifs/file.c +42
-0+*/ +do { + rc = server->ops->wait_mtu_credits (server, + wdata->bytes, &wsize, &credits); + +if (rc) +break ; + +if (wsize < wdata->bytes) { +add_credits_and_wake_if (server, credits,0 ); +msleep (1000 ); + wait_retry++; + } + }while (wsize < wdata->bytes && wait_retry <3 ); + +if (wsize < wdata->bytes) { + rc = -EBUSY ; + goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!wdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (wdata->cfile, false))) + rc = server->ops->async_writev (wdata, + + cifs_uncached_writedata_release); + +if (!rc) { +list_add_tail (&wdata->list, wdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, wdata->credits,0 ); +out: +kref_put (&wdata->refcount, +cifs_uncached_writedata_release); + +return rc; +} + +static int cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from, struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, structQuoted 2 lines
list_head*wdata_list, @@ -2537,6 +2586,8 @@
cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,Quoted 9 lines
loff_t saved_offset = offset; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +2
-0+struct page **pagevec; +size_t start;if (cifs_sb->mnt_cifs_flags &CIFS_MOUNT_RWPIDFORWARD ) pid = open_file->pid; @@ -2553,38 +2604,74 @@cifs_write_from_iter(loff_t offset,
size_tlen,struct
iov_iter*from,Quoted 44 lines
if (rc) break;fs/cifs/file.c +25
-13- nr_pages =get_numpages (wsize, len, &cur_len); - wdata =cifs_writedata_alloc (nr_pages, +if (ctx->direct_io) { + cur_len =iov_iter_get_pages_alloc ( + from, &pagevec, wsize, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"direct_writev couldn't get user pages " +"(rc=%zd) iter type %d iov_offset %zd count" +" %zd\n" , + cur_len, from->type, + from->iov_offset, from->count); +dump_stack (); +break ; + } +iov_iter_advance (from, cur_len); + + nr_pages = (cur_len + start +PAGE_SIZE - +1 ) /PAGE_SIZE ; + + wdata = +cifs_writedata_direct_alloc (pagevec, cifs_uncached_writev_complete); -if (!wdata) { - rc = -ENOMEM ; -add_credits_and_wake_if (server, credits,0 ); -break ; - } +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } - rc =cifs_write_allocate_pages (wdata->pages, nr_pages); -if (rc) { -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - }Quoted 2 lines
Quoted 2 lines
fs/cifs/file.c
Quoted 2 lines
fs/cifs/file.c +0
-2- num_pages = nr_pages; - rc = wdata_fill_from_iovec(wdata, from, &cur_len,
&num_pages);Quoted 23 lines
Quoted 23 lines
Quoted 23 lines
fs/cifs/file.c +15
-7-if (rc) { -for (i =0 ; i < nr_pages; i++) -put_page (wdata->pages[i]); -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } + wdata->page_offset = start; + wdata->tailsz = + nr_pages >1 ? + cur_len - (PAGE_SIZE - start) - + (nr_pages -2 ) *PAGE_SIZE : + cur_len; + }else { + nr_pages =get_numpages (wsize, len, &cur_len); + wdata =cifs_writedata_alloc (nr_pages, + cifs_uncached_writev_complete); +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + }Quoted 2 lines
Quoted 2 lines
fs/cifs/file.c
Quoted 2 lines
fs/cifs/file.c +0
-2-/* -* Bring nr_pages down to the number of pages we actually used,
Quoted 67 lines
Quoted 67 lines
Quoted 4 lines
fs/cifs/file.c +0
-4-* and free any pages that we didn't use. -*/ -for ( ; nr_pages > num_pages; nr_pages--) -put_page (wdata->pages[nr_pages -1 ]);fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 10 lines
fs/cifs/file.c +10
-0+ rc =cifs_write_allocate_pages (wdata->pages, nr_pages); +if (rc) { +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + + num_pages = nr_pages; + rc = wdata_fill_from_iovec(wdata, from, + &cur_len,
&num_pages);fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 11 lines
fs/cifs/file.c +11
-0+if (rc) { +for (i =0 ; i < nr_pages; i++) +put_page (wdata->pages[i]); +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + +/* +* Bring nr_pages down to the number of +pages we actually used,
Quoted 40 lines
fs/cifs/file.c +8
-0+* and free any pages that we didn't use. +*/ +for ( ; nr_pages > num_pages; nr_pages--) +put_page (wdata->pages[nr_pages - +1 ]); + + wdata->tailsz = cur_len - ((nr_pages -1 ) *PAGE_SIZE ); + } wdata->sync_mode =WB_SYNC_ALL ; wdata->nr_pages = nr_pages; @@ -2593,7 +2680,6 @@cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,wdata->pid = pid; wdata->bytes = cur_len; wdata->pagesz = PAGE_SIZE; - wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE); wdata->credits = credits; wdata->ctx = ctx; kref_get(&ctx->refcount); @@ -2668,13 +2754,17 @@ static void collect_uncached_write_data(struct cifs_aio_ctx *ctx) INIT_LIST_HEAD(&tmp_list); list_del_init(&wdata->list);fs/cifs/file.c +5
-1- iov_iter_advance(&tmp_from, +if (ctx->direct_io) + rc =cifs_resend_wdata (wdata, &tmp_list, ctx); +else { + + iov_iter_advance(&tmp_from, wdata->offset -
ctx->pos);fs/cifs/file.c +3
-1- rc =cifs_write_from_iter (wdata->offset, + rc = +cifs_write_from_iter (wdata->offset, wdata->bytes, &tmp_from, ctx->cfile, cifs_sb, &tmp_list, ctx); + }Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
list_splice(&tmp_list,
&ctx->list);fs/cifs/file.c +0
-0@@ -2687,8 +2777,9 @@ static voidcollect_uncached_write_data(struct
cifs_aio_ctx*ctx)Quoted 1 line
kref_put(&wdata->refcount,
cifs_uncached_writedata_release);Quoted 91 lines
Quoted 75 lines
Quoted 10 lines
}fs/cifs/file.c +3
-2-for (i =0 ; i < ctx->npages; i++) -put_page (ctx->bv[i].bv_page); +if (!ctx->direct_io) +for (i =0 ; i < ctx->npages; i++) +put_page (ctx->bv[i].bv_page);cifs_stats_bytes_written (tcon, ctx->total_len); set_bit(CIFS_INO_INVALID_MAPPING,Quoted 2 lines
&CIFS_I(dentry->d_inode)->flags); @@ -2703,7 +2794,7 @@ static void
collect_uncached_write_data(struct cifs_aio_ctx *ctx)Quoted 10 lines
complete(&ctx->done); }-ssize_t
cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)fs/cifs/file.c +2
-0+static ssize_t __cifs_writev (struct kiocb *iocb,struct iov_iter +*from,bool direct) {struct file *file = iocb->ki_filp; ssize_t total_written = 0; @@ -2712,13 +2803,18 @@ ssize_tcifs_user_writev(struct kiocb *iocb,
struct
iov_iter*from)fs/cifs/file.c ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)
Quoted 9 lines
struct cifs_sb_info *cifs_sb; struct cifs_aio_ctx *ctx; struct iov_iter saved_from = *from;fs/cifs/file.c +1
-2+size_t len =iov_iter_count (from);int rc;/* -* BB - optimize the way when signing is disabled. We can drop this -* extra memory-to-memory copying and use iovec buffers for constructing
Quoted 35 lines
fs/cifs/file.c +18
-5-* write request. +* iov_iter_get_pages_alloc doesn't work with ITER_KVEC. +* In this case, fall back to non-direct write function. +* this could be improved by getting pages directly in +ITER_KVEC */ +if (direct && from->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_writev for kvec I/O\n" ); + direct = false; + } rc =generic_write_checks (iocb, from);if (rc <=0 )@@ -2742,10 +2838,16 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from) ctx->pos = iocb->ki_pos; - rc =setup_aio_ctx_iter (ctx, from,WRITE ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->direct_io = true; + ctx->iter = *from; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, from,WRITE ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } }/* grab a lock here due to read response handlers canQuoted 2 lines
access ctx */ @@ -2795,6 +2897,16 @@
ssize_t
cifs_user_writev(struct kiocb*iocb, struct
iov_iter*from)Quoted 3 lines
return total_written;}
Quoted 15 lines
Quoted 10 lines
fs/cifs/file.c +7
-1+ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter +*from) { +return __cifs_writev (iocb, from, true); } + +ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) { +return __cifs_writev (iocb, from, false); } +static ssize_t cifs_writev (struct kiocb *iocb,struct iov_iter *from) { --Quoted 2 lines
2.7.4
Did you measure the performance benefit of using
O_DIRECTwith your patches for non-RDMA mode?Quoted 1 line
Here are some of the results measured on TCP/IP over IPoIB 40G Infiniband.
Please note that IPoIB is done in software so it's slower than a 40G Ethernet NIC.
Quoted 2 lines
All servers are running 2 X Intel(R) Xeon(R) CPU E5-2650 v3 @ 2.30GHz.
Tested on FIO 64k read or write with queue depths 1 or 16. All the tests are running with 1 FIO job, and with "direct=1".
Quoted 13 lines
Without direct I/O:
read 64k d1 113669KB/s read 64k d16 618428KB/s write 64k d1 134911KB/s write 64k d16 457005KB/sWith direct I/O:
read 64k d1 127134KB/s read 64k d16 714629KB/s write 64k d1 129268KB/s write 64k d16 461054KB/sQuoted 4 lines
Direct I/O improvement:
read 64k d1 11% read 64k d16 15% write 64k d1 -5%
Quoted 7 lines
^^^This is strange. Is it consistent across multiple runs?
Quoted 1 line
write 64k d16 1%
Good read performance improvements overall and it seems write performance results need more investigations.
My apologies, I found out that I was doing it wrong for comparing write tests. They were actually all direct I/O, 5% difference is due to benchmark variation at the time of tests.
Here are the re-run (3 times, each time for 90 seconds) for 64k write. This time, Linux client, Windows server and 40G IB switch are rebooted prior to tests.
direct I/O
64k write d16
470m/s
616m/s
479m/s
64k write d1
176m/s
160m/s
156m/s
without direct I/O
64k write d16
534m/s
405m/s
406m/s
64k write d1
146m/s
147m/s
147m/s
I'm not sure why test results at depth 16 are jumpy, but overall there are improvements in direct I/O for 64k writes.
RE: [Patch v4 2/3] CIFS: Add support for direct I/O write
Quoted 431 lines
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Long Li
Sent: Thursday, November 29, 2018 4:30 PM
To: Pavel Shilovsky <[email protected]>
Cc: Steve French <[email protected]>; linux-cifs <[email protected]>; samba-technical <[email protected]>; Kernel Mailing List <[email protected]>
Subject: RE: [Patch v4 2/3] CIFS: Add support for direct I/O writeQuoted 386 lines
Subject: Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 28 нояб. 2018 г. в 18:20, Long Li <[email protected]>:
Quoted 9 lines
Quoted 9 lines
Subject: Re: [Patch v4 2/3] CIFS: Add support for direct I/O write
ср, 31 окт. 2018 г. в 15:26, Long Li <[email protected]>:
Quoted 5 lines
From: Long Li <[email protected]>
With direct I/O write, user supplied buffers are pinned to the memory and data are transferred directly from user buffers to the
transport layer.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Change in v3: add support for kernel AIO
Change in v4:
Refactor common write code to__cifs_writevfor direct and non-directI/O.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Retry on direct I/O failure.
Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/file.c | 194+++++++++++++++++++++++++++++++++++++++++++----
Quoted 141 lines
Quoted 141 lines
--------
Quoted 27 lines
2 files changed, 154 insertions(+), 41 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-07fba9aa..e9c5103 100644
fs/cifs/cifsfs.h +0
-0@@ -105,6 +105,7 @@ extern ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to); extern ssize_tcifs_direct_readv (struct kiocb *iocb,struct iov_iter *to);extern
ssize_tcifs_strict_readv(struct kiocb *iocb, struct iov_iter *to); externssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_writev (struct kiocb *iocb,struct +iov_iter *from); extern ssize_t cifs_strict_writev(struct kiocb *iocb, struct
iov_iter*from); extern intcifs_lock(struct file *, int, struct file_lock *); extern intcifs_fsync(struct file *, loff_t, loff_t, int); diff --gita/fs/cifs/file.cb/fs/cifs/file.cindex daab878..1a41c04 100644fs/cifs/file.c +2
-0@@ -2524,6 +2524,55 @@ wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from, }static int +cifs_resend_wdata (struct cifs_writedata *wdata,struct list_head +*wdata_list,struct cifs_aio_ctx *ctx) {fs/cifs/file.c wdata_fill_from_iovec(struct cifs_writedata · line 2538
Quoted 10 lines
fs/cifs/file.c +10
-0+int wait_retry =0 ; +unsigned int wsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (wdata->cfile->tlink)->ses->server; + +/* +* Try to resend this wdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole wdata not +in segments
Quoted 45 lines
fs/cifs/file.c +42
-0+*/ +do { + rc = server->ops->wait_mtu_credits (server, + wdata->bytes, &wsize, &credits); + +if (rc) +break ; + +if (wsize < wdata->bytes) { +add_credits_and_wake_if (server, credits,0 ); +msleep (1000 ); + wait_retry++; + } + }while (wsize < wdata->bytes && wait_retry <3 ); + +if (wsize < wdata->bytes) { + rc = -EBUSY ; + goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!wdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (wdata->cfile, false))) + rc = server->ops->async_writev (wdata, + + cifs_uncached_writedata_release); + +if (!rc) { +list_add_tail (&wdata->list, wdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, wdata->credits,0 ); +out: +kref_put (&wdata->refcount, +cifs_uncached_writedata_release); + +return rc; +} + +static int cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from, struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, structQuoted 2 lines
list_head*wdata_list, @@ -2537,6 +2586,8 @@
cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,Quoted 9 lines
loff_t saved_offset = offset; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +2
-0+struct page **pagevec; +size_t start;if (cifs_sb->mnt_cifs_flags &CIFS_MOUNT_RWPIDFORWARD ) pid = open_file->pid; @@ -2553,38 +2604,74 @@cifs_write_from_iter(loff_t offset,
size_tlen,struct
iov_iter*from,Quoted 44 lines
if (rc) break;fs/cifs/file.c +25
-13- nr_pages =get_numpages (wsize, len, &cur_len); - wdata =cifs_writedata_alloc (nr_pages, +if (ctx->direct_io) { + cur_len =iov_iter_get_pages_alloc ( + from, &pagevec, wsize, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"direct_writev couldn't get user pages " +"(rc=%zd) iter type %d iov_offset %zd count" +" %zd\n" , + cur_len, from->type, + from->iov_offset, from->count); +dump_stack (); +break ; + } +iov_iter_advance (from, cur_len); + + nr_pages = (cur_len + start +PAGE_SIZE - +1 ) /PAGE_SIZE ; + + wdata = +cifs_writedata_direct_alloc (pagevec, cifs_uncached_writev_complete); -if (!wdata) { - rc = -ENOMEM ; -add_credits_and_wake_if (server, credits,0 ); -break ; - } +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + } - rc =cifs_write_allocate_pages (wdata->pages, nr_pages); -if (rc) { -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - }Quoted 2 lines
Quoted 2 lines
fs/cifs/file.c
Quoted 2 lines
fs/cifs/file.c +0
-2- num_pages = nr_pages; - rc = wdata_fill_from_iovec(wdata, from, &cur_len,
&num_pages);Quoted 23 lines
Quoted 23 lines
Quoted 23 lines
fs/cifs/file.c +15
-7-if (rc) { -for (i =0 ; i < nr_pages; i++) -put_page (wdata->pages[i]); -kfree (wdata); -add_credits_and_wake_if (server, credits,0 ); -break ; - } + wdata->page_offset = start; + wdata->tailsz = + nr_pages >1 ? + cur_len - (PAGE_SIZE - start) - + (nr_pages -2 ) *PAGE_SIZE : + cur_len; + }else { + nr_pages =get_numpages (wsize, len, &cur_len); + wdata =cifs_writedata_alloc (nr_pages, + cifs_uncached_writev_complete); +if (!wdata) { + rc = -ENOMEM ; +add_credits_and_wake_if (server, credits,0 ); +break ; + }Quoted 2 lines
Quoted 2 lines
fs/cifs/file.c
Quoted 2 lines
fs/cifs/file.c +0
-2-/* -* Bring nr_pages down to the number of pages we actually used,
Quoted 67 lines
Quoted 67 lines
Quoted 4 lines
fs/cifs/file.c +0
-4-* and free any pages that we didn't use. -*/ -for ( ; nr_pages > num_pages; nr_pages--) -put_page (wdata->pages[nr_pages -1 ]);fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 10 lines
fs/cifs/file.c +10
-0+ rc =cifs_write_allocate_pages (wdata->pages, nr_pages); +if (rc) { +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + + num_pages = nr_pages; + rc = wdata_fill_from_iovec(wdata, from, + &cur_len,
&num_pages);fs/cifs/file.c cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
Quoted 11 lines
fs/cifs/file.c +11
-0+if (rc) { +for (i =0 ; i < nr_pages; i++) +put_page (wdata->pages[i]); +kfree (wdata); +add_credits_and_wake_if (server, credits,0 ); +break ; + } + +/* +* Bring nr_pages down to the number of +pages we actually used,
Quoted 40 lines
fs/cifs/file.c +8
-0+* and free any pages that we didn't use. +*/ +for ( ; nr_pages > num_pages; nr_pages--) +put_page (wdata->pages[nr_pages - +1 ]); + + wdata->tailsz = cur_len - ((nr_pages -1 ) *PAGE_SIZE ); + } wdata->sync_mode =WB_SYNC_ALL ; wdata->nr_pages = nr_pages; @@ -2593,7 +2680,6 @@cifs_write_from_iter(loff_t offset,
size_tlen, structiov_iter*from,wdata->pid = pid; wdata->bytes = cur_len; wdata->pagesz = PAGE_SIZE; - wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE); wdata->credits = credits; wdata->ctx = ctx; kref_get(&ctx->refcount); @@ -2668,13 +2754,17 @@ static void collect_uncached_write_data(struct cifs_aio_ctx *ctx) INIT_LIST_HEAD(&tmp_list); list_del_init(&wdata->list);fs/cifs/file.c +5
-1- iov_iter_advance(&tmp_from, +if (ctx->direct_io) + rc =cifs_resend_wdata (wdata, &tmp_list, ctx); +else { + + iov_iter_advance(&tmp_from, wdata->offset -
ctx->pos);fs/cifs/file.c +3
-1- rc =cifs_write_from_iter (wdata->offset, + rc = +cifs_write_from_iter (wdata->offset, wdata->bytes, &tmp_from, ctx->cfile, cifs_sb, &tmp_list, ctx); + }Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
list_splice(&tmp_list,
&ctx->list);fs/cifs/file.c +0
-0@@ -2687,8 +2777,9 @@ static voidcollect_uncached_write_data(struct
cifs_aio_ctx*ctx)Quoted 1 line
kref_put(&wdata->refcount,
cifs_uncached_writedata_release);Quoted 91 lines
Quoted 75 lines
Quoted 10 lines
}fs/cifs/file.c +3
-2-for (i =0 ; i < ctx->npages; i++) -put_page (ctx->bv[i].bv_page); +if (!ctx->direct_io) +for (i =0 ; i < ctx->npages; i++) +put_page (ctx->bv[i].bv_page);cifs_stats_bytes_written (tcon, ctx->total_len); set_bit(CIFS_INO_INVALID_MAPPING,Quoted 2 lines
&CIFS_I(dentry->d_inode)->flags); @@ -2703,7 +2794,7 @@ static void
collect_uncached_write_data(struct cifs_aio_ctx *ctx)Quoted 10 lines
complete(&ctx->done); }-ssize_t
cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)fs/cifs/file.c +2
-0+static ssize_t __cifs_writev (struct kiocb *iocb,struct iov_iter +*from,bool direct) {struct file *file = iocb->ki_filp; ssize_t total_written = 0; @@ -2712,13 +2803,18 @@ ssize_tcifs_user_writev(struct kiocb *iocb,
struct
iov_iter*from)fs/cifs/file.c ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)
Quoted 9 lines
struct cifs_sb_info *cifs_sb; struct cifs_aio_ctx *ctx; struct iov_iter saved_from = *from;fs/cifs/file.c +1
-2+size_t len =iov_iter_count (from);int rc;/* -* BB - optimize the way when signing is disabled. We can drop this -* extra memory-to-memory copying and use iovec buffers for constructing
Quoted 35 lines
fs/cifs/file.c +18
-5-* write request. +* iov_iter_get_pages_alloc doesn't work with ITER_KVEC. +* In this case, fall back to non-direct write function. +* this could be improved by getting pages directly in +ITER_KVEC */ +if (direct && from->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_writev for kvec I/O\n" ); + direct = false; + } rc =generic_write_checks (iocb, from);if (rc <=0 )@@ -2742,10 +2838,16 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from) ctx->pos = iocb->ki_pos; - rc =setup_aio_ctx_iter (ctx, from,WRITE ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->direct_io = true; + ctx->iter = *from; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, from,WRITE ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } }/* grab a lock here due to read response handlers canQuoted 2 lines
access ctx */ @@ -2795,6 +2897,16 @@
ssize_t
cifs_user_writev(struct kiocb*iocb, struct
iov_iter*from)Quoted 3 lines
return total_written;}
Quoted 15 lines
Quoted 10 lines
fs/cifs/file.c +7
-1+ssize_t cifs_direct_writev (struct kiocb *iocb,struct iov_iter +*from) { +return __cifs_writev (iocb, from, true); } + +ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from) { +return __cifs_writev (iocb, from, false); } +static ssize_t cifs_writev (struct kiocb *iocb,struct iov_iter *from) { --Quoted 2 lines
2.7.4
Did you measure the performance benefit of using
O_DIRECTwith your patches for non-RDMA mode?Quoted 1 line
Here are some of the results measured on TCP/IP over IPoIB 40G Infiniband.
Please note that IPoIB is done in software so it's slower than a 40G Ethernet NIC.
Quoted 2 lines
All servers are running 2 X Intel(R) Xeon(R) CPU E5-2650 v3 @ 2.30GHz.
Tested on FIO 64k read or write with queue depths 1 or 16. All the tests are running with 1 FIO job, and with "direct=1".
Quoted 13 lines
Without direct I/O:
read 64k d1 113669KB/s read 64k d16 618428KB/s write 64k d1 134911KB/s write 64k d16 457005KB/sWith direct I/O:
read 64k d1 127134KB/s read 64k d16 714629KB/s write 64k d1 129268KB/s write 64k d16 461054KB/sQuoted 4 lines
Direct I/O improvement:
read 64k d1 11% read 64k d16 15% write 64k d1 -5%Quoted 7 lines
^^^This is strange. Is it consistent across multiple runs?
Quoted 1 line
write 64k d16 1%
Good read performance improvements overall and it seems write performance results need more investigations.
My apologies, I found out that I was doing it wrong for comparing write tests. They were actually all direct I/O, 5% difference is due to benchmark variation at the time of tests.
Here are the re-run (3 times, each time for 90 seconds) for 64k write. This time, Linux client, Windows server and 40G IB switch are rebooted prior to tests.
direct I/O
64k write d16
470m/s
616m/s
479m/s64k write d1
176m/s
160m/s
156m/swithout direct I/O
64k write d16
534m/s
405m/s
406m/s
Quoted 7 lines
64k write d1
146m/s
147m/s
147m/sI'm not sure why test results at depth 16 are jumpy, but overall there are improvements in direct I/O for 64k writes.
Better! Regarding the "jumpy" results, it's likely from queuing effects creeping in. You might want to try doing a sweep of write queue depth, say 1, 4, 8, 16, and more, then comparing the results across them all, and ignoring the jumpy ones - assuming the jumpiness appears at about the same point. The idea being to compare the improvement at the maximum stable point. Getting onto a decent 10GbE NIC would also help. Still, it's clear there's a win here.
I'll chat with you offline about testing with John Hubbard's get_user_pages() RFC. Would be very interesting to compare using that, too.
Tom.
PATCH v4 3/3 CIFS: Add direct I/O functions to file_operations
In reply to Long LiFrom: Long Li <[email protected]>
With direct read/write functions implemented, add them to file_operations.
Dircet I/O is used under two conditions:
1. When mounting with "cache=none", CIFS uses direct I/O for all user file data transfer.
2. When opening a file with O_DIRECT, CIFS uses direct I/O for all data transfer on this file.
--- fs/cifs/cifsfs.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
fs/cifs/cifsfs.c +4 -6
@@ -1113,9 +1113,8 @@ const struct file_operations cifs_file_strict_ops = { };const struct file_operations cifs_file_direct_ops = { -/* BB reevaluate whether they can be done with directio, no cache */ - .read_iter = cifs_user_readv, - .write_iter = cifs_user_writev, + .read_iter = cifs_direct_readv, + .write_iter = cifs_direct_writev, .open = cifs_open, .release = cifs_close, .lock = cifs_lock,
@@ -1169,9 +1168,8 @@ const struct file_operations cifs_file_strict_nobrl_ops = { };const struct file_operations cifs_file_direct_nobrl_ops = { -/* BB reevaluate whether they can be done with directio, no cache */ - .read_iter = cifs_user_readv, - .write_iter = cifs_user_writev, + .read_iter = cifs_direct_readv, + .write_iter = cifs_direct_writev, .open = cifs_open, .release = cifs_close, .fsync = cifs_fsync,
Signature · 1 line
2.7.4
Re: [Patch v4 3/3] CIFS: Add direct I/O functions to file_operations
made minor cleanup to patch 1 and 2, added Ronnie's reviewed to patch 3, and tentatively merged to cifs-2.6.git for-next
Quoted 47 lines · Long Li
From: Long Li <[email protected]>
With direct read/write functions implemented, add them to
file_operations.Dircet I/O is used under two conditions:
1. When mounting with "cache=none", CIFS uses direct I/O for all user file data transfer.
2. When opening a file withO_DIRECT, CIFS uses direct I/O for all data transfer on this file.Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)fs/cifs/cifsfs.c +4
-7@@ -1113,9 +1113,8 @@ const struct file_operations cifs_file_strict_ops = { };const struct file_operations cifs_file_direct_ops = { -/* BB reevaluate whether they can be done with directio, no cache */ - .read_iter = cifs_user_readv, - .write_iter = cifs_user_writev, + .read_iter = cifs_direct_readv, + .write_iter = cifs_direct_writev, .open = cifs_open, .release = cifs_close, .lock = cifs_lock,@@ -1169,9 +1168,8 @@ const struct file_operations cifs_file_strict_nobrl_ops = { };const struct file_operations cifs_file_direct_nobrl_ops = { -/* BB reevaluate whether they can be done with directio, no cache */ - .read_iter = cifs_user_readv, - .write_iter = cifs_user_writev, + .read_iter = cifs_direct_readv, + .write_iter = cifs_direct_writev, .open = cifs_open, .release = cifs_close, .fsync = cifs_fsync, --2.7.4
Signature · 3 lines
Thanks, Steve
Hi Long,
Please find my comments below.
ср, 31 окт. 2018 г. в 15:14, Long Li <[email protected]>:
Quoted 87 lines
From: Long Li <[email protected]>
With direct I/O read, we transfer the data directly from transport layer to the user data buffer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common read code to__cifs_readvfor direct and non-direct I/O. Retry on direct I/O failure.Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/cifsglob.h | 5 ++ fs/cifs/file.c | 219 +++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 186 insertions(+), 39 deletions(-)fs/cifs/cifsfs.h +1
-0@@ -102,6 +102,7 @@ extern int cifs_open(struct inode *inode, struct file *file);extern int cifs_close (struct inode *inode,struct file *file);extern int cifs_closedir (struct inode *inode,struct file *file);extern ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to); +extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_strict_readv (struct kiocb *iocb,struct iov_iter *to);extern ssize_t cifs_user_writev (struct kiocb *iocb,struct iov_iter *from);extern ssize_t cifs_strict_writev (struct kiocb *iocb,struct iov_iter *from);fs/cifs/cifsglob.h +5
-0@@ -1146,6 +1146,11 @@ struct cifs_aio_ctx {unsigned int len;unsigned int total_len;bool should_dirty; +/* +* Indicates if this aio_ctx is for direct_io, +* If yes, iter is a copy of the user passed iov_iter +*/ +bool direct_io; };struct cifs_readdata ;fs/cifs/file.c +24
-1@@ -2965,7 +2965,6 @@ cifs_uncached_readdata_release(struct kref *refcount)kref_put (&rdata->ctx->refcount, cifs_aio_ctx_release);for (i =0 ; i < rdata->nr_pages; i++) {put_page (rdata->pages[i]); - rdata->pages[i] =NULL ; }cifs_readdata_release (refcount); }@@ -3092,6 +3091,63 @@ cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,return uncached_fill_pages (server, rdata, iter, iter->count); } +static int cifs_resend_rdata (struct cifs_readdata *rdata, +struct list_head *rdata_list, +struct cifs_aio_ctx *ctx) +{ +int wait_retry =0 ; +unsigned int rsize, credits; +int rc; +struct TCP_Server_Info *server =tlink_tcon (rdata->cfile->tlink)->ses->server; + +/* +* Try to resend this rdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole rdata not in segments +*/ + do { + rc = server->ops->wait_mtu_credits(server, rdata->bytes, + &rsize, &credits); + + if (rc) + break; + + if (rsize < rdata->bytes) { + add_credits_and_wake_if(server, credits, 0); + msleep(1000); + wait_retry++;
fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server, · line 3127
Quoted 10 lines
fs/cifs/file.c +10
-0+ } + }while (rsize < rdata->bytes && wait_retry <3 ); + +/* +* If we can't find enough credits to send this rdata +* release the rdata and return failure, this will pass +* whatever I/O amount we have finished to VFS. +*/ +if (rsize < rdata->bytes) { + rc = -EBUSY ;
We don't have enough credits and return EBUSY here...
Quoted 151 lines
fs/cifs/file.c +84
-16+ goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!rdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (rdata->cfile, true))) + rc = server->ops->async_readv (rdata); + +if (!rc) { +/* Add to aio pending list */ +list_add_tail (&rdata->list, rdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, rdata->credits,0 ); +out: +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + +return rc; +} + static int cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, struct list_head *rdata_list,@@ -3103,6 +3159,9 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,int rc;pid_t pid;struct TCP_Server_Info *server; +struct page **pagevec; +size_t start; +struct iov_iter direct_iov = ctx->iter; server =tlink_tcon (open_file->tlink)->ses->server;@@ -3111,6 +3170,9 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,else pid = current->tgid; +if (ctx->direct_io) +iov_iter_advance (&direct_iov, offset - ctx->pos); + do { rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize, &rsize, &credits);@@ -3118,20 +3180,56 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,break ; cur_len = min_t(const size_t, len, rsize); - npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); -/* allocate a readdata struct */ - rdata =cifs_readdata_alloc (npages, +if (ctx->direct_io) { + + cur_len =iov_iter_get_pages_alloc ( + &direct_iov, &pagevec, + cur_len, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"couldn't get user pages (cur_len=%zd)" +" iter type %d" +" iov_offset %zd count %zd\n" , + cur_len, direct_iov.type, direct_iov.iov_offset, + direct_iov.count); +dump_stack (); +break ; + } +iov_iter_advance (&direct_iov, cur_len); + + rdata =cifs_readdata_direct_alloc ( + pagevec, cifs_uncached_readv_complete); +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } + + npages = (cur_len + start +PAGE_SIZE -1 ) /PAGE_SIZE ; + rdata->page_offset = start; + rdata->tailsz = npages >1 ? + cur_len-(PAGE_SIZE -start)-(npages-2 )*PAGE_SIZE : + cur_len; + + }else { + + npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); +/* allocate a readdata struct */ + rdata =cifs_readdata_alloc (npages, cifs_uncached_readv_complete); -if (!rdata) { -add_credits_and_wake_if (server, credits,0 ); - rc = -ENOMEM ; -break ; - } +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } - rc =cifs_read_allocate_pages (rdata, npages); -if (rc) - goto error; + rc =cifs_read_allocate_pages (rdata, npages); +if (rc) + goto error; + + rdata->tailsz =PAGE_SIZE ; + } rdata->cfile =cifsFileInfo_get (open_file); rdata->nr_pages = npages;@@ -3139,7 +3237,6 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file, rdata->bytes = cur_len; rdata->pid = pid; rdata->pagesz =PAGE_SIZE ; - rdata->tailsz =PAGE_SIZE ; rdata->read_into_pages = cifs_uncached_read_into_pages; rdata->copy_into_pages = cifs_uncached_copy_into_pages; rdata->credits = credits;@@ -3153,9 +3250,11 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,if (rc) {add_credits_and_wake_if (server, rdata->credits,0 );kref_put (&rdata->refcount, - cifs_uncached_readdata_release); -if (rc == -EAGAIN ) + cifs_uncached_readdata_release); +if (rc == -EAGAIN ) { +iov_iter_revert (&direct_iov, cur_len);continue ; + }break ; }@@ -3211,45 +3310,62 @@ collect_uncached_read_data(struct cifs_aio_ctx *ctx)* reading. */ if (got_bytes && got_bytes < rdata->bytes) { - rc =cifs_readdata_to_iov (rdata, to); + rc =0 ; +if (!ctx->direct_io) + rc =cifs_readdata_to_iov (rdata, to);if (rc) {kref_put (&rdata->refcount, - cifs_uncached_readdata_release); + cifs_uncached_readdata_release);continue ; } }
fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx) · line 3330
Quoted 10 lines
fs/cifs/file.c +8
-1- rc = cifs_send_async_read( +if (ctx->direct_io) { +/* +* Re-use rdata as this is a +* direct I/O +*/ + rc =cifs_resend_rdata ( + rdata, + &tmp_list, ctx);
...so we set rc to -EBUSY...
Quoted 51 lines
fs/cifs/file.c +17
-9+ }else { + rc =cifs_send_async_read ( rdata->offset + got_bytes, rdata->bytes - got_bytes, rdata->cfile, cifs_sb, &tmp_list, ctx); +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + } +list_splice (&tmp_list, &ctx->list); -kref_put (&rdata->refcount, - cifs_uncached_readdata_release); goto again; }else if (rdata->result) rc = rdata->result; -else +else if (!ctx->direct_io) rc =cifs_readdata_to_iov (rdata, to);/* if there was a short read -- discard anything left */ if (rdata->got_bytes && rdata->got_bytes < rdata->bytes) rc = -ENODATA ; + + ctx->total_len += rdata->got_bytes; }list_del_init (&rdata->list);kref_put (&rdata->refcount, cifs_uncached_readdata_release); } -for (i =0 ; i < ctx->npages; i++) { -if (ctx->should_dirty) -set_page_dirty (ctx->bv[i].bv_page); -put_page (ctx->bv[i].bv_page); - } +if (!ctx->direct_io) { +for (i =0 ; i < ctx->npages; i++) { +if (ctx->should_dirty) +set_page_dirty (ctx->bv[i].bv_page); +put_page (ctx->bv[i].bv_page); + } - ctx->total_len = ctx->len -iov_iter_count (to); + ctx->total_len = ctx->len -iov_iter_count (to); + }cifs_stats_bytes_read (tcon, ctx->total_len);
fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx) · line 3383
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3267,18 +3383,27 @@ collect_uncached_read_data(struct cifs_aio_ctx *ctx)complete (&ctx->done);
and then ctx->rc is set to -EBUSY too.
Quoted 60 lines
fs/cifs/file.c +27
-12} -ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to) +ssize_t __cifs_readv (struct kiocb *iocb,struct iov_iter *to,bool direct) { -struct file *file = iocb->ki_filp; -ssize_t rc;size_t len; -ssize_t total_read =0 ; -loff_t offset = iocb->ki_pos; +struct file *file = iocb->ki_filp;struct cifs_sb_info *cifs_sb; -struct cifs_tcon *tcon;struct cifsFileInfo *cfile; +struct cifs_tcon *tcon; +ssize_t rc, total_read =0 ; +loff_t offset = iocb->ki_pos;struct cifs_aio_ctx *ctx; +/* +* iov_iter_get_pages_alloc() doesn't work with ITER_KVEC, +* fall back to data copy read path +* this could be improved by getting pages directly in ITER_KVEC +*/ +if (direct && to->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_user_readv for kvec I/O\n" ); + direct = false; + } + len =iov_iter_count (to);if (!len)return 0 ;@@ -3305,14 +3430,20 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)if (to->type ==ITER_IOVEC ) ctx->should_dirty = true; - rc =setup_aio_ctx_iter (ctx, to,READ ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->pos = offset; + ctx->direct_io = true; + ctx->iter = *to; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, to,READ ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } + len = ctx->len; } - len = ctx->len; -/* grab a lock here due to read response handlers can access ctx */ mutex_lock (&ctx->aio_mutex);
fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to) · line 3485
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3354,6 +3485,16 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)return rc;
In case of a blocking read we will return -EBUSY to the caller but read man page doesn't say anything about a possibility to return this error code. Is it being mapped somehow by VFS or is there another consideration? The same question applies to the write patch as well.
Quoted 16 lines
fs/cifs/file.c +10
-1} +ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter *to) +{ +return __cifs_readv (iocb, to, true); +} + +ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to) +{ +return __cifs_readv (iocb, to, false); +} +ssize_t cifs_strict_readv (struct kiocb *iocb,struct iov_iter *to) { --
Quoted 2 lines
2.7.4
--
Best regards,
Pavel Shilovsky
Quoted 399 lines
Subject: Re: [Patch v4 1/3] CIFS: Add support for direct I/O read
Hi Long,
Please find my comments below.
ср, 31 окт. 2018 г. в 15:14, Long Li <[email protected]>:
Quoted 12 lines
From: Long Li <[email protected]>
With direct I/O read, we transfer the data directly from transport layer to the user data buffer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common read code to__cifs_readvfor direct and non-direct I/O. Retry on direct I/O failure.Quoted 5 lines
Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/cifsglob.h | 5 ++ fs/cifs/file.c | 219 +++++++++++++++++++++++++++++++++++++++++++----------
fs/cifs/file.c cifs_uncached_readdata_release(struct kref
Quoted 37 lines
3 files changed, 186 insertions(+), 39 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-05f02318..7fba9aa 100644
fs/cifs/cifsfs.h +0
-0@@ -102,6 +102,7 @@ extern int cifs_open(struct inode *inode, structfile *file); extern int
cifs_close(struct inode *inode, struct file *file); extern intcifs_closedir(struct inode *inode, struct file *file); externssize_tcifs_user_readv(struct kiocb *iocb, struct iov_iter *to);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter +*to); extern ssize_t cifs_strict_readv(struct kiocb *iocb, struct iov_iter*to); extern
ssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from); externssize_tcifs_strict_writev(struct kiocb *iocb, struct iov_iter *from); diff --gita/fs/cifs/cifsglob.hb/fs/cifs/cifsglob.hindex 7f62c98..52248dd 100644fs/cifs/cifsglob.h +5
-0@@ -1146,6 +1146,11 @@ struct cifs_aio_ctx {unsigned int len;unsigned int total_len;bool should_dirty; +/* +* Indicates if this aio_ctx is for direct_io, +* If yes, iter is a copy of the user passed iov_iter +*/ +bool direct_io; };struct cifs_readdata ;fs/cifs/file.c index 87eece6..daab878 renamed from fs/cifs/file.c +0
-0100644
fs/cifs/file.c +0
-0@@ -2965,7 +2965,6 @@ cifs_uncached_readdata_release(struct kref*refcount)
Quoted 34 lines
fs/cifs/file.c +0
-1kref_put (&rdata->ctx->refcount, cifs_aio_ctx_release);for (i =0 ; i < rdata->nr_pages; i++) {put_page (rdata->pages[i]); - rdata->pages[i] =NULL ; } cifs_readdata_release(refcount); } @@ -3092,6 +3091,63 @@ cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,return uncached_fill_pages(server, rdata, iter, iter->count); }fs/cifs/file.c +24
-0+static int cifs_resend_rdata (struct cifs_readdata *rdata, +struct list_head *rdata_list, +struct cifs_aio_ctx *ctx) { +int wait_retry =0 ; +unsigned int rsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (rdata->cfile->tlink)->ses->server; + +/* +* Try to resend this rdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole rdata not in segments +*/ + do { + rc = server->ops->wait_mtu_credits(server, rdata->bytes, + &rsize, &credits); + + if (rc) + break; + + if (rsize < rdata->bytes) { + add_credits_and_wake_if(server, credits, 0); + msleep(1000); + wait_retry++;fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server, · line 3127
Quoted 10 lines
fs/cifs/file.c +10
-0+ } + }while (rsize < rdata->bytes && wait_retry <3 ); + +/* +* If we can't find enough credits to send this rdata +* release the rdata and return failure, this will pass +* whatever I/O amount we have finished to VFS. +*/ +if (rsize < rdata->bytes) { + rc = -EBUSY ;We don't have enough credits and return EBUSY here...
Quoted 26 lines
fs/cifs/file.c +23
-0+ goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!rdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (rdata->cfile, true))) + rc = server->ops->async_readv (rdata); + +if (!rc) { +/* Add to aio pending list */ +list_add_tail (&rdata->list, rdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, rdata->credits,0 ); +out: +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + +return rc; +} +static int cifs_send_async_read (loff_t offset,size_t len,struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, struct list_headQuoted 1 line
*rdata_list, @@ -3103,6 +3159,9 @@ cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo*open_file,fs/cifs/file.c cifs_send_async_read(loff_t offset, size_t len,
Quoted 10 lines
int rc; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +3
-0+struct page **pagevec; +size_t start; +struct iov_iter direct_iov = ctx->iter; server =tlink_tcon (open_file->tlink)->ses->server;@@ -3111,6 +3170,9 @@ cifs_send_async_read(loff_t offset, size_t len,struct cifsFileInfo
*open_file,fs/cifs/file.c cifs_send_async_read(loff_t offset, size_t len,
Quoted 10 lines
fs/cifs/file.c +3
-0else pid = current->tgid; +if (ctx->direct_io) +iov_iter_advance (&direct_iov, offset - ctx->pos); + do { rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize, &rsize, &credits);@@ -3118,20 +3180,56 @@ cifs_send_async_read(loff_t offset, size_t len,struct cifsFileInfo
*open_file,fs/cifs/file.c collect_uncached_read_data(struct
Quoted 90 lines
fs/cifs/file.c +47
-11break ; cur_len = min_t(const size_t, len, rsize); - npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); -/* allocate a readdata struct */ - rdata =cifs_readdata_alloc (npages, +if (ctx->direct_io) { + + cur_len =iov_iter_get_pages_alloc ( + &direct_iov, &pagevec, + cur_len, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"couldn't get user pages (cur_len=%zd)" +" iter type %d" +" iov_offset %zd count %zd\n" , + cur_len, direct_iov.type, direct_iov.iov_offset, + direct_iov.count); +dump_stack (); +break ; + } +iov_iter_advance (&direct_iov, cur_len); + + rdata =cifs_readdata_direct_alloc ( + pagevec, cifs_uncached_readv_complete); +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } + + npages = (cur_len + start +PAGE_SIZE -1 ) /PAGE_SIZE ; + rdata->page_offset = start; + rdata->tailsz = npages >1 ? + cur_len-(PAGE_SIZE -start)-(npages-2 )*PAGE_SIZE : + cur_len; + + }else { + + npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); +/* allocate a readdata struct */ + rdata =cifs_readdata_alloc (npages, cifs_uncached_readv_complete); -if (!rdata) { -add_credits_and_wake_if (server, credits,0 ); - rc = -ENOMEM ; -break ; - } +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } - rc =cifs_read_allocate_pages (rdata, npages); -if (rc) - goto error; + rc =cifs_read_allocate_pages (rdata, npages); +if (rc) + goto error; + + rdata->tailsz =PAGE_SIZE ; + } rdata->cfile =cifsFileInfo_get (open_file); rdata->nr_pages = npages; @@ -3139,7 +3237,6 @@cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo*open_file,rdata->bytes = cur_len; rdata->pid = pid; rdata->pagesz = PAGE_SIZE; - rdata->tailsz = PAGE_SIZE; rdata->read_into_pages = cifs_uncached_read_into_pages; rdata->copy_into_pages = cifs_uncached_copy_into_pages; rdata->credits = credits; @@ -3153,9 +3250,11 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file, if (rc) { add_credits_and_wake_if(server, rdata->credits, 0); kref_put(&rdata->refcount,fs/cifs/file.c +4
-2- cifs_uncached_readdata_release); -if (rc == -EAGAIN ) + cifs_uncached_readdata_release); +if (rc == -EAGAIN ) { +iov_iter_revert (&direct_iov, cur_len);continue ; + }break ; }@@ -3211,45 +3310,62 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)Quoted 16 lines
fs/cifs/file.c +6
-2* reading. */ if (got_bytes && got_bytes < rdata->bytes) { - rc =cifs_readdata_to_iov (rdata, to); + rc =0 ; +if (!ctx->direct_io) + rc = +cifs_readdata_to_iov (rdata, to);if (rc) {kref_put (&rdata->refcount, - cifs_uncached_readdata_release); + + cifs_uncached_readdata_release);continue ; } }fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx)
Quoted 10 lines
fs/cifs/file.c +8
-1- rc = cifs_send_async_read( +if (ctx->direct_io) { +/* +* Re-use rdata as this is a +* direct I/O +*/ + rc =cifs_resend_rdata ( + rdata, + &tmp_list, ctx);...so we set rc to
-EBUSY...fs/cifs/file.c collect_uncached_read_data(struct
Quoted 51 lines
fs/cifs/file.c +17
-9+ }else { + rc =cifs_send_async_read ( rdata->offset + got_bytes, rdata->bytes - got_bytes, rdata->cfile, cifs_sb, &tmp_list, ctx); +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + } +list_splice (&tmp_list, &ctx->list); -kref_put (&rdata->refcount, - cifs_uncached_readdata_release); goto again; }else if (rdata->result) rc = rdata->result; -else +else if (!ctx->direct_io) rc =cifs_readdata_to_iov (rdata, to);/* if there was a short read -- discard anything left */ if (rdata->got_bytes && rdata->got_bytes < rdata->bytes) rc = -ENODATA ; + + ctx->total_len += rdata->got_bytes; }list_del_init (&rdata->list);kref_put (&rdata->refcount, cifs_uncached_readdata_release); } -for (i =0 ; i < ctx->npages; i++) { -if (ctx->should_dirty) -set_page_dirty (ctx->bv[i].bv_page); -put_page (ctx->bv[i].bv_page); - } +if (!ctx->direct_io) { +for (i =0 ; i < ctx->npages; i++) { +if (ctx->should_dirty) +set_page_dirty (ctx->bv[i].bv_page); +put_page (ctx->bv[i].bv_page); + } - ctx->total_len = ctx->len -iov_iter_count (to); + ctx->total_len = ctx->len -iov_iter_count (to); + }cifs_stats_bytes_read (tcon, ctx->total_len);@@ -3267,18 +3383,27 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx) · line 3383
Quoted 1 line
fs/cifs/file.c +0
-0complete (&ctx->done);and then
ctx->rcis set to-EBUSYtoo.fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb,
Quoted 34 lines
fs/cifs/file.c +16
-6} -ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to) +ssize_t __cifs_readv (struct kiocb *iocb,struct iov_iter *to,bool +direct) { -struct file *file = iocb->ki_filp; -ssize_t rc;size_t len; -ssize_t total_read =0 ; -loff_t offset = iocb->ki_pos; +struct file *file = iocb->ki_filp;struct cifs_sb_info *cifs_sb; -struct cifs_tcon *tcon;struct cifsFileInfo *cfile; +struct cifs_tcon *tcon; +ssize_t rc, total_read =0 ; +loff_t offset = iocb->ki_pos;struct cifs_aio_ctx *ctx; +/* +* iov_iter_get_pages_alloc() doesn't work with ITER_KVEC, +* fall back to data copy read path +* this could be improved by getting pages directly in ITER_KVEC +*/ +if (direct && to->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_user_readv for kvec I/O\n" ); + direct = false; + } + len =iov_iter_count (to);if (!len)return 0 ;@@ -3305,14 +3430,20 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)Quoted 26 lines
fs/cifs/file.c +12
-6if (to->type ==ITER_IOVEC ) ctx->should_dirty = true; - rc =setup_aio_ctx_iter (ctx, to,READ ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->pos = offset; + ctx->direct_io = true; + ctx->iter = *to; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, to,READ ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } + len = ctx->len; } - len = ctx->len; -/* grab a lock here due to read response handlers can access ctx */ mutex_lock (&ctx->aio_mutex);
Quoted 8 lines
fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb,
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3354,6 +3485,16 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to) · line 3485
Quoted 1 line
fs/cifs/file.c +0
-0return rc;In case of a blocking read we will return
-EBUSYto the caller but read man page doesn't say anything about a possibility to return this error code. Is it being mapped somehow by VFS or is there another consideration? The same question applies to the write patch as well.
Thanks Pavel, Yes this is a bug.
In practice, the retry logic will rarely get hit. If getting hit, normally the server will give us far more credits for resending this whole packet.
How about just retrying forever (instead of trying 3 times) on this resend packet? This will change the code to the same behavior before direct I/O patches.
e.g.:
/*
* Try to resend this wdata, waiting for credits before sending
* Note: we are attempting to resend the whole wdata not in segments
*/
do {
rc = server->ops->wait_mtu_credits(
server, wdata->bytes, &wsize, &credits);
if (rc)
break;
if (wsize < wdata->bytes) {
add_credits_and_wake_if(server, credits, 0);
msleep(1000);
}
} while (wsize < wdata->bytes);Quoted 18 lines
Quoted 12 lines
fs/cifs/file.c +6
-1} +ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter *to) { +return __cifs_readv (iocb, to, true); } + +ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to) { +return __cifs_readv (iocb, to, false); } +ssize_t cifs_strict_readv (struct kiocb *iocb,struct iov_iter *to) { --Quoted 2 lines
2.7.4
--
Best regards,
Pavel Shilovsky
ср, 28 нояб. 2018 г. в 15:43, Long Li <[email protected]>:
Quoted 417 lines
Quoted 400 lines
Subject: Re: [Patch v4 1/3] CIFS: Add support for direct I/O read
Hi Long,
Please find my comments below.
ср, 31 окт. 2018 г. в 15:14, Long Li <[email protected]>:
Quoted 12 lines
From: Long Li <[email protected]>
With direct I/O read, we transfer the data directly from transport layer to the user data buffer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common read code to__cifs_readvfor direct and non-direct I/O. Retry on direct I/O failure.Quoted 5 lines
Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/cifsglob.h | 5 ++ fs/cifs/file.c | 219 +++++++++++++++++++++++++++++++++++++++++++----------
fs/cifs/file.c cifs_uncached_readdata_release(struct kref
Quoted 37 lines
3 files changed, 186 insertions(+), 39 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-05f02318..7fba9aa 100644
fs/cifs/cifsfs.h +0
-0@@ -102,6 +102,7 @@ extern int cifs_open(struct inode *inode, structfile *file); extern int
cifs_close(struct inode *inode, struct file *file); extern intcifs_closedir(struct inode *inode, struct file *file); externssize_tcifs_user_readv(struct kiocb *iocb, struct iov_iter *to);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct iov_iter +*to); extern ssize_t cifs_strict_readv(struct kiocb *iocb, struct iov_iter*to); extern
ssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from); externssize_tcifs_strict_writev(struct kiocb *iocb, struct iov_iter *from); diff --gita/fs/cifs/cifsglob.hb/fs/cifs/cifsglob.hindex 7f62c98..52248dd 100644fs/cifs/cifsglob.h +5
-0@@ -1146,6 +1146,11 @@ struct cifs_aio_ctx {unsigned int len;unsigned int total_len;bool should_dirty; +/* +* Indicates if this aio_ctx is for direct_io, +* If yes, iter is a copy of the user passed iov_iter +*/ +bool direct_io; };struct cifs_readdata ;fs/cifs/file.c index 87eece6..daab878 renamed from fs/cifs/file.c +0
-0100644
fs/cifs/file.c +0
-0@@ -2965,7 +2965,6 @@ cifs_uncached_readdata_release(struct kref*refcount)
Quoted 34 lines
fs/cifs/file.c +0
-1kref_put (&rdata->ctx->refcount, cifs_aio_ctx_release);for (i =0 ; i < rdata->nr_pages; i++) {put_page (rdata->pages[i]); - rdata->pages[i] =NULL ; } cifs_readdata_release(refcount); } @@ -3092,6 +3091,63 @@ cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,return uncached_fill_pages(server, rdata, iter, iter->count); }fs/cifs/file.c +24
-0+static int cifs_resend_rdata (struct cifs_readdata *rdata, +struct list_head *rdata_list, +struct cifs_aio_ctx *ctx) { +int wait_retry =0 ; +unsigned int rsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (rdata->cfile->tlink)->ses->server; + +/* +* Try to resend this rdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole rdata not in segments +*/ + do { + rc = server->ops->wait_mtu_credits(server, rdata->bytes, + &rsize, &credits); + + if (rc) + break; + + if (rsize < rdata->bytes) { + add_credits_and_wake_if(server, credits, 0); + msleep(1000); + wait_retry++;fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server, · line 3127
Quoted 10 lines
fs/cifs/file.c +10
-0+ } + }while (rsize < rdata->bytes && wait_retry <3 ); + +/* +* If we can't find enough credits to send this rdata +* release the rdata and return failure, this will pass +* whatever I/O amount we have finished to VFS. +*/ +if (rsize < rdata->bytes) { + rc = -EBUSY ;We don't have enough credits and return EBUSY here...
Quoted 26 lines
fs/cifs/file.c +23
-0+ goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!rdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (rdata->cfile, true))) + rc = server->ops->async_readv (rdata); + +if (!rc) { +/* Add to aio pending list */ +list_add_tail (&rdata->list, rdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, rdata->credits,0 ); +out: +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + +return rc; +} +static int cifs_send_async_read (loff_t offset,size_t len,struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, struct list_headQuoted 1 line
*rdata_list, @@ -3103,6 +3159,9 @@ cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo*open_file,fs/cifs/file.c cifs_send_async_read(loff_t offset, size_t len,
Quoted 10 lines
int rc; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +3
-0+struct page **pagevec; +size_t start; +struct iov_iter direct_iov = ctx->iter; server =tlink_tcon (open_file->tlink)->ses->server;@@ -3111,6 +3170,9 @@ cifs_send_async_read(loff_t offset, size_t len,struct cifsFileInfo
*open_file,fs/cifs/file.c cifs_send_async_read(loff_t offset, size_t len,
Quoted 10 lines
fs/cifs/file.c +3
-0else pid = current->tgid; +if (ctx->direct_io) +iov_iter_advance (&direct_iov, offset - ctx->pos); + do { rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize, &rsize, &credits);@@ -3118,20 +3180,56 @@ cifs_send_async_read(loff_t offset, size_t len,struct cifsFileInfo
*open_file,fs/cifs/file.c collect_uncached_read_data(struct
Quoted 90 lines
fs/cifs/file.c +47
-11break ; cur_len = min_t(const size_t, len, rsize); - npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); -/* allocate a readdata struct */ - rdata =cifs_readdata_alloc (npages, +if (ctx->direct_io) { + + cur_len =iov_iter_get_pages_alloc ( + &direct_iov, &pagevec, + cur_len, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"couldn't get user pages (cur_len=%zd)" +" iter type %d" +" iov_offset %zd count %zd\n" , + cur_len, direct_iov.type, direct_iov.iov_offset, + direct_iov.count); +dump_stack (); +break ; + } +iov_iter_advance (&direct_iov, cur_len); + + rdata =cifs_readdata_direct_alloc ( + pagevec, cifs_uncached_readv_complete); +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } + + npages = (cur_len + start +PAGE_SIZE -1 ) /PAGE_SIZE ; + rdata->page_offset = start; + rdata->tailsz = npages >1 ? + cur_len-(PAGE_SIZE -start)-(npages-2 )*PAGE_SIZE : + cur_len; + + }else { + + npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); +/* allocate a readdata struct */ + rdata =cifs_readdata_alloc (npages, cifs_uncached_readv_complete); -if (!rdata) { -add_credits_and_wake_if (server, credits,0 ); - rc = -ENOMEM ; -break ; - } +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } - rc =cifs_read_allocate_pages (rdata, npages); -if (rc) - goto error; + rc =cifs_read_allocate_pages (rdata, npages); +if (rc) + goto error; + + rdata->tailsz =PAGE_SIZE ; + } rdata->cfile =cifsFileInfo_get (open_file); rdata->nr_pages = npages; @@ -3139,7 +3237,6 @@cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo*open_file,rdata->bytes = cur_len; rdata->pid = pid; rdata->pagesz = PAGE_SIZE; - rdata->tailsz = PAGE_SIZE; rdata->read_into_pages = cifs_uncached_read_into_pages; rdata->copy_into_pages = cifs_uncached_copy_into_pages; rdata->credits = credits; @@ -3153,9 +3250,11 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file, if (rc) { add_credits_and_wake_if(server, rdata->credits, 0); kref_put(&rdata->refcount,fs/cifs/file.c +4
-2- cifs_uncached_readdata_release); -if (rc == -EAGAIN ) + cifs_uncached_readdata_release); +if (rc == -EAGAIN ) { +iov_iter_revert (&direct_iov, cur_len);continue ; + }break ; }@@ -3211,45 +3310,62 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)Quoted 16 lines
fs/cifs/file.c +6
-2* reading. */ if (got_bytes && got_bytes < rdata->bytes) { - rc =cifs_readdata_to_iov (rdata, to); + rc =0 ; +if (!ctx->direct_io) + rc = +cifs_readdata_to_iov (rdata, to);if (rc) {kref_put (&rdata->refcount, - cifs_uncached_readdata_release); + + cifs_uncached_readdata_release);continue ; } }fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx)
Quoted 10 lines
fs/cifs/file.c +8
-1- rc = cifs_send_async_read( +if (ctx->direct_io) { +/* +* Re-use rdata as this is a +* direct I/O +*/ + rc =cifs_resend_rdata ( + rdata, + &tmp_list, ctx);...so we set rc to
-EBUSY...fs/cifs/file.c collect_uncached_read_data(struct
Quoted 51 lines
fs/cifs/file.c +17
-9+ }else { + rc =cifs_send_async_read ( rdata->offset + got_bytes, rdata->bytes - got_bytes, rdata->cfile, cifs_sb, &tmp_list, ctx); +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + } +list_splice (&tmp_list, &ctx->list); -kref_put (&rdata->refcount, - cifs_uncached_readdata_release); goto again; }else if (rdata->result) rc = rdata->result; -else +else if (!ctx->direct_io) rc =cifs_readdata_to_iov (rdata, to);/* if there was a short read -- discard anything left */ if (rdata->got_bytes && rdata->got_bytes < rdata->bytes) rc = -ENODATA ; + + ctx->total_len += rdata->got_bytes; }list_del_init (&rdata->list);kref_put (&rdata->refcount, cifs_uncached_readdata_release); } -for (i =0 ; i < ctx->npages; i++) { -if (ctx->should_dirty) -set_page_dirty (ctx->bv[i].bv_page); -put_page (ctx->bv[i].bv_page); - } +if (!ctx->direct_io) { +for (i =0 ; i < ctx->npages; i++) { +if (ctx->should_dirty) +set_page_dirty (ctx->bv[i].bv_page); +put_page (ctx->bv[i].bv_page); + } - ctx->total_len = ctx->len -iov_iter_count (to); + ctx->total_len = ctx->len -iov_iter_count (to); + }cifs_stats_bytes_read (tcon, ctx->total_len);@@ -3267,18 +3383,27 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx) · line 3383
Quoted 1 line
fs/cifs/file.c +0
-0complete (&ctx->done);and then
ctx->rcis set to-EBUSYtoo.fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb,
Quoted 34 lines
fs/cifs/file.c +16
-6} -ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to) +ssize_t __cifs_readv (struct kiocb *iocb,struct iov_iter *to,bool +direct) { -struct file *file = iocb->ki_filp; -ssize_t rc;size_t len; -ssize_t total_read =0 ; -loff_t offset = iocb->ki_pos; +struct file *file = iocb->ki_filp;struct cifs_sb_info *cifs_sb; -struct cifs_tcon *tcon;struct cifsFileInfo *cfile; +struct cifs_tcon *tcon; +ssize_t rc, total_read =0 ; +loff_t offset = iocb->ki_pos;struct cifs_aio_ctx *ctx; +/* +* iov_iter_get_pages_alloc() doesn't work with ITER_KVEC, +* fall back to data copy read path +* this could be improved by getting pages directly in ITER_KVEC +*/ +if (direct && to->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_user_readv for kvec I/O\n" ); + direct = false; + } + len =iov_iter_count (to);if (!len)return 0 ;@@ -3305,14 +3430,20 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)Quoted 26 lines
fs/cifs/file.c +12
-6if (to->type ==ITER_IOVEC ) ctx->should_dirty = true; - rc =setup_aio_ctx_iter (ctx, to,READ ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->pos = offset; + ctx->direct_io = true; + ctx->iter = *to; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, to,READ ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } + len = ctx->len; } - len = ctx->len; -/* grab a lock here due to read response handlers can access ctx */ mutex_lock (&ctx->aio_mutex);Quoted 8 lines
fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb,
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3354,6 +3485,16 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to) · line 3485
Quoted 1 line
fs/cifs/file.c +0
-0return rc;In case of a blocking read we will return
-EBUSYto the caller but read man page doesn't say anything about a possibility to return this error code. Is it being mapped somehow by VFS or is there another consideration? The same question applies to the write patch as well.Thanks Pavel, Yes this is a bug.
In practice, the retry logic will rarely get hit. If getting hit, normally the server will give us far more credits for resending this whole packet.
How about just retrying forever (instead of trying 3 times) on this resend packet? This will change the code to the same behavior before direct I/O patches.
e.g.:
Quoted 17 lines
/*
* Try to resend this wdata, waiting for credits before sending
* Note: we are attempting to resend the whole wdata not in segments
*/
do {
rc = server->ops->wait_mtu_credits(
server, wdata->bytes, &wsize, &credits);
if (rc)
break;
if (wsize < wdata->bytes) {
add_credits_and_wake_if(server, credits, 0);
msleep(1000);
}
} while (wsize < wdata->bytes);We can do that but it won't be the same behavior because we were using a partial send:
+ if (ctx->direct_io) {
+ /*
+ * Re-use rdata as this is a + * direct I/O + */
+ rc = cifs_resend_rdata(
+ rdata,
+ &tmp_list, ctx);
+ } else {
+ rc = cifs_send_async_read(
rdata->offset + got_bytes,
rdata->bytes - got_bytes,
rdata->cfile, cifs_sb,
&tmp_list, ctx);--
Best regards,
Pavel Shilovsky
Quoted 459 lines
Subject: Re: [Patch v4 1/3] CIFS: Add support for direct I/O read
ср, 28 нояб. 2018 г. в 15:43, Long Li <[email protected]>:
Quoted 12 lines
Quoted 12 lines
Subject: Re: [Patch v4 1/3] CIFS: Add support for direct I/O read
Hi Long,
Please find my comments below.
ср, 31 окт. 2018 г. в 15:14, Long Li <[email protected]>:
Quoted 3 lines
From: Long Li <[email protected]>
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
With direct I/O read, we transfer the data directly from transport layer to the user data buffer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common read code to__cifs_readvfor direct and non-directI/O.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Retry on direct I/O failure.
Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/cifsglob.h | 5 ++ fs/cifs/file.c | 219+++++++++++++++++++++++++++++++++++++++++++--
Quoted 51 lines
Quoted 51 lines
--------
fs/cifs/file.c cifs_uncached_readdata_release(struct kref
Quoted 39 lines
3 files changed, 186 insertions(+), 39 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-05f02318..7fba9aa 100644
fs/cifs/cifsfs.h +0
-0@@ -102,6 +102,7 @@ extern int cifs_open(struct inode *inode, struct file *file); extern int cifs_close(struct inode *inode, struct file *file); extern int cifs_closedir(struct inode *inode,struct file *file); extern
ssize_tcifs_user_readv(struct kiocb *iocb, struct iov_iter *to);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct +iov_iter *to); extern ssize_t cifs_strict_readv(struct kiocb *iocb, struct
iov_iter*to); externssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from); externssize_tcifs_strict_writev(struct kiocb *iocb, struct iov_iter *from);fs/cifs/cifsglob.h index renamed from fs/cifs/cifsglob.h +0
-07f62c98..52248dd 100644
fs/cifs/cifsglob.h +5
-0@@ -1146,6 +1146,11 @@ struct cifs_aio_ctx {unsigned int len;unsigned int total_len;bool should_dirty; +/* +* Indicates if this aio_ctx is for direct_io, +* If yes, iter is a copy of the user passed iov_iter +*/ +bool direct_io; };struct cifs_readdata ;fs/cifs/file.c index renamed from fs/cifs/file.c +0
-087eece6..daab878
100644fs/cifs/file.c +0
-0@@ -2965,7 +2965,6 @@ cifs_uncached_readdata_release(struct kref*refcount)
Quoted 10 lines
fs/cifs/file.c +0
-1kref_put (&rdata->ctx->refcount, cifs_aio_ctx_release);for (i =0 ; i < rdata->nr_pages; i++) {put_page (rdata->pages[i]); - rdata->pages[i] =NULL ; } cifs_readdata_release(refcount); } @@ -3092,6 +3091,63 @@ cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,return uncached_fill_pages(server, rdata, iter,
iter->count); }Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
fs/cifs/file.c +4
-0+static int cifs_resend_rdata (struct cifs_readdata *rdata, +struct list_head *rdata_list, +struct cifs_aio_ctx *ctx) { +int wait_retry =0 ;Quoted 8 lines
fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,
Quoted 8 lines
fs/cifs/file.c +8
-0+unsigned int rsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (rdata->cfile->tlink)->ses->server; + +/* +* Try to resend this rdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole rdata not in segments
Quoted 42 lines
Quoted 42 lines
Quoted 12 lines
fs/cifs/file.c +12
-0+*/ + do { + rc = server->ops->wait_mtu_credits(server, rdata->bytes, + &rsize, &credits); + + if (rc) + break; + + if (rsize < rdata->bytes) { + add_credits_and_wake_if(server, credits, 0); + msleep(1000); + wait_retry++;fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server, · line 3127
Quoted 10 lines
fs/cifs/file.c +10
-0+ } + }while (rsize < rdata->bytes && wait_retry <3 ); + +/* +* If we can't find enough credits to send this rdata +* release the rdata and return failure, this will pass +* whatever I/O amount we have finished to VFS. +*/ +if (rsize < rdata->bytes) { + rc = -EBUSY ;We don't have enough credits and return EBUSY here...
Quoted 17 lines
fs/cifs/file.c +17
-0+ goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!rdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (rdata->cfile, true))) + rc = server->ops->async_readv (rdata); + +if (!rc) { +/* Add to aio pending list */ +list_add_tail (&rdata->list, rdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, rdata->credits,0 ); +out:Quoted 8 lines
Quoted 8 lines
fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,
Quoted 8 lines
fs/cifs/file.c +6
-0+kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + +return rc; +} +static int cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo
*open_file,Quoted 93 lines
Quoted 93 lines
Quoted 3 lines
struct
cifs_sb_info*cifs_sb, structlist_head*rdata_list, @@ -3103,6 +3159,9 @@ cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo*open_file,fs/cifs/file.c cifs_send_async_read(loff_t offset, size_t · line 3170
Quoted 11 lines
int rc; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +3
-0+struct page **pagevec; +size_t start; +struct iov_iter direct_iov = ctx->iter; server =tlink_tcon (open_file->tlink)->ses->server;@@ -3111,6 +3170,9 @@ cifs_send_async_read(loff_t offset, size_t len,struct cifsFileInfo
*open_file,Quoted 11 lines
fs/cifs/file.c +3
-0else pid = current->tgid; +if (ctx->direct_io) +iov_iter_advance (&direct_iov, offset - ctx->pos); + do { rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize,&rsize,&credits); @@ -3118,20 +3180,56 @@ cifs_send_async_read(loff_t offset,
size_tlen,struct cifsFileInfo
*open_file,Quoted 65 lines
break; cur_len = min_t(const size_t, len, rsize);fs/cifs/file.c +47
-11- npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); -/* allocate a readdata struct */ - rdata =cifs_readdata_alloc (npages, +if (ctx->direct_io) { + + cur_len =iov_iter_get_pages_alloc ( + &direct_iov, &pagevec, + cur_len, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"couldn't get user pages (cur_len=%zd)" +" iter type %d" +" iov_offset %zd count %zd\n" , + cur_len, direct_iov.type, direct_iov.iov_offset, + direct_iov.count); +dump_stack (); +break ; + } +iov_iter_advance (&direct_iov, cur_len); + + rdata =cifs_readdata_direct_alloc ( + pagevec, cifs_uncached_readv_complete); +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } + + npages = (cur_len + start +PAGE_SIZE -1 ) /PAGE_SIZE ; + rdata->page_offset = start; + rdata->tailsz = npages >1 ? + cur_len-(PAGE_SIZE -start)-(npages-2 )*PAGE_SIZE : + cur_len; + + }else { + + npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); +/* allocate a readdata struct */ + rdata =cifs_readdata_alloc (npages, cifs_uncached_readv_complete); -if (!rdata) { -add_credits_and_wake_if (server, credits,0 ); - rc = -ENOMEM ; -break ; - } +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } - rc =cifs_read_allocate_pages (rdata, npages); -if (rc) - goto error; + rc =cifs_read_allocate_pages (rdata, npages); +if (rc) + goto error; + + rdata->tailsz =PAGE_SIZE ; + }Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
rdata->cfile = cifsFileInfo_get(open_file); rdata->nr_pages = npages; @@ -3139,7 +3237,6 @@cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo
*open_file,Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
rdata->bytes = cur_len; rdata->pid = pid; rdata->pagesz = PAGE_SIZE; - rdata->tailsz = PAGE_SIZE; rdata->read_into_pages = cifs_uncached_read_into_pages; rdata->copy_into_pages = cifs_uncached_copy_into_pages; rdata->credits = credits; @@ -3153,9 +3250,11 @@cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo
*open_file,Quoted 167 lines
Quoted 167 lines
fs/cifs/file.c collect_uncached_read_data(struct
Quoted 15 lines
if (rc) { add_credits_and_wake_if(server, rdata->credits, 0); kref_put(&rdata->refcount,fs/cifs/file.c +5
-2- cifs_uncached_readdata_release); -if (rc == -EAGAIN ) + cifs_uncached_readdata_release); +if (rc == -EAGAIN ) { +iov_iter_revert (&direct_iov, + cur_len);continue ; + }break ; }@@ -3211,45 +3310,62 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)Quoted 16 lines
fs/cifs/file.c +6
-2* reading. */ if (got_bytes && got_bytes < rdata->bytes) { - rc =cifs_readdata_to_iov (rdata, to); + rc =0 ; +if (!ctx->direct_io) + rc = +cifs_readdata_to_iov (rdata, to);if (rc) {kref_put (&rdata->refcount, - cifs_uncached_readdata_release); + + cifs_uncached_readdata_release);continue ; } }fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx)
Quoted 10 lines
fs/cifs/file.c +8
-1- rc = cifs_send_async_read( +if (ctx->direct_io) { +/* +* Re-use rdata as this is a +* direct I/O +*/ + rc =cifs_resend_rdata ( + rdata, + &tmp_list, ctx);...so we set rc to
-EBUSY...fs/cifs/file.c collect_uncached_read_data(struct
Quoted 53 lines
fs/cifs/file.c +17
-9+ }else { + rc =cifs_send_async_read ( rdata->offset + got_bytes, rdata->bytes - got_bytes, rdata->cfile, cifs_sb, &tmp_list, ctx); +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + } +list_splice (&tmp_list, &ctx->list); -kref_put (&rdata->refcount, - cifs_uncached_readdata_release); goto again; }else if (rdata->result) rc = rdata->result; -else +else if (!ctx->direct_io) rc =cifs_readdata_to_iov (rdata, to);/* if there was a short read -- discard anything left */ if (rdata->got_bytes && rdata->got_bytes < rdata->bytes) rc = -ENODATA ; + + ctx->total_len += rdata->got_bytes; }list_del_init (&rdata->list);kref_put (&rdata->refcount, cifs_uncached_readdata_release); } -for (i =0 ; i < ctx->npages; i++) { -if (ctx->should_dirty) -set_page_dirty (ctx->bv[i].bv_page); -put_page (ctx->bv[i].bv_page); - } +if (!ctx->direct_io) { +for (i =0 ; i < ctx->npages; i++) { +if (ctx->should_dirty) +set_page_dirty (ctx->bv[i].bv_page); +put_page (ctx->bv[i].bv_page); + } - ctx->total_len = ctx->len -iov_iter_count (to); + ctx->total_len = ctx->len -iov_iter_count (to); + }cifs_stats_bytes_read (tcon, ctx->total_len);@@ -3267,18 +3383,27 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx) · line 3383
Quoted 1 line
fs/cifs/file.c +0
-0complete (&ctx->done);and then
ctx->rcis set to-EBUSYtoo.Quoted 35 lines
fs/cifs/file.c +17
-6} -ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to) +ssize_t __cifs_readv (struct kiocb *iocb,struct iov_iter *to, +bool +direct) { -struct file *file = iocb->ki_filp; -ssize_t rc;size_t len; -ssize_t total_read =0 ; -loff_t offset = iocb->ki_pos; +struct file *file = iocb->ki_filp;struct cifs_sb_info *cifs_sb; -struct cifs_tcon *tcon;struct cifsFileInfo *cfile; +struct cifs_tcon *tcon; +ssize_t rc, total_read =0 ; +loff_t offset = iocb->ki_pos;struct cifs_aio_ctx *ctx; +/* +* iov_iter_get_pages_alloc() doesn't work with ITER_KVEC, +* fall back to data copy read path +* this could be improved by getting pages directly in ITER_KVEC +*/ +if (direct && to->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_user_readv for kvec I/O\n" ); + direct = false; + } + len =iov_iter_count (to);if (!len)return 0 ;fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb · line 3430
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3305,14 +3430,20 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)Quoted 26 lines
fs/cifs/file.c +12
-6if (to->type ==ITER_IOVEC ) ctx->should_dirty = true; - rc =setup_aio_ctx_iter (ctx, to,READ ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->pos = offset; + ctx->direct_io = true; + ctx->iter = *to; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, to,READ ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } + len = ctx->len; } - len = ctx->len; -/* grab a lock here due to read response handlers can access ctx */ mutex_lock (&ctx->aio_mutex);Quoted 8 lines
Quoted 8 lines
fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb,
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3354,6 +3485,16 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to) · line 3485
Quoted 1 line
fs/cifs/file.c +0
-0return rc;In case of a blocking read we will return
-EBUSYto the caller but read man page doesn't say anything about a possibility to return this error code. Is it being mapped somehow by VFS or is there another consideration? The same question applies to the write patch aswell.
Quoted 4 lines
Thanks Pavel, Yes this is a bug.
In practice, the retry logic will rarely get hit. If getting hit, normally the
server will give us far more credits for resending this whole packet.
Quoted 2 lines
How about just retrying forever (instead of trying 3 times) on this resend
packet? This will change the code to the same behavior before direct I/O patches.
Quoted 6 lines
e.g.:
/*
* Try to resend this wdata, waiting for credits before sending
* Note: we are attempting to resend the whole wdata not insegments
Quoted 5 lines
*/ do { rc = server->ops->wait_mtu_credits( server, wdata->bytes, &wsize, &credits);Quoted 9 lines
if (rc) break; if (wsize < wdata->bytes) { add_credits_and_wake_if(server, credits, 0); msleep(1000); } } while (wsize < wdata->bytes);
Quoted 2 lines
We can do that but it won't be the same behavior because we were using a partial send:
Yes it's a little different.
I don't see any ill effects of doing this, other than we may wait for a little longer on a full credit to send the whole packet. In practice the server always offers a full credit on the first call to wait_mtu_credits(), I am yet to see the while loop actually get looped.
The resend path is rarely getting hit in stress tests on TCP, I estimate maybe less than 0.001% on my test setup, the while loop for waiting credits is ever harder to hit that I never see one.
On RDMA, the resend path is never used.
Quoted 19 lines
fs/cifs/file.c +10
-1+if (ctx->direct_io) { +/* +* Re-use rdata as this is a +* direct I/O +*/ + rc =cifs_resend_rdata ( + rdata, + &tmp_list, ctx); + }else { + rc =cifs_send_async_read ( rdata->offset + got_bytes, rdata->bytes - got_bytes, rdata->cfile, cifs_sb, &tmp_list, ctx); --Best regards,
Pavel Shilovsky
чт, 29 нояб. 2018 г. в 14:48, Long Li <[email protected]>:
Quoted 463 lines
Quoted 460 lines
Subject: Re: [Patch v4 1/3] CIFS: Add support for direct I/O read
ср, 28 нояб. 2018 г. в 15:43, Long Li <[email protected]>:
Quoted 12 lines
Quoted 12 lines
Subject: Re: [Patch v4 1/3] CIFS: Add support for direct I/O read
Hi Long,
Please find my comments below.
ср, 31 окт. 2018 г. в 15:14, Long Li <[email protected]>:
Quoted 3 lines
From: Long Li <[email protected]>
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
With direct I/O read, we transfer the data directly from transport layer to the user data buffer.
Change in v3: add support for kernel AIO
Change in v4:
Refactor common read code to__cifs_readvfor direct and non-directI/O.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Retry on direct I/O failure.
Signed-off-by Long Li <[email protected]>--- fs/cifs/cifsfs.h | 1 + fs/cifs/cifsglob.h | 5 ++ fs/cifs/file.c | 219+++++++++++++++++++++++++++++++++++++++++++--
Quoted 51 lines
Quoted 51 lines
--------
fs/cifs/file.c cifs_uncached_readdata_release(struct kref
Quoted 39 lines
3 files changed, 186 insertions(+), 39 deletions(-)fs/cifs/cifsfs.h index renamed from fs/cifs/cifsfs.h +0
-05f02318..7fba9aa 100644
fs/cifs/cifsfs.h +0
-0@@ -102,6 +102,7 @@ extern int cifs_open(struct inode *inode, struct file *file); extern int cifs_close(struct inode *inode, struct file *file); extern int cifs_closedir(struct inode *inode,struct file *file); extern
ssize_tcifs_user_readv(struct kiocb *iocb, struct iov_iter *to);fs/cifs/cifsfs.h +2
-0+extern ssize_t cifs_direct_readv (struct kiocb *iocb,struct +iov_iter *to); extern ssize_t cifs_strict_readv(struct kiocb *iocb, struct
iov_iter*to); externssize_tcifs_user_writev(struct kiocb *iocb, struct iov_iter *from); externssize_tcifs_strict_writev(struct kiocb *iocb, struct iov_iter *from);fs/cifs/cifsglob.h index renamed from fs/cifs/cifsglob.h +0
-07f62c98..52248dd 100644
fs/cifs/cifsglob.h +5
-0@@ -1146,6 +1146,11 @@ struct cifs_aio_ctx {unsigned int len;unsigned int total_len;bool should_dirty; +/* +* Indicates if this aio_ctx is for direct_io, +* If yes, iter is a copy of the user passed iov_iter +*/ +bool direct_io; };struct cifs_readdata ;fs/cifs/file.c index renamed from fs/cifs/file.c +0
-087eece6..daab878
100644fs/cifs/file.c +0
-0@@ -2965,7 +2965,6 @@ cifs_uncached_readdata_release(struct kref*refcount)
Quoted 10 lines
fs/cifs/file.c +0
-1kref_put (&rdata->ctx->refcount, cifs_aio_ctx_release);for (i =0 ; i < rdata->nr_pages; i++) {put_page (rdata->pages[i]); - rdata->pages[i] =NULL ; } cifs_readdata_release(refcount); } @@ -3092,6 +3091,63 @@ cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,return uncached_fill_pages(server, rdata, iter,
iter->count); }Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
fs/cifs/file.c +4
-0+static int cifs_resend_rdata (struct cifs_readdata *rdata, +struct list_head *rdata_list, +struct cifs_aio_ctx *ctx) { +int wait_retry =0 ;Quoted 8 lines
fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,
Quoted 8 lines
fs/cifs/file.c +8
-0+unsigned int rsize, credits; +int rc; +struct TCP_Server_Info *server = +tlink_tcon (rdata->cfile->tlink)->ses->server; + +/* +* Try to resend this rdata, waiting for credits up to 3 seconds. +* Note: we are attempting to resend the whole rdata not in segments
Quoted 42 lines
Quoted 42 lines
Quoted 12 lines
fs/cifs/file.c +12
-0+*/ + do { + rc = server->ops->wait_mtu_credits(server, rdata->bytes, + &rsize, &credits); + + if (rc) + break; + + if (rsize < rdata->bytes) { + add_credits_and_wake_if(server, credits, 0); + msleep(1000); + wait_retry++;fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server, · line 3127
Quoted 10 lines
fs/cifs/file.c +10
-0+ } + }while (rsize < rdata->bytes && wait_retry <3 ); + +/* +* If we can't find enough credits to send this rdata +* release the rdata and return failure, this will pass +* whatever I/O amount we have finished to VFS. +*/ +if (rsize < rdata->bytes) { + rc = -EBUSY ;We don't have enough credits and return EBUSY here...
Quoted 17 lines
fs/cifs/file.c +17
-0+ goto out; + } + + rc = -EAGAIN ; +while (rc == -EAGAIN ) +if (!rdata->cfile->invalidHandle || + !(rc =cifs_reopen_file (rdata->cfile, true))) + rc = server->ops->async_readv (rdata); + +if (!rc) { +/* Add to aio pending list */ +list_add_tail (&rdata->list, rdata_list); +return 0 ; + } + +add_credits_and_wake_if (server, rdata->credits,0 ); +out:Quoted 8 lines
Quoted 8 lines
fs/cifs/file.c cifs_uncached_copy_into_pages(struct TCP_Server_Info *server,
Quoted 8 lines
fs/cifs/file.c +6
-0+kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + +return rc; +} +static int cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo
*open_file,Quoted 93 lines
Quoted 93 lines
Quoted 3 lines
struct
cifs_sb_info*cifs_sb, structlist_head*rdata_list, @@ -3103,6 +3159,9 @@ cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo*open_file,fs/cifs/file.c cifs_send_async_read(loff_t offset, size_t · line 3170
Quoted 11 lines
int rc; pid_t pid; struct TCP_Server_Info *server;fs/cifs/file.c +3
-0+struct page **pagevec; +size_t start; +struct iov_iter direct_iov = ctx->iter; server =tlink_tcon (open_file->tlink)->ses->server;@@ -3111,6 +3170,9 @@ cifs_send_async_read(loff_t offset, size_t len,struct cifsFileInfo
*open_file,Quoted 11 lines
fs/cifs/file.c +3
-0else pid = current->tgid; +if (ctx->direct_io) +iov_iter_advance (&direct_iov, offset - ctx->pos); + do { rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize,&rsize,&credits); @@ -3118,20 +3180,56 @@ cifs_send_async_read(loff_t offset,
size_tlen,struct cifsFileInfo
*open_file,Quoted 65 lines
break; cur_len = min_t(const size_t, len, rsize);fs/cifs/file.c +47
-11- npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); -/* allocate a readdata struct */ - rdata =cifs_readdata_alloc (npages, +if (ctx->direct_io) { + + cur_len =iov_iter_get_pages_alloc ( + &direct_iov, &pagevec, + cur_len, &start); +if (cur_len <0 ) { +cifs_dbg (VFS , +"couldn't get user pages (cur_len=%zd)" +" iter type %d" +" iov_offset %zd count %zd\n" , + cur_len, direct_iov.type, direct_iov.iov_offset, + direct_iov.count); +dump_stack (); +break ; + } +iov_iter_advance (&direct_iov, cur_len); + + rdata =cifs_readdata_direct_alloc ( + pagevec, cifs_uncached_readv_complete); +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } + + npages = (cur_len + start +PAGE_SIZE -1 ) /PAGE_SIZE ; + rdata->page_offset = start; + rdata->tailsz = npages >1 ? + cur_len-(PAGE_SIZE -start)-(npages-2 )*PAGE_SIZE : + cur_len; + + }else { + + npages =DIV_ROUND_UP (cur_len,PAGE_SIZE ); +/* allocate a readdata struct */ + rdata =cifs_readdata_alloc (npages, cifs_uncached_readv_complete); -if (!rdata) { -add_credits_and_wake_if (server, credits,0 ); - rc = -ENOMEM ; -break ; - } +if (!rdata) { +add_credits_and_wake_if (server, credits,0 ); + rc = -ENOMEM ; +break ; + } - rc =cifs_read_allocate_pages (rdata, npages); -if (rc) - goto error; + rc =cifs_read_allocate_pages (rdata, npages); +if (rc) + goto error; + + rdata->tailsz =PAGE_SIZE ; + }Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
rdata->cfile = cifsFileInfo_get(open_file); rdata->nr_pages = npages; @@ -3139,7 +3237,6 @@cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo
*open_file,Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
rdata->bytes = cur_len; rdata->pid = pid; rdata->pagesz = PAGE_SIZE; - rdata->tailsz = PAGE_SIZE; rdata->read_into_pages = cifs_uncached_read_into_pages; rdata->copy_into_pages = cifs_uncached_copy_into_pages; rdata->credits = credits; @@ -3153,9 +3250,11 @@cifs_send_async_read(loff_t offset,
size_tlen, struct cifsFileInfo
*open_file,Quoted 167 lines
Quoted 167 lines
fs/cifs/file.c collect_uncached_read_data(struct
Quoted 15 lines
if (rc) { add_credits_and_wake_if(server, rdata->credits, 0); kref_put(&rdata->refcount,fs/cifs/file.c +5
-2- cifs_uncached_readdata_release); -if (rc == -EAGAIN ) + cifs_uncached_readdata_release); +if (rc == -EAGAIN ) { +iov_iter_revert (&direct_iov, + cur_len);continue ; + }break ; }@@ -3211,45 +3310,62 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)Quoted 16 lines
fs/cifs/file.c +6
-2* reading. */ if (got_bytes && got_bytes < rdata->bytes) { - rc =cifs_readdata_to_iov (rdata, to); + rc =0 ; +if (!ctx->direct_io) + rc = +cifs_readdata_to_iov (rdata, to);if (rc) {kref_put (&rdata->refcount, - cifs_uncached_readdata_release); + + cifs_uncached_readdata_release);continue ; } }fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx)
Quoted 10 lines
fs/cifs/file.c +8
-1- rc = cifs_send_async_read( +if (ctx->direct_io) { +/* +* Re-use rdata as this is a +* direct I/O +*/ + rc =cifs_resend_rdata ( + rdata, + &tmp_list, ctx);...so we set rc to
-EBUSY...fs/cifs/file.c collect_uncached_read_data(struct
Quoted 53 lines
fs/cifs/file.c +17
-9+ }else { + rc =cifs_send_async_read ( rdata->offset + got_bytes, rdata->bytes - got_bytes, rdata->cfile, cifs_sb, &tmp_list, ctx); +kref_put (&rdata->refcount, + cifs_uncached_readdata_release); + } +list_splice (&tmp_list, &ctx->list); -kref_put (&rdata->refcount, - cifs_uncached_readdata_release); goto again; }else if (rdata->result) rc = rdata->result; -else +else if (!ctx->direct_io) rc =cifs_readdata_to_iov (rdata, to);/* if there was a short read -- discard anything left */ if (rdata->got_bytes && rdata->got_bytes < rdata->bytes) rc = -ENODATA ; + + ctx->total_len += rdata->got_bytes; }list_del_init (&rdata->list);kref_put (&rdata->refcount, cifs_uncached_readdata_release); } -for (i =0 ; i < ctx->npages; i++) { -if (ctx->should_dirty) -set_page_dirty (ctx->bv[i].bv_page); -put_page (ctx->bv[i].bv_page); - } +if (!ctx->direct_io) { +for (i =0 ; i < ctx->npages; i++) { +if (ctx->should_dirty) +set_page_dirty (ctx->bv[i].bv_page); +put_page (ctx->bv[i].bv_page); + } - ctx->total_len = ctx->len -iov_iter_count (to); + ctx->total_len = ctx->len -iov_iter_count (to); + }cifs_stats_bytes_read (tcon, ctx->total_len);@@ -3267,18 +3383,27 @@ collect_uncached_read_data(struct
cifs_aio_ctx*ctx)fs/cifs/file.c collect_uncached_read_data(struct cifs_aio_ctx *ctx) · line 3383
Quoted 1 line
fs/cifs/file.c +0
-0complete (&ctx->done);and then
ctx->rcis set to-EBUSYtoo.Quoted 35 lines
fs/cifs/file.c +17
-6} -ssize_t cifs_user_readv (struct kiocb *iocb,struct iov_iter *to) +ssize_t __cifs_readv (struct kiocb *iocb,struct iov_iter *to, +bool +direct) { -struct file *file = iocb->ki_filp; -ssize_t rc;size_t len; -ssize_t total_read =0 ; -loff_t offset = iocb->ki_pos; +struct file *file = iocb->ki_filp;struct cifs_sb_info *cifs_sb; -struct cifs_tcon *tcon;struct cifsFileInfo *cfile; +struct cifs_tcon *tcon; +ssize_t rc, total_read =0 ; +loff_t offset = iocb->ki_pos;struct cifs_aio_ctx *ctx; +/* +* iov_iter_get_pages_alloc() doesn't work with ITER_KVEC, +* fall back to data copy read path +* this could be improved by getting pages directly in ITER_KVEC +*/ +if (direct && to->type &ITER_KVEC ) { +cifs_dbg (FYI ,"use non-direct cifs_user_readv for kvec I/O\n" ); + direct = false; + } + len =iov_iter_count (to);if (!len)return 0 ;fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb · line 3430
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3305,14 +3430,20 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)Quoted 26 lines
fs/cifs/file.c +12
-6if (to->type ==ITER_IOVEC ) ctx->should_dirty = true; - rc =setup_aio_ctx_iter (ctx, to,READ ); -if (rc) { -kref_put (&ctx->refcount, cifs_aio_ctx_release); -return rc; +if (direct) { + ctx->pos = offset; + ctx->direct_io = true; + ctx->iter = *to; + ctx->len = len; + }else { + rc =setup_aio_ctx_iter (ctx, to,READ ); +if (rc) { +kref_put (&ctx->refcount, cifs_aio_ctx_release); +return rc; + } + len = ctx->len; } - len = ctx->len; -/* grab a lock here due to read response handlers can access ctx */ mutex_lock (&ctx->aio_mutex);Quoted 8 lines
Quoted 8 lines
fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb,
Quoted 1 line
fs/cifs/file.c +0
-0@@ -3354,6 +3485,16 @@ ssize_t cifs_user_readv(struct kiocb *iocb,struct
iov_iter*to)fs/cifs/file.c ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to) · line 3485
Quoted 1 line
fs/cifs/file.c +0
-0return rc;In case of a blocking read we will return
-EBUSYto the caller but read man page doesn't say anything about a possibility to return this error code. Is it being mapped somehow by VFS or is there another consideration? The same question applies to the write patch aswell.
Quoted 4 lines
Thanks Pavel, Yes this is a bug.
In practice, the retry logic will rarely get hit. If getting hit, normally the
server will give us far more credits for resending this whole packet.
Quoted 2 lines
How about just retrying forever (instead of trying 3 times) on this resend
packet? This will change the code to the same behavior before direct I/O patches.
Quoted 6 lines
e.g.:
/*
* Try to resend this wdata, waiting for credits before sending
* Note: we are attempting to resend the whole wdata not insegments
Quoted 5 lines
*/ do { rc = server->ops->wait_mtu_credits( server, wdata->bytes, &wsize, &credits);Quoted 9 lines
if (rc) break; if (wsize < wdata->bytes) { add_credits_and_wake_if(server, credits, 0); msleep(1000); } } while (wsize < wdata->bytes);Quoted 2 lines
We can do that but it won't be the same behavior because we were using a partial send:
Quoted 7 lines
Yes it's a little different.
I don't see any ill effects of doing this, other than we may wait for a little longer on a full credit to send the whole packet. In practice the server always offers a full credit on the first call to
wait_mtu_credits(), I am yet to see the while loop actually get looped.The resend path is rarely getting hit in stress tests on TCP, I estimate maybe less than 0.001% on my test setup, the while loop for waiting credits is ever harder to hit that I never see one.
On RDMA, the resend path is never used.
Ok. Another way to fix this is to break the request into several smaller ones consuming 1 credit per request - see cifs_writev_requeue() function that does the same thing for writes.
--
Best regards,
Pavel Shilovsky