[PATCH v3 00/16] SMB 2.1/3 multicredit requests for reading/writing

[PATCH v3 00/16] SMB 2.1/3 multicredit requests for reading/writing

[Pavel Shilovsky]

The goal of this patchset is to add large MTU/multicredits support for reading and writing from the CIFS client to servers that support SMB 2.1 and higher versions of the protocol.

The first patch fixes a reconnect bug in reading code. Other patches simplify the read/write code (2-4,7,8,11,13), fix rsize/wsize usage bug on reconnects (5,6,9,12,14,15) and introduce multicredits support (10,16).

Changes since v2:
1) patch #3 is refactored;
2) fix missed set_mapping_error() in patch #6;
3) fix missed updating of nr_pages value in patch #7;
4) remove unused offset var in patch #8;
5) add reviewed-by/acked-by tags.

Changes since v1:
1) read part is added;
2) several reconnects bug (related to the patchset and not) is fixed;
3) overall patch simplification/refactoring is done.


Pavel Shilovsky (16):
  CIFS: Fix async reading on reconnects
  CIFS: Separate page processing from writepages
  CIFS: Separate page sending from writepages
  CIFS: Separate pages initialization from writepages
  CIFS: Fix wsize usage in writepages
  CIFS: Fix cifs_writev_requeue when wsize changes
  CIFS: Separate filling pages from iovec write
  CIFS: Separate writing from iovec write
  CIFS: Fix wsize usage in iovec write
  CIFS: Use multicredits for SMB 2.1/3 writes
  CIFS: Separate page search from readpages
  CIFS: Fix rsize usage in readpages
  CIFS: Separate page reading from user read
  CIFS: Fix rsize usage in user read
  CIFS: Fix rsize usage for sync read
  CIFS: Use multicredits for SMB 2.1/3 reads

 fs/cifs/cifsglob.h      |  18 ++
 fs/cifs/cifsproto.h     |   3 +
 fs/cifs/cifssmb.c       |  88 ++++--
 fs/cifs/file.c          | 817 +++++++++++++++++++++++++++++-------------------
 fs/cifs/smb1ops.c       |   8 +
 fs/cifs/smb2ops.c       |  64 +++-
 fs/cifs/smb2pdu.c       |  63 +++-
 fs/cifs/smb2transport.c |   5 +
 fs/cifs/transport.c     |  25 +-
 9 files changed, 724 insertions(+), 367 deletions(-)

-- 
1.8.1.2

[PATCH v3 01/16] CIFS: Fix async reading on reconnects

[Pavel Shilovsky]

If we get into read_into_pages() from cifs_readv_receive() and then
loose a network, we issue cifs_reconnect that moves all mids to
a private list and issue their callbacks. The callback of the async
read request sets a mid to retry, frees it and wakes up a process
that waits on the rdata completion.

After the connection is established we return from read_into_pages()
with a short read, use the mid that was freed before and try to read
the remaining data from the a newly created socket. Both actions are
not what we want to do. In reconnect cases (-EAGAIN) we should not
mask off the error with a short read but should return the error
code instead.

Acked-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index e90a1e9..6b6df30 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2823,7 +2823,7 @@ cifs_uncached_read_into_pages(struct TCP_Server_Info *server,
 		total_read += result;
 	}
 
-	return total_read > 0 ? total_read : result;
+	return total_read > 0 && result != -EAGAIN ? total_read : result;
 }
 
 ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
@@ -3231,7 +3231,7 @@ cifs_readpages_read_into_pages(struct TCP_Server_Info *server,
 		total_read += result;
 	}
 
-	return total_read > 0 ? total_read : result;
+	return total_read > 0 && result != -EAGAIN ? total_read : result;
 }
 
 static int cifs_readpages(struct file *file, struct address_space *mapping,
-- 
1.8.1.2

[PATCH v3 02/16] CIFS: Separate page processing from writepages

[Pavel Shilovsky]

Reviewed-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 152 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 82 insertions(+), 70 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 6b6df30..69d1763 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1878,6 +1878,86 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
 	return rc;
 }
 
+static unsigned int
+wdata_prepare_pages(struct cifs_writedata *wdata, unsigned int found_pages,
+		    struct address_space *mapping,
+		    struct writeback_control *wbc,
+		    pgoff_t end, pgoff_t *index, pgoff_t *next, bool *done)
+{
+	unsigned int nr_pages = 0, i;
+	struct page *page;
+
+	for (i = 0; i < found_pages; i++) {
+		page = wdata->pages[i];
+		/*
+		 * At this point we hold neither mapping->tree_lock nor
+		 * lock on the page itself: the page may be truncated or
+		 * invalidated (changing page->mapping to NULL), or even
+		 * swizzled back from swapper_space to tmpfs file
+		 * mapping
+		 */
+
+		if (nr_pages == 0)
+			lock_page(page);
+		else if (!trylock_page(page))
+			break;
+
+		if (unlikely(page->mapping != mapping)) {
+			unlock_page(page);
+			break;
+		}
+
+		if (!wbc->range_cyclic && page->index > end) {
+			*done = true;
+			unlock_page(page);
+			break;
+		}
+
+		if (*next && (page->index != *next)) {
+			/* Not next consecutive page */
+			unlock_page(page);
+			break;
+		}
+
+		if (wbc->sync_mode != WB_SYNC_NONE)
+			wait_on_page_writeback(page);
+
+		if (PageWriteback(page) ||
+				!clear_page_dirty_for_io(page)) {
+			unlock_page(page);
+			break;
+		}
+
+		/*
+		 * This actually clears the dirty bit in the radix tree.
+		 * See cifs_writepage() for more commentary.
+		 */
+		set_page_writeback(page);
+		if (page_offset(page) >= i_size_read(mapping->host)) {
+			*done = true;
+			unlock_page(page);
+			end_page_writeback(page);
+			break;
+		}
+
+		wdata->pages[i] = page;
+		*next = page->index + 1;
+		++nr_pages;
+	}
+
+	/* reset index to refind any pages skipped */
+	if (nr_pages == 0)
+		*index = wdata->pages[0]->index + 1;
+
+	/* put any pages we aren't going to use */
+	for (i = nr_pages; i < found_pages; i++) {
+		page_cache_release(wdata->pages[i]);
+		wdata->pages[i] = NULL;
+	}
+
+	return nr_pages;
+}
+
 static int cifs_writepages(struct address_space *mapping,
 			   struct writeback_control *wbc)
 {
@@ -1886,7 +1966,6 @@ static int cifs_writepages(struct address_space *mapping,
 	pgoff_t end, index;
 	struct cifs_writedata *wdata;
 	struct TCP_Server_Info *server;
-	struct page *page;
 	int rc = 0;
 
 	/*
@@ -1944,75 +2023,8 @@ retry:
 			break;
 		}
 
-		nr_pages = 0;
-		for (i = 0; i < found_pages; i++) {
-			page = wdata->pages[i];
-			/*
-			 * At this point we hold neither mapping->tree_lock nor
-			 * lock on the page itself: the page may be truncated or
-			 * invalidated (changing page->mapping to NULL), or even
-			 * swizzled back from swapper_space to tmpfs file
-			 * mapping
-			 */
-
-			if (nr_pages == 0)
-				lock_page(page);
-			else if (!trylock_page(page))
-				break;
-
-			if (unlikely(page->mapping != mapping)) {
-				unlock_page(page);
-				break;
-			}
-
-			if (!wbc->range_cyclic && page->index > end) {
-				done = true;
-				unlock_page(page);
-				break;
-			}
-
-			if (next && (page->index != next)) {
-				/* Not next consecutive page */
-				unlock_page(page);
-				break;
-			}
-
-			if (wbc->sync_mode != WB_SYNC_NONE)
-				wait_on_page_writeback(page);
-
-			if (PageWriteback(page) ||
-					!clear_page_dirty_for_io(page)) {
-				unlock_page(page);
-				break;
-			}
-
-			/*
-			 * This actually clears the dirty bit in the radix tree.
-			 * See cifs_writepage() for more commentary.
-			 */
-			set_page_writeback(page);
-
-			if (page_offset(page) >= i_size_read(mapping->host)) {
-				done = true;
-				unlock_page(page);
-				end_page_writeback(page);
-				break;
-			}
-
-			wdata->pages[i] = page;
-			next = page->index + 1;
-			++nr_pages;
-		}
-
-		/* reset index to refind any pages skipped */
-		if (nr_pages == 0)
-			index = wdata->pages[0]->index + 1;
-
-		/* put any pages we aren't going to use */
-		for (i = nr_pages; i < found_pages; i++) {
-			page_cache_release(wdata->pages[i]);
-			wdata->pages[i] = NULL;
-		}
+		nr_pages = wdata_prepare_pages(wdata, found_pages, mapping, wbc,
+					       end, &index, &next, &done);
 
 		/* nothing to write? */
 		if (nr_pages == 0) {
-- 
1.8.1.2

[PATCH v3 03/16] CIFS: Separate page sending from writepages

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 68 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 69d1763..e76581f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1958,6 +1958,43 @@ wdata_prepare_pages(struct cifs_writedata *wdata, unsigned int found_pages,
 	return nr_pages;
 }
 
+static int
+wdata_send_pages(struct cifs_writedata *wdata, unsigned int nr_pages,
+		 struct address_space *mapping, struct writeback_control *wbc)
+{
+	int rc = 0;
+	struct TCP_Server_Info *server;
+	unsigned int i;
+
+	wdata->sync_mode = wbc->sync_mode;
+	wdata->nr_pages = nr_pages;
+	wdata->offset = page_offset(wdata->pages[0]);
+	wdata->pagesz = PAGE_CACHE_SIZE;
+	wdata->tailsz = min(i_size_read(mapping->host) -
+			page_offset(wdata->pages[nr_pages - 1]),
+			(loff_t)PAGE_CACHE_SIZE);
+	wdata->bytes = ((nr_pages - 1) * PAGE_CACHE_SIZE) + wdata->tailsz;
+
+	do {
+		if (wdata->cfile != NULL)
+			cifsFileInfo_put(wdata->cfile);
+		wdata->cfile = find_writable_file(CIFS_I(mapping->host), false);
+		if (!wdata->cfile) {
+			cifs_dbg(VFS, "No writable handles for inode\n");
+			rc = -EBADF;
+			break;
+		}
+		wdata->pid = wdata->cfile->pid;
+		server = tlink_tcon(wdata->cfile->tlink)->ses->server;
+		rc = server->ops->async_writev(wdata, cifs_writedata_release);
+	} while (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN);
+
+	for (i = 0; i < nr_pages; ++i)
+		unlock_page(wdata->pages[i]);
+
+	return rc;
+}
+
 static int cifs_writepages(struct address_space *mapping,
 			   struct writeback_control *wbc)
 {
@@ -1965,7 +2002,6 @@ static int cifs_writepages(struct address_space *mapping,
 	bool done = false, scanned = false, range_whole = false;
 	pgoff_t end, index;
 	struct cifs_writedata *wdata;
-	struct TCP_Server_Info *server;
 	int rc = 0;
 
 	/*
@@ -2032,35 +2068,7 @@ retry:
 			continue;
 		}
 
-		wdata->sync_mode = wbc->sync_mode;
-		wdata->nr_pages = nr_pages;
-		wdata->offset = page_offset(wdata->pages[0]);
-		wdata->pagesz = PAGE_CACHE_SIZE;
-		wdata->tailsz =
-			min(i_size_read(mapping->host) -
-			    page_offset(wdata->pages[nr_pages - 1]),
-			    (loff_t)PAGE_CACHE_SIZE);
-		wdata->bytes = ((nr_pages - 1) * PAGE_CACHE_SIZE) +
-					wdata->tailsz;
-
-		do {
-			if (wdata->cfile != NULL)
-				cifsFileInfo_put(wdata->cfile);
-			wdata->cfile = find_writable_file(CIFS_I(mapping->host),
-							  false);
-			if (!wdata->cfile) {
-				cifs_dbg(VFS, "No writable handles for inode\n");
-				rc = -EBADF;
-				break;
-			}
-			wdata->pid = wdata->cfile->pid;
-			server = tlink_tcon(wdata->cfile->tlink)->ses->server;
-			rc = server->ops->async_writev(wdata,
-							cifs_writedata_release);
-		} while (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN);
-
-		for (i = 0; i < nr_pages; ++i)
-			unlock_page(wdata->pages[i]);
+		rc = wdata_send_pages(wdata, nr_pages, mapping, wbc);
 
 		/* send failure -- clean up the mess */
 		if (rc != 0) {
-- 
1.8.1.2

Re: [PATCH v3 03/16] CIFS: Separate page sending from writepages

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 68 ++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 38 insertions(+), 30 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 69d1763..e76581f 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1958,6 +1958,43 @@ wdata_prepare_pages(struct cifs_writedata *wdata, unsigned int found_pages,
>         return nr_pages;
>  }
>
> +static int
> +wdata_send_pages(struct cifs_writedata *wdata, unsigned int nr_pages,
> +                struct address_space *mapping, struct writeback_control *wbc)
> +{
> +       int rc = 0;
> +       struct TCP_Server_Info *server;
> +       unsigned int i;
> +
> +       wdata->sync_mode = wbc->sync_mode;
> +       wdata->nr_pages = nr_pages;
> +       wdata->offset = page_offset(wdata->pages[0]);
> +       wdata->pagesz = PAGE_CACHE_SIZE;
> +       wdata->tailsz = min(i_size_read(mapping->host) -
> +                       page_offset(wdata->pages[nr_pages - 1]),
> +                       (loff_t)PAGE_CACHE_SIZE);
> +       wdata->bytes = ((nr_pages - 1) * PAGE_CACHE_SIZE) + wdata->tailsz;
> +
> +       do {
> +               if (wdata->cfile != NULL)
> +                       cifsFileInfo_put(wdata->cfile);
> +               wdata->cfile = find_writable_file(CIFS_I(mapping->host), false);
> +               if (!wdata->cfile) {
> +                       cifs_dbg(VFS, "No writable handles for inode\n");
> +                       rc = -EBADF;
> +                       break;
> +               }
> +               wdata->pid = wdata->cfile->pid;
> +               server = tlink_tcon(wdata->cfile->tlink)->ses->server;
> +               rc = server->ops->async_writev(wdata, cifs_writedata_release);
> +       } while (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN);
> +
> +       for (i = 0; i < nr_pages; ++i)
> +               unlock_page(wdata->pages[i]);
> +
> +       return rc;
> +}
> +
>  static int cifs_writepages(struct address_space *mapping,
>                            struct writeback_control *wbc)
>  {
> @@ -1965,7 +2002,6 @@ static int cifs_writepages(struct address_space *mapping,
>         bool done = false, scanned = false, range_whole = false;
>         pgoff_t end, index;
>         struct cifs_writedata *wdata;
> -       struct TCP_Server_Info *server;
>         int rc = 0;
>
>         /*
> @@ -2032,35 +2068,7 @@ retry:
>                         continue;
>                 }
>
> -               wdata->sync_mode = wbc->sync_mode;
> -               wdata->nr_pages = nr_pages;
> -               wdata->offset = page_offset(wdata->pages[0]);
> -               wdata->pagesz = PAGE_CACHE_SIZE;
> -               wdata->tailsz =
> -                       min(i_size_read(mapping->host) -
> -                           page_offset(wdata->pages[nr_pages - 1]),
> -                           (loff_t)PAGE_CACHE_SIZE);
> -               wdata->bytes = ((nr_pages - 1) * PAGE_CACHE_SIZE) +
> -                                       wdata->tailsz;
> -
> -               do {
> -                       if (wdata->cfile != NULL)
> -                               cifsFileInfo_put(wdata->cfile);
> -                       wdata->cfile = find_writable_file(CIFS_I(mapping->host),
> -                                                         false);
> -                       if (!wdata->cfile) {
> -                               cifs_dbg(VFS, "No writable handles for inode\n");
> -                               rc = -EBADF;
> -                               break;
> -                       }
> -                       wdata->pid = wdata->cfile->pid;
> -                       server = tlink_tcon(wdata->cfile->tlink)->ses->server;
> -                       rc = server->ops->async_writev(wdata,
> -                                                       cifs_writedata_release);
> -               } while (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN);
> -
> -               for (i = 0; i < nr_pages; ++i)
> -                       unlock_page(wdata->pages[i]);
> +               rc = wdata_send_pages(wdata, nr_pages, mapping, wbc);
>
>                 /* send failure -- clean up the mess */
>                 if (rc != 0) {
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 04/16] CIFS: Separate pages initialization from writepages

[Pavel Shilovsky]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 56 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index e76581f..0064d38f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1878,6 +1878,40 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
 	return rc;
 }
 
+static struct cifs_writedata *
+wdata_alloc_and_fillpages(pgoff_t tofind, struct address_space *mapping,
+			  pgoff_t end, pgoff_t *index,
+			  unsigned int *found_pages)
+{
+	unsigned int nr_pages;
+	struct page **pages;
+	struct cifs_writedata *wdata;
+
+	wdata = cifs_writedata_alloc((unsigned int)tofind,
+				     cifs_writev_complete);
+	if (!wdata)
+		return NULL;
+
+	/*
+	 * find_get_pages_tag seems to return a max of 256 on each
+	 * iteration, so we must call it several times in order to
+	 * fill the array or the wsize is effectively limited to
+	 * 256 * PAGE_CACHE_SIZE.
+	 */
+	*found_pages = 0;
+	pages = wdata->pages;
+	do {
+		nr_pages = find_get_pages_tag(mapping, index,
+					      PAGECACHE_TAG_DIRTY, tofind,
+					      pages);
+		*found_pages += nr_pages;
+		tofind -= nr_pages;
+		pages += nr_pages;
+	} while (nr_pages && tofind && *index <= end);
+
+	return wdata;
+}
+
 static unsigned int
 wdata_prepare_pages(struct cifs_writedata *wdata, unsigned int found_pages,
 		    struct address_space *mapping,
@@ -2025,35 +2059,17 @@ retry:
 	while (!done && index <= end) {
 		unsigned int i, nr_pages, found_pages;
 		pgoff_t next = 0, tofind;
-		struct page **pages;
 
 		tofind = min((cifs_sb->wsize / PAGE_CACHE_SIZE) - 1,
 				end - index) + 1;
 
-		wdata = cifs_writedata_alloc((unsigned int)tofind,
-					     cifs_writev_complete);
+		wdata = wdata_alloc_and_fillpages(tofind, mapping, end, &index,
+						  &found_pages);
 		if (!wdata) {
 			rc = -ENOMEM;
 			break;
 		}
 
-		/*
-		 * find_get_pages_tag seems to return a max of 256 on each
-		 * iteration, so we must call it several times in order to
-		 * fill the array or the wsize is effectively limited to
-		 * 256 * PAGE_CACHE_SIZE.
-		 */
-		found_pages = 0;
-		pages = wdata->pages;
-		do {
-			nr_pages = find_get_pages_tag(mapping, &index,
-							PAGECACHE_TAG_DIRTY,
-							tofind, pages);
-			found_pages += nr_pages;
-			tofind -= nr_pages;
-			pages += nr_pages;
-		} while (nr_pages && tofind && index <= end);
-
 		if (found_pages == 0) {
 			kref_put(&wdata->refcount, cifs_writedata_release);
 			break;
-- 
1.8.1.2

[PATCH v3 05/16] CIFS: Fix wsize usage in writepages

[Pavel Shilovsky]

If a server change maximum buffer size for write (wsize) requests
on reconnect we can fail on repeating with a big size buffer on
-EAGAIN error in writepages. Fix this by checking wsize all the
time before repeating request in writepages.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 0064d38f..ff4a9c3 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2009,19 +2009,17 @@ wdata_send_pages(struct cifs_writedata *wdata, unsigned int nr_pages,
 			(loff_t)PAGE_CACHE_SIZE);
 	wdata->bytes = ((nr_pages - 1) * PAGE_CACHE_SIZE) + wdata->tailsz;
 
-	do {
-		if (wdata->cfile != NULL)
-			cifsFileInfo_put(wdata->cfile);
-		wdata->cfile = find_writable_file(CIFS_I(mapping->host), false);
-		if (!wdata->cfile) {
-			cifs_dbg(VFS, "No writable handles for inode\n");
-			rc = -EBADF;
-			break;
-		}
+	if (wdata->cfile != NULL)
+		cifsFileInfo_put(wdata->cfile);
+	wdata->cfile = find_writable_file(CIFS_I(mapping->host), false);
+	if (!wdata->cfile) {
+		cifs_dbg(VFS, "No writable handles for inode\n");
+		rc = -EBADF;
+	} else {
 		wdata->pid = wdata->cfile->pid;
 		server = tlink_tcon(wdata->cfile->tlink)->ses->server;
 		rc = server->ops->async_writev(wdata, cifs_writedata_release);
-	} while (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN);
+	}
 
 	for (i = 0; i < nr_pages; ++i)
 		unlock_page(wdata->pages[i]);
@@ -2058,7 +2056,7 @@ static int cifs_writepages(struct address_space *mapping,
 retry:
 	while (!done && index <= end) {
 		unsigned int i, nr_pages, found_pages;
-		pgoff_t next = 0, tofind;
+		pgoff_t next = 0, tofind, saved_index = index;
 
 		tofind = min((cifs_sb->wsize / PAGE_CACHE_SIZE) - 1,
 				end - index) + 1;
@@ -2102,6 +2100,11 @@ retry:
 		}
 		kref_put(&wdata->refcount, cifs_writedata_release);
 
+		if (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN) {
+			index = saved_index;
+			continue;
+		}
+
 		wbc->nr_to_write -= nr_pages;
 		if (wbc->nr_to_write <= 0)
 			done = true;
-- 
1.8.1.2

Re: [PATCH v3 05/16] CIFS: Fix wsize usage in writepages

[Shirish Pargaonkar]

Reviewed-by; Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If a server change maximum buffer size for write (wsize) requests
> on reconnect we can fail on repeating with a big size buffer on
> -EAGAIN error in writepages. Fix this by checking wsize all the
> time before repeating request in writepages.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 0064d38f..ff4a9c3 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2009,19 +2009,17 @@ wdata_send_pages(struct cifs_writedata *wdata, unsigned int nr_pages,
>                         (loff_t)PAGE_CACHE_SIZE);
>         wdata->bytes = ((nr_pages - 1) * PAGE_CACHE_SIZE) + wdata->tailsz;
>
> -       do {
> -               if (wdata->cfile != NULL)
> -                       cifsFileInfo_put(wdata->cfile);
> -               wdata->cfile = find_writable_file(CIFS_I(mapping->host), false);
> -               if (!wdata->cfile) {
> -                       cifs_dbg(VFS, "No writable handles for inode\n");
> -                       rc = -EBADF;
> -                       break;
> -               }
> +       if (wdata->cfile != NULL)
> +               cifsFileInfo_put(wdata->cfile);
> +       wdata->cfile = find_writable_file(CIFS_I(mapping->host), false);
> +       if (!wdata->cfile) {
> +               cifs_dbg(VFS, "No writable handles for inode\n");
> +               rc = -EBADF;
> +       } else {
>                 wdata->pid = wdata->cfile->pid;
>                 server = tlink_tcon(wdata->cfile->tlink)->ses->server;
>                 rc = server->ops->async_writev(wdata, cifs_writedata_release);
> -       } while (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN);
> +       }
>
>         for (i = 0; i < nr_pages; ++i)
>                 unlock_page(wdata->pages[i]);
> @@ -2058,7 +2056,7 @@ static int cifs_writepages(struct address_space *mapping,
>  retry:
>         while (!done && index <= end) {
>                 unsigned int i, nr_pages, found_pages;
> -               pgoff_t next = 0, tofind;
> +               pgoff_t next = 0, tofind, saved_index = index;
>
>                 tofind = min((cifs_sb->wsize / PAGE_CACHE_SIZE) - 1,
>                                 end - index) + 1;
> @@ -2102,6 +2100,11 @@ retry:
>                 }
>                 kref_put(&wdata->refcount, cifs_writedata_release);
>
> +               if (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN) {
> +                       index = saved_index;
> +                       continue;
> +               }
> +
>                 wbc->nr_to_write -= nr_pages;
>                 if (wbc->nr_to_write <= 0)
>                         done = true;
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 06/16] CIFS: Fix cifs_writev_requeue when wsize changes

[Pavel Shilovsky]

If wsize changes on reconnect we need to use new writedata structure
that for retrying.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h |  2 ++
 fs/cifs/cifssmb.c  | 84 +++++++++++++++++++++++++++++++++++++++++++-----------
 fs/cifs/smb1ops.c  |  7 +++++
 fs/cifs/smb2ops.c  | 10 +++++++
 4 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index de6aed8..8c53f20 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -404,6 +404,8 @@ struct smb_version_operations {
 			const struct cifs_fid *, u32 *);
 	int (*set_acl)(struct cifs_ntsd *, __u32, struct inode *, const char *,
 			int);
+	/* writepages retry size */
+	unsigned int (*wp_retry_size)(struct inode *);
 };
 
 struct smb_version_values {
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 6ce4e09..e273ded 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1899,28 +1899,80 @@ cifs_writedata_release(struct kref *refcount)
 static void
 cifs_writev_requeue(struct cifs_writedata *wdata)
 {
-	int i, rc;
+	int i, rc = 0;
 	struct inode *inode = wdata->cfile->dentry->d_inode;
 	struct TCP_Server_Info *server;
+	unsigned int rest_len;
 
-	for (i = 0; i < wdata->nr_pages; i++) {
-		lock_page(wdata->pages[i]);
-		clear_page_dirty_for_io(wdata->pages[i]);
-	}
-
+	server = tlink_tcon(wdata->cfile->tlink)->ses->server;
+	i = 0;
+	rest_len = wdata->bytes;
 	do {
-		server = tlink_tcon(wdata->cfile->tlink)->ses->server;
-		rc = server->ops->async_writev(wdata, cifs_writedata_release);
-	} while (rc == -EAGAIN);
+		struct cifs_writedata *wdata2;
+		unsigned int j, nr_pages, wsize, tailsz, cur_len;
+
+		wsize = server->ops->wp_retry_size(inode);
+		if (wsize < rest_len) {
+			nr_pages = wsize / PAGE_CACHE_SIZE;
+			if (!nr_pages) {
+				rc = -ENOTSUPP;
+				break;
+			}
+			cur_len = nr_pages * PAGE_CACHE_SIZE;
+			tailsz = PAGE_CACHE_SIZE;
+		} else {
+			nr_pages = DIV_ROUND_UP(rest_len, PAGE_CACHE_SIZE);
+			cur_len = rest_len;
+			tailsz = rest_len - (nr_pages - 1) * PAGE_CACHE_SIZE;
+		}
 
-	for (i = 0; i < wdata->nr_pages; i++) {
-		unlock_page(wdata->pages[i]);
-		if (rc != 0) {
-			SetPageError(wdata->pages[i]);
-			end_page_writeback(wdata->pages[i]);
-			page_cache_release(wdata->pages[i]);
+		wdata2 = cifs_writedata_alloc(nr_pages, cifs_writev_complete);
+		if (!wdata2) {
+			rc = -ENOMEM;
+			break;
 		}
-	}
+
+		for (j = 0; j < nr_pages; j++) {
+			wdata2->pages[j] = wdata->pages[i + j];
+			lock_page(wdata2->pages[j]);
+			clear_page_dirty_for_io(wdata2->pages[j]);
+		}
+
+		wdata2->sync_mode = wdata->sync_mode;
+		wdata2->nr_pages = nr_pages;
+		wdata2->offset = page_offset(wdata2->pages[0]);
+		wdata2->pagesz = PAGE_CACHE_SIZE;
+		wdata2->tailsz = tailsz;
+		wdata2->bytes = cur_len;
+
+		wdata2->cfile = find_writable_file(CIFS_I(inode), false);
+		if (!wdata2->cfile) {
+			cifs_dbg(VFS, "No writable handles for inode\n");
+			rc = -EBADF;
+			break;
+		}
+		wdata2->pid = wdata2->cfile->pid;
+		rc = server->ops->async_writev(wdata2, cifs_writedata_release);
+
+		for (j = 0; j < nr_pages; j++) {
+			unlock_page(wdata2->pages[j]);
+			if (rc != 0 && rc != -EAGAIN) {
+				SetPageError(wdata2->pages[j]);
+				end_page_writeback(wdata2->pages[j]);
+				page_cache_release(wdata2->pages[j]);
+			}
+		}
+
+		if (rc) {
+			kref_put(&wdata2->refcount, cifs_writedata_release);
+			if (rc == -EAGAIN)
+				continue;
+			break;
+		}
+
+		rest_len -= cur_len;
+		i += nr_pages;
+	} while (i < wdata->nr_pages);
 
 	mapping_set_error(inode->i_mapping, rc);
 	kref_put(&wdata->refcount, cifs_writedata_release);
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index d1fdfa8..8a96342 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -1009,6 +1009,12 @@ cifs_is_read_op(__u32 oplock)
 	return oplock == OPLOCK_READ;
 }
 
+static unsigned int
+cifs_wp_retry_size(struct inode *inode)
+{
+	return CIFS_SB(inode->i_sb)->wsize;
+}
+
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
@@ -1078,6 +1084,7 @@ struct smb_version_operations smb1_operations = {
 	.query_mf_symlink = cifs_query_mf_symlink,
 	.create_mf_symlink = cifs_create_mf_symlink,
 	.is_read_op = cifs_is_read_op,
+	.wp_retry_size = cifs_wp_retry_size,
 #ifdef CONFIG_CIFS_XATTR
 	.query_all_EAs = CIFSSMBQAllEAs,
 	.set_EA = CIFSSMBSetEA,
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 787844b..e35ce5b 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1104,6 +1104,13 @@ smb3_parse_lease_buf(void *buf, unsigned int *epoch)
 	return le32_to_cpu(lc->lcontext.LeaseState);
 }
 
+static unsigned int
+smb2_wp_retry_size(struct inode *inode)
+{
+	return min_t(unsigned int, CIFS_SB(inode->i_sb)->wsize,
+		     SMB2_MAX_BUFFER_SIZE);
+}
+
 struct smb_version_operations smb20_operations = {
 	.compare_fids = smb2_compare_fids,
 	.setup_request = smb2_setup_request,
@@ -1177,6 +1184,7 @@ struct smb_version_operations smb20_operations = {
 	.create_lease_buf = smb2_create_lease_buf,
 	.parse_lease_buf = smb2_parse_lease_buf,
 	.clone_range = smb2_clone_range,
+	.wp_retry_size = smb2_wp_retry_size,
 };
 
 struct smb_version_operations smb21_operations = {
@@ -1252,6 +1260,7 @@ struct smb_version_operations smb21_operations = {
 	.create_lease_buf = smb2_create_lease_buf,
 	.parse_lease_buf = smb2_parse_lease_buf,
 	.clone_range = smb2_clone_range,
+	.wp_retry_size = smb2_wp_retry_size,
 };
 
 struct smb_version_operations smb30_operations = {
@@ -1330,6 +1339,7 @@ struct smb_version_operations smb30_operations = {
 	.parse_lease_buf = smb3_parse_lease_buf,
 	.clone_range = smb2_clone_range,
 	.validate_negotiate = smb3_validate_negotiate,
+	.wp_retry_size = smb2_wp_retry_size,
 };
 
 struct smb_version_values smb20_values = {
-- 
1.8.1.2

Re: [PATCH v3 06/16] CIFS: Fix cifs_writev_requeue when wsize changes

[Shirish Pargaonkar]

Looks correct (retainng mapping_set_error()).

Reviewed-by:  Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If wsize changes on reconnect we need to use new writedata structure
> that for retrying.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |  2 ++
>  fs/cifs/cifssmb.c  | 84 +++++++++++++++++++++++++++++++++++++++++++-----------
>  fs/cifs/smb1ops.c  |  7 +++++
>  fs/cifs/smb2ops.c  | 10 +++++++
>  4 files changed, 87 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index de6aed8..8c53f20 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -404,6 +404,8 @@ struct smb_version_operations {
>                         const struct cifs_fid *, u32 *);
>         int (*set_acl)(struct cifs_ntsd *, __u32, struct inode *, const char *,
>                         int);
> +       /* writepages retry size */
> +       unsigned int (*wp_retry_size)(struct inode *);
>  };
>
>  struct smb_version_values {
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 6ce4e09..e273ded 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -1899,28 +1899,80 @@ cifs_writedata_release(struct kref *refcount)
>  static void
>  cifs_writev_requeue(struct cifs_writedata *wdata)
>  {
> -       int i, rc;
> +       int i, rc = 0;
>         struct inode *inode = wdata->cfile->dentry->d_inode;
>         struct TCP_Server_Info *server;
> +       unsigned int rest_len;
>
> -       for (i = 0; i < wdata->nr_pages; i++) {
> -               lock_page(wdata->pages[i]);
> -               clear_page_dirty_for_io(wdata->pages[i]);
> -       }
> -
> +       server = tlink_tcon(wdata->cfile->tlink)->ses->server;
> +       i = 0;
> +       rest_len = wdata->bytes;
>         do {
> -               server = tlink_tcon(wdata->cfile->tlink)->ses->server;
> -               rc = server->ops->async_writev(wdata, cifs_writedata_release);
> -       } while (rc == -EAGAIN);
> +               struct cifs_writedata *wdata2;
> +               unsigned int j, nr_pages, wsize, tailsz, cur_len;
> +
> +               wsize = server->ops->wp_retry_size(inode);
> +               if (wsize < rest_len) {
> +                       nr_pages = wsize / PAGE_CACHE_SIZE;
> +                       if (!nr_pages) {
> +                               rc = -ENOTSUPP;
> +                               break;
> +                       }
> +                       cur_len = nr_pages * PAGE_CACHE_SIZE;
> +                       tailsz = PAGE_CACHE_SIZE;
> +               } else {
> +                       nr_pages = DIV_ROUND_UP(rest_len, PAGE_CACHE_SIZE);
> +                       cur_len = rest_len;
> +                       tailsz = rest_len - (nr_pages - 1) * PAGE_CACHE_SIZE;
> +               }
>
> -       for (i = 0; i < wdata->nr_pages; i++) {
> -               unlock_page(wdata->pages[i]);
> -               if (rc != 0) {
> -                       SetPageError(wdata->pages[i]);
> -                       end_page_writeback(wdata->pages[i]);
> -                       page_cache_release(wdata->pages[i]);
> +               wdata2 = cifs_writedata_alloc(nr_pages, cifs_writev_complete);
> +               if (!wdata2) {
> +                       rc = -ENOMEM;
> +                       break;
>                 }
> -       }
> +
> +               for (j = 0; j < nr_pages; j++) {
> +                       wdata2->pages[j] = wdata->pages[i + j];
> +                       lock_page(wdata2->pages[j]);
> +                       clear_page_dirty_for_io(wdata2->pages[j]);
> +               }
> +
> +               wdata2->sync_mode = wdata->sync_mode;
> +               wdata2->nr_pages = nr_pages;
> +               wdata2->offset = page_offset(wdata2->pages[0]);
> +               wdata2->pagesz = PAGE_CACHE_SIZE;
> +               wdata2->tailsz = tailsz;
> +               wdata2->bytes = cur_len;
> +
> +               wdata2->cfile = find_writable_file(CIFS_I(inode), false);
> +               if (!wdata2->cfile) {
> +                       cifs_dbg(VFS, "No writable handles for inode\n");
> +                       rc = -EBADF;
> +                       break;
> +               }
> +               wdata2->pid = wdata2->cfile->pid;
> +               rc = server->ops->async_writev(wdata2, cifs_writedata_release);
> +
> +               for (j = 0; j < nr_pages; j++) {
> +                       unlock_page(wdata2->pages[j]);
> +                       if (rc != 0 && rc != -EAGAIN) {
> +                               SetPageError(wdata2->pages[j]);
> +                               end_page_writeback(wdata2->pages[j]);
> +                               page_cache_release(wdata2->pages[j]);
> +                       }
> +               }
> +
> +               if (rc) {
> +                       kref_put(&wdata2->refcount, cifs_writedata_release);
> +                       if (rc == -EAGAIN)
> +                               continue;
> +                       break;
> +               }
> +
> +               rest_len -= cur_len;
> +               i += nr_pages;
> +       } while (i < wdata->nr_pages);
>
>         mapping_set_error(inode->i_mapping, rc);
>         kref_put(&wdata->refcount, cifs_writedata_release);
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index d1fdfa8..8a96342 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -1009,6 +1009,12 @@ cifs_is_read_op(__u32 oplock)
>         return oplock == OPLOCK_READ;
>  }
>
> +static unsigned int
> +cifs_wp_retry_size(struct inode *inode)
> +{
> +       return CIFS_SB(inode->i_sb)->wsize;
> +}
> +
>  struct smb_version_operations smb1_operations = {
>         .send_cancel = send_nt_cancel,
>         .compare_fids = cifs_compare_fids,
> @@ -1078,6 +1084,7 @@ struct smb_version_operations smb1_operations = {
>         .query_mf_symlink = cifs_query_mf_symlink,
>         .create_mf_symlink = cifs_create_mf_symlink,
>         .is_read_op = cifs_is_read_op,
> +       .wp_retry_size = cifs_wp_retry_size,
>  #ifdef CONFIG_CIFS_XATTR
>         .query_all_EAs = CIFSSMBQAllEAs,
>         .set_EA = CIFSSMBSetEA,
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 787844b..e35ce5b 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1104,6 +1104,13 @@ smb3_parse_lease_buf(void *buf, unsigned int *epoch)
>         return le32_to_cpu(lc->lcontext.LeaseState);
>  }
>
> +static unsigned int
> +smb2_wp_retry_size(struct inode *inode)
> +{
> +       return min_t(unsigned int, CIFS_SB(inode->i_sb)->wsize,
> +                    SMB2_MAX_BUFFER_SIZE);
> +}
> +
>  struct smb_version_operations smb20_operations = {
>         .compare_fids = smb2_compare_fids,
>         .setup_request = smb2_setup_request,
> @@ -1177,6 +1184,7 @@ struct smb_version_operations smb20_operations = {
>         .create_lease_buf = smb2_create_lease_buf,
>         .parse_lease_buf = smb2_parse_lease_buf,
>         .clone_range = smb2_clone_range,
> +       .wp_retry_size = smb2_wp_retry_size,
>  };
>
>  struct smb_version_operations smb21_operations = {
> @@ -1252,6 +1260,7 @@ struct smb_version_operations smb21_operations = {
>         .create_lease_buf = smb2_create_lease_buf,
>         .parse_lease_buf = smb2_parse_lease_buf,
>         .clone_range = smb2_clone_range,
> +       .wp_retry_size = smb2_wp_retry_size,
>  };
>
>  struct smb_version_operations smb30_operations = {
> @@ -1330,6 +1339,7 @@ struct smb_version_operations smb30_operations = {
>         .parse_lease_buf = smb3_parse_lease_buf,
>         .clone_range = smb2_clone_range,
>         .validate_negotiate = smb3_validate_negotiate,
> +       .wp_retry_size = smb2_wp_retry_size,
>  };
>
>  struct smb_version_values smb20_values = {
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 07/16] CIFS: Separate filling pages from iovec write

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 84 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 50 insertions(+), 34 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index ff4a9c3..f96f61c 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2423,11 +2423,53 @@ cifs_uncached_retry_writev(struct cifs_writedata *wdata)
 	return rc;
 }
 
+static int
+wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
+		      size_t *len, unsigned long *num_pages)
+{
+	size_t save_len, copied, bytes, cur_len = *len;
+	unsigned long i, nr_pages = *num_pages;
+
+	save_len = cur_len;
+	for (i = 0; i < nr_pages; i++) {
+		bytes = min_t(const size_t, cur_len, PAGE_SIZE);
+		copied = copy_page_from_iter(wdata->pages[i], 0, bytes, from);
+		cur_len -= copied;
+		/*
+		 * If we didn't copy as much as we expected, then that
+		 * may mean we trod into an unmapped area. Stop copying
+		 * at that point. On the next pass through the big
+		 * loop, we'll likely end up getting a zero-length
+		 * write and bailing out of it.
+		 */
+		if (copied < bytes)
+			break;
+	}
+	cur_len = save_len - cur_len;
+	*len = cur_len;
+
+	/*
+	 * If we have no data to send, then that probably means that
+	 * the copy above failed altogether. That's most likely because
+	 * the address in the iovec was bogus. Return -EFAULT and let
+	 * the caller free anything we allocated and bail out.
+	 */
+	if (!cur_len)
+		return -EFAULT;
+
+	/*
+	 * i + 1 now represents the number of pages we actually used in
+	 * the copy phase above.
+	 */
+	*num_pages = i + 1;
+	return 0;
+}
+
 static ssize_t
 cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 {
-	unsigned long nr_pages, i;
-	size_t bytes, copied, len, cur_len;
+	unsigned long nr_pages, num_pages, i;
+	size_t len, cur_len;
 	ssize_t total_written = 0;
 	loff_t offset;
 	struct cifsFileInfo *open_file;
@@ -2464,8 +2506,6 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 		pid = current->tgid;
 
 	do {
-		size_t save_len;
-
 		nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
 		wdata = cifs_writedata_alloc(nr_pages,
 					     cifs_uncached_writev_complete);
@@ -2480,44 +2520,20 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 			break;
 		}
 
-		save_len = cur_len;
-		for (i = 0; i < nr_pages; i++) {
-			bytes = min_t(size_t, cur_len, PAGE_SIZE);
-			copied = copy_page_from_iter(wdata->pages[i], 0, bytes,
-						     from);
-			cur_len -= copied;
-			/*
-			 * If we didn't copy as much as we expected, then that
-			 * may mean we trod into an unmapped area. Stop copying
-			 * at that point. On the next pass through the big
-			 * loop, we'll likely end up getting a zero-length
-			 * write and bailing out of it.
-			 */
-			if (copied < bytes)
-				break;
-		}
-		cur_len = save_len - cur_len;
-
-		/*
-		 * If we have no data to send, then that probably means that
-		 * the copy above failed altogether. That's most likely because
-		 * the address in the iovec was bogus. Set the rc to -EFAULT,
-		 * free anything we allocated and bail out.
-		 */
-		if (!cur_len) {
+		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);
-			rc = -EFAULT;
 			break;
 		}
 
 		/*
-		 * i + 1 now represents the number of pages we actually used in
-		 * the copy phase above. Bring nr_pages down to that, and free
-		 * any pages that we didn't use.
+		 * Bring nr_pages down to the number of pages we actually used,
+		 * and free any pages that we didn't use.
 		 */
-		for ( ; nr_pages > i + 1; nr_pages--)
+		for ( ; nr_pages > num_pages; nr_pages--)
 			put_page(wdata->pages[nr_pages - 1]);
 
 		wdata->sync_mode = WB_SYNC_ALL;
-- 
1.8.1.2

Re: [PATCH v3 07/16] CIFS: Separate filling pages from iovec write

[Shirish Pargaonkar]

Looks correct (fixing the return value from the function
wdata_fill_from_iovec())

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 84 ++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 50 insertions(+), 34 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index ff4a9c3..f96f61c 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2423,11 +2423,53 @@ cifs_uncached_retry_writev(struct cifs_writedata *wdata)
>         return rc;
>  }
>
> +static int
> +wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
> +                     size_t *len, unsigned long *num_pages)
> +{
> +       size_t save_len, copied, bytes, cur_len = *len;
> +       unsigned long i, nr_pages = *num_pages;
> +
> +       save_len = cur_len;
> +       for (i = 0; i < nr_pages; i++) {
> +               bytes = min_t(const size_t, cur_len, PAGE_SIZE);
> +               copied = copy_page_from_iter(wdata->pages[i], 0, bytes, from);
> +               cur_len -= copied;
> +               /*
> +                * If we didn't copy as much as we expected, then that
> +                * may mean we trod into an unmapped area. Stop copying
> +                * at that point. On the next pass through the big
> +                * loop, we'll likely end up getting a zero-length
> +                * write and bailing out of it.
> +                */
> +               if (copied < bytes)
> +                       break;
> +       }
> +       cur_len = save_len - cur_len;
> +       *len = cur_len;
> +
> +       /*
> +        * If we have no data to send, then that probably means that
> +        * the copy above failed altogether. That's most likely because
> +        * the address in the iovec was bogus. Return -EFAULT and let
> +        * the caller free anything we allocated and bail out.
> +        */
> +       if (!cur_len)
> +               return -EFAULT;
> +
> +       /*
> +        * i + 1 now represents the number of pages we actually used in
> +        * the copy phase above.
> +        */
> +       *num_pages = i + 1;
> +       return 0;
> +}
> +
>  static ssize_t
>  cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>  {
> -       unsigned long nr_pages, i;
> -       size_t bytes, copied, len, cur_len;
> +       unsigned long nr_pages, num_pages, i;
> +       size_t len, cur_len;
>         ssize_t total_written = 0;
>         loff_t offset;
>         struct cifsFileInfo *open_file;
> @@ -2464,8 +2506,6 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>                 pid = current->tgid;
>
>         do {
> -               size_t save_len;
> -
>                 nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
>                 wdata = cifs_writedata_alloc(nr_pages,
>                                              cifs_uncached_writev_complete);
> @@ -2480,44 +2520,20 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>                         break;
>                 }
>
> -               save_len = cur_len;
> -               for (i = 0; i < nr_pages; i++) {
> -                       bytes = min_t(size_t, cur_len, PAGE_SIZE);
> -                       copied = copy_page_from_iter(wdata->pages[i], 0, bytes,
> -                                                    from);
> -                       cur_len -= copied;
> -                       /*
> -                        * If we didn't copy as much as we expected, then that
> -                        * may mean we trod into an unmapped area. Stop copying
> -                        * at that point. On the next pass through the big
> -                        * loop, we'll likely end up getting a zero-length
> -                        * write and bailing out of it.
> -                        */
> -                       if (copied < bytes)
> -                               break;
> -               }
> -               cur_len = save_len - cur_len;
> -
> -               /*
> -                * If we have no data to send, then that probably means that
> -                * the copy above failed altogether. That's most likely because
> -                * the address in the iovec was bogus. Set the rc to -EFAULT,
> -                * free anything we allocated and bail out.
> -                */
> -               if (!cur_len) {
> +               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);
> -                       rc = -EFAULT;
>                         break;
>                 }
>
>                 /*
> -                * i + 1 now represents the number of pages we actually used in
> -                * the copy phase above. Bring nr_pages down to that, and free
> -                * any pages that we didn't use.
> +                * Bring nr_pages down to the number of pages we actually used,
> +                * and free any pages that we didn't use.
>                  */
> -               for ( ; nr_pages > i + 1; nr_pages--)
> +               for ( ; nr_pages > num_pages; nr_pages--)
>                         put_page(wdata->pages[nr_pages - 1]);
>
>                 wdata->sync_mode = WB_SYNC_ALL;
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 08/16] CIFS: Separate writing from iovec write

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 76 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 44 insertions(+), 32 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index f96f61c..6660698 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2465,41 +2465,17 @@ wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
 	return 0;
 }
 
-static ssize_t
-cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
+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)
 {
+	int rc = 0;
+	size_t cur_len;
 	unsigned long nr_pages, num_pages, i;
-	size_t len, cur_len;
-	ssize_t total_written = 0;
-	loff_t offset;
-	struct cifsFileInfo *open_file;
-	struct cifs_tcon *tcon;
-	struct cifs_sb_info *cifs_sb;
-	struct cifs_writedata *wdata, *tmp;
-	struct list_head wdata_list;
-	int rc;
+	struct cifs_writedata *wdata;
 	pid_t pid;
 
-	len = iov_iter_count(from);
-	rc = generic_write_checks(file, poffset, &len, 0);
-	if (rc)
-		return rc;
-
-	if (!len)
-		return 0;
-
-	iov_iter_truncate(from, len);
-
-	INIT_LIST_HEAD(&wdata_list);
-	cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
-	open_file = file->private_data;
-	tcon = tlink_tcon(open_file->tlink);
-
-	if (!tcon->ses->server->ops->async_writev)
-		return -ENOSYS;
-
-	offset = *poffset;
-
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
 		pid = open_file->pid;
 	else
@@ -2551,11 +2527,47 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 			break;
 		}
 
-		list_add_tail(&wdata->list, &wdata_list);
+		list_add_tail(&wdata->list, wdata_list);
 		offset += cur_len;
 		len -= cur_len;
 	} while (len > 0);
 
+	return rc;
+}
+
+static ssize_t
+cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
+{
+	size_t len;
+	ssize_t total_written = 0;
+	struct cifsFileInfo *open_file;
+	struct cifs_tcon *tcon;
+	struct cifs_sb_info *cifs_sb;
+	struct cifs_writedata *wdata, *tmp;
+	struct list_head wdata_list;
+	int rc;
+
+	len = iov_iter_count(from);
+	rc = generic_write_checks(file, poffset, &len, 0);
+	if (rc)
+		return rc;
+
+	if (!len)
+		return 0;
+
+	iov_iter_truncate(from, len);
+
+	INIT_LIST_HEAD(&wdata_list);
+	cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
+	open_file = file->private_data;
+	tcon = tlink_tcon(open_file->tlink);
+
+	if (!tcon->ses->server->ops->async_writev)
+		return -ENOSYS;
+
+	rc = cifs_write_from_iter(*poffset, len, from, open_file, cifs_sb,
+				  &wdata_list);
+
 	/*
 	 * If at least one write was successfully sent, then discard any rc
 	 * value from the later writes. If the other write succeeds, then
-- 
1.8.1.2

Re: [PATCH v3 08/16] CIFS: Separate writing from iovec write

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 76 +++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index f96f61c..6660698 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2465,41 +2465,17 @@ wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
>         return 0;
>  }
>
> -static ssize_t
> -cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
> +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)
>  {
> +       int rc = 0;
> +       size_t cur_len;
>         unsigned long nr_pages, num_pages, i;
> -       size_t len, cur_len;
> -       ssize_t total_written = 0;
> -       loff_t offset;
> -       struct cifsFileInfo *open_file;
> -       struct cifs_tcon *tcon;
> -       struct cifs_sb_info *cifs_sb;
> -       struct cifs_writedata *wdata, *tmp;
> -       struct list_head wdata_list;
> -       int rc;
> +       struct cifs_writedata *wdata;
>         pid_t pid;
>
> -       len = iov_iter_count(from);
> -       rc = generic_write_checks(file, poffset, &len, 0);
> -       if (rc)
> -               return rc;
> -
> -       if (!len)
> -               return 0;
> -
> -       iov_iter_truncate(from, len);
> -
> -       INIT_LIST_HEAD(&wdata_list);
> -       cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
> -       open_file = file->private_data;
> -       tcon = tlink_tcon(open_file->tlink);
> -
> -       if (!tcon->ses->server->ops->async_writev)
> -               return -ENOSYS;
> -
> -       offset = *poffset;
> -
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
>                 pid = open_file->pid;
>         else
> @@ -2551,11 +2527,47 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>                         break;
>                 }
>
> -               list_add_tail(&wdata->list, &wdata_list);
> +               list_add_tail(&wdata->list, wdata_list);
>                 offset += cur_len;
>                 len -= cur_len;
>         } while (len > 0);
>
> +       return rc;
> +}
> +
> +static ssize_t
> +cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
> +{
> +       size_t len;
> +       ssize_t total_written = 0;
> +       struct cifsFileInfo *open_file;
> +       struct cifs_tcon *tcon;
> +       struct cifs_sb_info *cifs_sb;
> +       struct cifs_writedata *wdata, *tmp;
> +       struct list_head wdata_list;
> +       int rc;
> +
> +       len = iov_iter_count(from);
> +       rc = generic_write_checks(file, poffset, &len, 0);
> +       if (rc)
> +               return rc;
> +
> +       if (!len)
> +               return 0;
> +
> +       iov_iter_truncate(from, len);
> +
> +       INIT_LIST_HEAD(&wdata_list);
> +       cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
> +       open_file = file->private_data;
> +       tcon = tlink_tcon(open_file->tlink);
> +
> +       if (!tcon->ses->server->ops->async_writev)
> +               return -ENOSYS;
> +
> +       rc = cifs_write_from_iter(*poffset, len, from, open_file, cifs_sb,
> +                                 &wdata_list);
> +
>         /*
>          * If at least one write was successfully sent, then discard any rc
>          * value from the later writes. If the other write succeeds, then
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 09/16] CIFS: Fix wsize usage in iovec write

[Pavel Shilovsky]

If a server change maximum buffer size for write (wsize) requests
on reconnect we can fail on repeating with a big size buffer on
-EAGAIN error in iovec write. Fix this by checking wsize all the
time before repeating request in iovec write.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifssmb.c |  4 ----
 fs/cifs/file.c    | 63 ++++++++++++++++++++++++++++++++++---------------------
 fs/cifs/smb2pdu.c |  4 ----
 3 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index e273ded..784f105 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -196,10 +196,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	if (rc)
 		goto out;
 
-	/*
-	 * FIXME: check if wsize needs updated due to negotiated smb buffer
-	 * 	  size shrinking
-	 */
 	atomic_inc(&tconInfoReconnectCount);
 
 	/* tell server Unix caps we support */
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 6660698..c9c4f5a 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2401,28 +2401,6 @@ cifs_uncached_writev_complete(struct work_struct *work)
 	kref_put(&wdata->refcount, cifs_uncached_writedata_release);
 }
 
-/* attempt to send write to server, retry on any -EAGAIN errors */
-static int
-cifs_uncached_retry_writev(struct cifs_writedata *wdata)
-{
-	int rc;
-	struct TCP_Server_Info *server;
-
-	server = tlink_tcon(wdata->cfile->tlink)->ses->server;
-
-	do {
-		if (wdata->cfile->invalidHandle) {
-			rc = cifs_reopen_file(wdata->cfile, false);
-			if (rc != 0)
-				continue;
-		}
-		rc = server->ops->async_writev(wdata,
-					       cifs_uncached_writedata_release);
-	} while (rc == -EAGAIN);
-
-	return rc;
-}
-
 static int
 wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
 		      size_t *len, unsigned long *num_pages)
@@ -2474,13 +2452,19 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 	size_t cur_len;
 	unsigned long nr_pages, num_pages, i;
 	struct cifs_writedata *wdata;
+	struct iov_iter saved_from;
+	loff_t saved_offset = offset;
 	pid_t pid;
+	struct TCP_Server_Info *server;
 
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
 		pid = open_file->pid;
 	else
 		pid = current->tgid;
 
+	server = tlink_tcon(open_file->tlink)->ses->server;
+	memcpy(&saved_from, from, sizeof(struct iov_iter));
+
 	do {
 		nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
 		wdata = cifs_writedata_alloc(nr_pages,
@@ -2520,10 +2504,20 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 		wdata->bytes = cur_len;
 		wdata->pagesz = PAGE_SIZE;
 		wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE);
-		rc = cifs_uncached_retry_writev(wdata);
+
+		if (!wdata->cfile->invalidHandle ||
+		    !cifs_reopen_file(wdata->cfile, false))
+			rc = server->ops->async_writev(wdata,
+					cifs_uncached_writedata_release);
 		if (rc) {
 			kref_put(&wdata->refcount,
 				 cifs_uncached_writedata_release);
+			if (rc == -EAGAIN) {
+				memcpy(from, &saved_from,
+				       sizeof(struct iov_iter));
+				iov_iter_advance(from, offset - saved_offset);
+				continue;
+			}
 			break;
 		}
 
@@ -2545,6 +2539,7 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 	struct cifs_sb_info *cifs_sb;
 	struct cifs_writedata *wdata, *tmp;
 	struct list_head wdata_list;
+	struct iov_iter saved_from;
 	int rc;
 
 	len = iov_iter_count(from);
@@ -2565,6 +2560,8 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 	if (!tcon->ses->server->ops->async_writev)
 		return -ENOSYS;
 
+	memcpy(&saved_from, from, sizeof(struct iov_iter));
+
 	rc = cifs_write_from_iter(*poffset, len, from, open_file, cifs_sb,
 				  &wdata_list);
 
@@ -2596,7 +2593,25 @@ restart_loop:
 
 			/* resend call if it's a retryable error */
 			if (rc == -EAGAIN) {
-				rc = cifs_uncached_retry_writev(wdata);
+				struct list_head tmp_list;
+				struct iov_iter tmp_from;
+
+				INIT_LIST_HEAD(&tmp_list);
+				list_del_init(&wdata->list);
+
+				memcpy(&tmp_from, &saved_from,
+				       sizeof(struct iov_iter));
+				iov_iter_advance(&tmp_from,
+						 wdata->offset - *poffset);
+
+				rc = cifs_write_from_iter(wdata->offset,
+						wdata->bytes, &tmp_from,
+						open_file, cifs_sb, &tmp_list);
+
+				list_splice(&tmp_list, &wdata_list);
+
+				kref_put(&wdata->refcount,
+					 cifs_uncached_writedata_release);
 				goto restart_loop;
 			}
 		}
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index b0b260d..8f5754a 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -245,10 +245,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 	if (rc)
 		goto out;
 	atomic_inc(&tconInfoReconnectCount);
-	/*
-	 * BB FIXME add code to check if wsize needs update due to negotiated
-	 * smb buffer size shrinking.
-	 */
 out:
 	/*
 	 * Check if handle based operation so we know whether we can continue
-- 
1.8.1.2

Re: [PATCH v3 09/16] CIFS: Fix wsize usage in iovec write

[Shirish Pargaonkar]

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If a server change maximum buffer size for write (wsize) requests
> on reconnect we can fail on repeating with a big size buffer on
> -EAGAIN error in iovec write. Fix this by checking wsize all the
> time before repeating request in iovec write.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifssmb.c |  4 ----
>  fs/cifs/file.c    | 63 ++++++++++++++++++++++++++++++++++---------------------
>  fs/cifs/smb2pdu.c |  4 ----
>  3 files changed, 39 insertions(+), 32 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index e273ded..784f105 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -196,10 +196,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         if (rc)
>                 goto out;
>
> -       /*
> -        * FIXME: check if wsize needs updated due to negotiated smb buffer
> -        *        size shrinking
> -        */
>         atomic_inc(&tconInfoReconnectCount);
>
>         /* tell server Unix caps we support */
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 6660698..c9c4f5a 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2401,28 +2401,6 @@ cifs_uncached_writev_complete(struct work_struct *work)
>         kref_put(&wdata->refcount, cifs_uncached_writedata_release);
>  }
>
> -/* attempt to send write to server, retry on any -EAGAIN errors */
> -static int
> -cifs_uncached_retry_writev(struct cifs_writedata *wdata)
> -{
> -       int rc;
> -       struct TCP_Server_Info *server;
> -
> -       server = tlink_tcon(wdata->cfile->tlink)->ses->server;
> -
> -       do {
> -               if (wdata->cfile->invalidHandle) {
> -                       rc = cifs_reopen_file(wdata->cfile, false);
> -                       if (rc != 0)
> -                               continue;
> -               }
> -               rc = server->ops->async_writev(wdata,
> -                                              cifs_uncached_writedata_release);
> -       } while (rc == -EAGAIN);
> -
> -       return rc;
> -}
> -
>  static int
>  wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
>                       size_t *len, unsigned long *num_pages)
> @@ -2474,13 +2452,19 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>         size_t cur_len;
>         unsigned long nr_pages, num_pages, i;
>         struct cifs_writedata *wdata;
> +       struct iov_iter saved_from;
> +       loff_t saved_offset = offset;
>         pid_t pid;
> +       struct TCP_Server_Info *server;
>
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
>                 pid = open_file->pid;
>         else
>                 pid = current->tgid;
>
> +       server = tlink_tcon(open_file->tlink)->ses->server;
> +       memcpy(&saved_from, from, sizeof(struct iov_iter));
> +
>         do {
>                 nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
>                 wdata = cifs_writedata_alloc(nr_pages,
> @@ -2520,10 +2504,20 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>                 wdata->bytes = cur_len;
>                 wdata->pagesz = PAGE_SIZE;
>                 wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE);
> -               rc = cifs_uncached_retry_writev(wdata);
> +
> +               if (!wdata->cfile->invalidHandle ||
> +                   !cifs_reopen_file(wdata->cfile, false))
> +                       rc = server->ops->async_writev(wdata,
> +                                       cifs_uncached_writedata_release);
>                 if (rc) {
>                         kref_put(&wdata->refcount,
>                                  cifs_uncached_writedata_release);
> +                       if (rc == -EAGAIN) {
> +                               memcpy(from, &saved_from,
> +                                      sizeof(struct iov_iter));
> +                               iov_iter_advance(from, offset - saved_offset);
> +                               continue;
> +                       }
>                         break;
>                 }
>
> @@ -2545,6 +2539,7 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>         struct cifs_sb_info *cifs_sb;
>         struct cifs_writedata *wdata, *tmp;
>         struct list_head wdata_list;
> +       struct iov_iter saved_from;
>         int rc;
>
>         len = iov_iter_count(from);
> @@ -2565,6 +2560,8 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>         if (!tcon->ses->server->ops->async_writev)
>                 return -ENOSYS;
>
> +       memcpy(&saved_from, from, sizeof(struct iov_iter));
> +
>         rc = cifs_write_from_iter(*poffset, len, from, open_file, cifs_sb,
>                                   &wdata_list);
>
> @@ -2596,7 +2593,25 @@ restart_loop:
>
>                         /* resend call if it's a retryable error */
>                         if (rc == -EAGAIN) {
> -                               rc = cifs_uncached_retry_writev(wdata);
> +                               struct list_head tmp_list;
> +                               struct iov_iter tmp_from;
> +
> +                               INIT_LIST_HEAD(&tmp_list);
> +                               list_del_init(&wdata->list);
> +
> +                               memcpy(&tmp_from, &saved_from,
> +                                      sizeof(struct iov_iter));
> +                               iov_iter_advance(&tmp_from,
> +                                                wdata->offset - *poffset);
> +
> +                               rc = cifs_write_from_iter(wdata->offset,
> +                                               wdata->bytes, &tmp_from,
> +                                               open_file, cifs_sb, &tmp_list);
> +
> +                               list_splice(&tmp_list, &wdata_list);

Do we list_splice unconditionally or do we list splice only if e.g.
if (!list_empty(&tmp_list)) i.e. could tmp_list be empty if
cifs_write_from_iter returned with non-zero rc (thus we did not
add a wdata to the list)?


> +
> +                               kref_put(&wdata->refcount,
> +                                        cifs_uncached_writedata_release);
>                                 goto restart_loop;
>                         }
>                 }
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index b0b260d..8f5754a 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -245,10 +245,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
>         if (rc)
>                 goto out;
>         atomic_inc(&tconInfoReconnectCount);
> -       /*
> -        * BB FIXME add code to check if wsize needs update due to negotiated
> -        * smb buffer size shrinking.
> -        */
>  out:
>         /*
>          * Check if handle based operation so we know whether we can continue
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 09/16] CIFS: Fix wsize usage in iovec write

[Pavel Shilovsky]

23.07.2014, в 10:15, Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> написал(а):

> On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky@samba.org> wrote:
>> If a server change maximum buffer size for write (wsize) requests
>> on reconnect we can fail on repeating with a big size buffer on
>> -EAGAIN error in iovec write. Fix this by checking wsize all the
>> time before repeating request in iovec write.
>> 
>> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> ---
>> fs/cifs/cifssmb.c |  4 ----
>> fs/cifs/file.c    | 63 ++++++++++++++++++++++++++++++++++---------------------
>> fs/cifs/smb2pdu.c |  4 ----
>> 3 files changed, 39 insertions(+), 32 deletions(-)
>> 
>> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
>> index e273ded..784f105 100644
>> --- a/fs/cifs/cifssmb.c
>> +++ b/fs/cifs/cifssmb.c
>> @@ -196,10 +196,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>>        if (rc)
>>                goto out;
>> 
>> -       /*
>> -        * FIXME: check if wsize needs updated due to negotiated smb buffer
>> -        *        size shrinking
>> -        */
>>        atomic_inc(&tconInfoReconnectCount);
>> 
>>        /* tell server Unix caps we support */
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index 6660698..c9c4f5a 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -2401,28 +2401,6 @@ cifs_uncached_writev_complete(struct work_struct *work)
>>        kref_put(&wdata->refcount, cifs_uncached_writedata_release);
>> }
>> 
>> -/* attempt to send write to server, retry on any -EAGAIN errors */
>> -static int
>> -cifs_uncached_retry_writev(struct cifs_writedata *wdata)
>> -{
>> -       int rc;
>> -       struct TCP_Server_Info *server;
>> -
>> -       server = tlink_tcon(wdata->cfile->tlink)->ses->server;
>> -
>> -       do {
>> -               if (wdata->cfile->invalidHandle) {
>> -                       rc = cifs_reopen_file(wdata->cfile, false);
>> -                       if (rc != 0)
>> -                               continue;
>> -               }
>> -               rc = server->ops->async_writev(wdata,
>> -                                              cifs_uncached_writedata_release);
>> -       } while (rc == -EAGAIN);
>> -
>> -       return rc;
>> -}
>> -
>> static int
>> wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
>>                      size_t *len, unsigned long *num_pages)
>> @@ -2474,13 +2452,19 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>>        size_t cur_len;
>>        unsigned long nr_pages, num_pages, i;
>>        struct cifs_writedata *wdata;
>> +       struct iov_iter saved_from;
>> +       loff_t saved_offset = offset;
>>        pid_t pid;
>> +       struct TCP_Server_Info *server;
>> 
>>        if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
>>                pid = open_file->pid;
>>        else
>>                pid = current->tgid;
>> 
>> +       server = tlink_tcon(open_file->tlink)->ses->server;
>> +       memcpy(&saved_from, from, sizeof(struct iov_iter));
>> +
>>        do {
>>                nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
>>                wdata = cifs_writedata_alloc(nr_pages,
>> @@ -2520,10 +2504,20 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>>                wdata->bytes = cur_len;
>>                wdata->pagesz = PAGE_SIZE;
>>                wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE);
>> -               rc = cifs_uncached_retry_writev(wdata);
>> +
>> +               if (!wdata->cfile->invalidHandle ||
>> +                   !cifs_reopen_file(wdata->cfile, false))
>> +                       rc = server->ops->async_writev(wdata,
>> +                                       cifs_uncached_writedata_release);
>>                if (rc) {
>>                        kref_put(&wdata->refcount,
>>                                 cifs_uncached_writedata_release);
>> +                       if (rc == -EAGAIN) {
>> +                               memcpy(from, &saved_from,
>> +                                      sizeof(struct iov_iter));
>> +                               iov_iter_advance(from, offset - saved_offset);
>> +                               continue;
>> +                       }
>>                        break;
>>                }
>> 
>> @@ -2545,6 +2539,7 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>>        struct cifs_sb_info *cifs_sb;
>>        struct cifs_writedata *wdata, *tmp;
>>        struct list_head wdata_list;
>> +       struct iov_iter saved_from;
>>        int rc;
>> 
>>        len = iov_iter_count(from);
>> @@ -2565,6 +2560,8 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>>        if (!tcon->ses->server->ops->async_writev)
>>                return -ENOSYS;
>> 
>> +       memcpy(&saved_from, from, sizeof(struct iov_iter));
>> +
>>        rc = cifs_write_from_iter(*poffset, len, from, open_file, cifs_sb,
>>                                  &wdata_list);
>> 
>> @@ -2596,7 +2593,25 @@ restart_loop:
>> 
>>                        /* resend call if it's a retryable error */
>>                        if (rc == -EAGAIN) {
>> -                               rc = cifs_uncached_retry_writev(wdata);
>> +                               struct list_head tmp_list;
>> +                               struct iov_iter tmp_from;
>> +
>> +                               INIT_LIST_HEAD(&tmp_list);
>> +                               list_del_init(&wdata->list);
>> +
>> +                               memcpy(&tmp_from, &saved_from,
>> +                                      sizeof(struct iov_iter));
>> +                               iov_iter_advance(&tmp_from,
>> +                                                wdata->offset - *poffset);
>> +
>> +                               rc = cifs_write_from_iter(wdata->offset,
>> +                                               wdata->bytes, &tmp_from,
>> +                                               open_file, cifs_sb, &tmp_list);
>> +
>> +                               list_splice(&tmp_list, &wdata_list);
> 
> Do we list_splice unconditionally or do we list splice only if e.g.
> if (!list_empty(&tmp_list)) i.e. could tmp_list be empty if
> cifs_write_from_iter returned with non-zero rc (thus we did not
> add a wdata to the list)?


Yes. The check is done inside the list_splice().

--
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 09/16] CIFS: Fix wsize usage in iovec write

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If a server change maximum buffer size for write (wsize) requests
> on reconnect we can fail on repeating with a big size buffer on
> -EAGAIN error in iovec write. Fix this by checking wsize all the
> time before repeating request in iovec write.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifssmb.c |  4 ----
>  fs/cifs/file.c    | 63 ++++++++++++++++++++++++++++++++++---------------------
>  fs/cifs/smb2pdu.c |  4 ----
>  3 files changed, 39 insertions(+), 32 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index e273ded..784f105 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -196,10 +196,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         if (rc)
>                 goto out;
>
> -       /*
> -        * FIXME: check if wsize needs updated due to negotiated smb buffer
> -        *        size shrinking
> -        */
>         atomic_inc(&tconInfoReconnectCount);
>
>         /* tell server Unix caps we support */
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 6660698..c9c4f5a 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2401,28 +2401,6 @@ cifs_uncached_writev_complete(struct work_struct *work)
>         kref_put(&wdata->refcount, cifs_uncached_writedata_release);
>  }
>
> -/* attempt to send write to server, retry on any -EAGAIN errors */
> -static int
> -cifs_uncached_retry_writev(struct cifs_writedata *wdata)
> -{
> -       int rc;
> -       struct TCP_Server_Info *server;
> -
> -       server = tlink_tcon(wdata->cfile->tlink)->ses->server;
> -
> -       do {
> -               if (wdata->cfile->invalidHandle) {
> -                       rc = cifs_reopen_file(wdata->cfile, false);
> -                       if (rc != 0)
> -                               continue;
> -               }
> -               rc = server->ops->async_writev(wdata,
> -                                              cifs_uncached_writedata_release);
> -       } while (rc == -EAGAIN);
> -
> -       return rc;
> -}
> -
>  static int
>  wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from,
>                       size_t *len, unsigned long *num_pages)
> @@ -2474,13 +2452,19 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>         size_t cur_len;
>         unsigned long nr_pages, num_pages, i;
>         struct cifs_writedata *wdata;
> +       struct iov_iter saved_from;
> +       loff_t saved_offset = offset;
>         pid_t pid;
> +       struct TCP_Server_Info *server;
>
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
>                 pid = open_file->pid;
>         else
>                 pid = current->tgid;
>
> +       server = tlink_tcon(open_file->tlink)->ses->server;
> +       memcpy(&saved_from, from, sizeof(struct iov_iter));
> +
>         do {
>                 nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
>                 wdata = cifs_writedata_alloc(nr_pages,
> @@ -2520,10 +2504,20 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>                 wdata->bytes = cur_len;
>                 wdata->pagesz = PAGE_SIZE;
>                 wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE);
> -               rc = cifs_uncached_retry_writev(wdata);
> +
> +               if (!wdata->cfile->invalidHandle ||
> +                   !cifs_reopen_file(wdata->cfile, false))
> +                       rc = server->ops->async_writev(wdata,
> +                                       cifs_uncached_writedata_release);
>                 if (rc) {
>                         kref_put(&wdata->refcount,
>                                  cifs_uncached_writedata_release);
> +                       if (rc == -EAGAIN) {
> +                               memcpy(from, &saved_from,
> +                                      sizeof(struct iov_iter));
> +                               iov_iter_advance(from, offset - saved_offset);
> +                               continue;
> +                       }
>                         break;
>                 }
>
> @@ -2545,6 +2539,7 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>         struct cifs_sb_info *cifs_sb;
>         struct cifs_writedata *wdata, *tmp;
>         struct list_head wdata_list;
> +       struct iov_iter saved_from;
>         int rc;
>
>         len = iov_iter_count(from);
> @@ -2565,6 +2560,8 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
>         if (!tcon->ses->server->ops->async_writev)
>                 return -ENOSYS;
>
> +       memcpy(&saved_from, from, sizeof(struct iov_iter));
> +
>         rc = cifs_write_from_iter(*poffset, len, from, open_file, cifs_sb,
>                                   &wdata_list);
>
> @@ -2596,7 +2593,25 @@ restart_loop:
>
>                         /* resend call if it's a retryable error */
>                         if (rc == -EAGAIN) {
> -                               rc = cifs_uncached_retry_writev(wdata);
> +                               struct list_head tmp_list;
> +                               struct iov_iter tmp_from;
> +
> +                               INIT_LIST_HEAD(&tmp_list);
> +                               list_del_init(&wdata->list);
> +
> +                               memcpy(&tmp_from, &saved_from,
> +                                      sizeof(struct iov_iter));
> +                               iov_iter_advance(&tmp_from,
> +                                                wdata->offset - *poffset);
> +
> +                               rc = cifs_write_from_iter(wdata->offset,
> +                                               wdata->bytes, &tmp_from,
> +                                               open_file, cifs_sb, &tmp_list);
> +
> +                               list_splice(&tmp_list, &wdata_list);
> +
> +                               kref_put(&wdata->refcount,
> +                                        cifs_uncached_writedata_release);
>                                 goto restart_loop;
>                         }
>                 }
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index b0b260d..8f5754a 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -245,10 +245,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
>         if (rc)
>                 goto out;
>         atomic_inc(&tconInfoReconnectCount);
> -       /*
> -        * BB FIXME add code to check if wsize needs update due to negotiated
> -        * smb buffer size shrinking.
> -        */
>  out:
>         /*
>          * Check if handle based operation so we know whether we can continue
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Pavel Shilovsky]

If we negotiate SMB 2.1 and higher version of the protocol and
a server supports large write buffer size, we need to consume 1
credit per 65536 bytes. So, we need to know how many credits
we have and obtain the required number of them before constructing
a writedata structure in writepages and iovec write.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h      | 15 ++++++++++++++
 fs/cifs/cifsproto.h     |  3 +++
 fs/cifs/file.c          | 36 ++++++++++++++++++++++++++++------
 fs/cifs/smb1ops.c       |  1 +
 fs/cifs/smb2ops.c       | 52 +++++++++++++++++++++++++++++++++++++++++++++++--
 fs/cifs/smb2pdu.c       | 29 +++++++++++++++++++++++----
 fs/cifs/smb2transport.c |  5 +++++
 fs/cifs/transport.c     | 25 +++++++++++++++++-------
 8 files changed, 147 insertions(+), 19 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8c53f20..54ca2b9 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -406,6 +406,9 @@ struct smb_version_operations {
 			int);
 	/* writepages retry size */
 	unsigned int (*wp_retry_size)(struct inode *);
+	/* get mtu credits */
+	int (*wait_mtu_credits)(struct TCP_Server_Info *, unsigned int,
+				unsigned int *, unsigned int *);
 };
 
 struct smb_version_values {
@@ -642,6 +645,16 @@ add_credits(struct TCP_Server_Info *server, const unsigned int add,
 }
 
 static inline void
+add_credits_and_wake_if(struct TCP_Server_Info *server, const unsigned int add,
+			const int optype)
+{
+	if (add) {
+		server->ops->add_credits(server, add, optype);
+		wake_up(&server->request_q);
+	}
+}
+
+static inline void
 set_credits(struct TCP_Server_Info *server, const int val)
 {
 	server->ops->set_credits(server, val);
@@ -1075,6 +1088,7 @@ struct cifs_writedata {
 	int				result;
 	unsigned int			pagesz;
 	unsigned int			tailsz;
+	unsigned int			credits;
 	unsigned int			nr_pages;
 	struct page			*pages[];
 };
@@ -1400,6 +1414,7 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
 #define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
 #define   CIFS_NEG_OP      0x0200    /* negotiate request */
 #define   CIFS_OP_MASK     0x0380    /* mask request type */
+#define   CIFS_HAS_CREDITS 0x0400    /* already has credits */
 
 /* Security Flags: indicate type of session setup needed */
 #define   CIFSSEC_MAY_SIGN	0x00001
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index ca7980a..009e5cb 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -89,6 +89,9 @@ extern struct mid_q_entry *cifs_setup_async_request(struct TCP_Server_Info *,
 						struct smb_rqst *);
 extern int cifs_check_receive(struct mid_q_entry *mid,
 			struct TCP_Server_Info *server, bool log_error);
+extern int cifs_wait_mtu_credits(struct TCP_Server_Info *server,
+				 unsigned int size, unsigned int *num,
+				 unsigned int *credits);
 extern int SendReceive2(const unsigned int /* xid */ , struct cifs_ses *,
 			struct kvec *, int /* nvec to send */,
 			int * /* type of buf returned */ , const int flags);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c9c4f5a..c79bdf3 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1670,8 +1670,8 @@ cifs_write(struct cifsFileInfo *open_file, __u32 pid, const char *write_data,
 					break;
 			}
 
-			len = min((size_t)cifs_sb->wsize,
-				  write_size - total_written);
+			len = min(server->ops->wp_retry_size(dentry->d_inode),
+				  (unsigned int)write_size - total_written);
 			/* iov[0] is reserved for smb header */
 			iov[1].iov_base = (char *)write_data + total_written;
 			iov[1].iov_len = len;
@@ -2031,6 +2031,7 @@ static int cifs_writepages(struct address_space *mapping,
 			   struct writeback_control *wbc)
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(mapping->host->i_sb);
+	struct TCP_Server_Info *server;
 	bool done = false, scanned = false, range_whole = false;
 	pgoff_t end, index;
 	struct cifs_writedata *wdata;
@@ -2053,23 +2054,30 @@ static int cifs_writepages(struct address_space *mapping,
 			range_whole = true;
 		scanned = true;
 	}
+	server = cifs_sb_master_tcon(cifs_sb)->ses->server;
 retry:
 	while (!done && index <= end) {
-		unsigned int i, nr_pages, found_pages;
+		unsigned int i, nr_pages, found_pages, wsize, credits;
 		pgoff_t next = 0, tofind, saved_index = index;
 
-		tofind = min((cifs_sb->wsize / PAGE_CACHE_SIZE) - 1,
-				end - index) + 1;
+		rc = server->ops->wait_mtu_credits(server, cifs_sb->wsize,
+						   &wsize, &credits);
+		if (rc)
+			break;
+
+		tofind = min((wsize / PAGE_CACHE_SIZE) - 1, end - index) + 1;
 
 		wdata = wdata_alloc_and_fillpages(tofind, mapping, end, &index,
 						  &found_pages);
 		if (!wdata) {
 			rc = -ENOMEM;
+			add_credits_and_wake_if(server, credits, 0);
 			break;
 		}
 
 		if (found_pages == 0) {
 			kref_put(&wdata->refcount, cifs_writedata_release);
+			add_credits_and_wake_if(server, credits, 0);
 			break;
 		}
 
@@ -2079,13 +2087,17 @@ retry:
 		/* nothing to write? */
 		if (nr_pages == 0) {
 			kref_put(&wdata->refcount, cifs_writedata_release);
+			add_credits_and_wake_if(server, credits, 0);
 			continue;
 		}
 
+		wdata->credits = credits;
+
 		rc = wdata_send_pages(wdata, nr_pages, mapping, wbc);
 
 		/* send failure -- clean up the mess */
 		if (rc != 0) {
+			add_credits_and_wake_if(server, wdata->credits, 0);
 			for (i = 0; i < nr_pages; ++i) {
 				if (rc == -EAGAIN)
 					redirty_page_for_writepage(wbc,
@@ -2466,17 +2478,26 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 	memcpy(&saved_from, from, sizeof(struct iov_iter));
 
 	do {
-		nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
+		unsigned int wsize, credits;
+
+		rc = server->ops->wait_mtu_credits(server, cifs_sb->wsize,
+						   &wsize, &credits);
+		if (rc)
+			break;
+
+		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;
 		}
 
 		rc = cifs_write_allocate_pages(wdata->pages, nr_pages);
 		if (rc) {
 			kfree(wdata);
+			add_credits_and_wake_if(server, credits, 0);
 			break;
 		}
 
@@ -2486,6 +2507,7 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 			for (i = 0; i < nr_pages; i++)
 				put_page(wdata->pages[i]);
 			kfree(wdata);
+			add_credits_and_wake_if(server, credits, 0);
 			break;
 		}
 
@@ -2504,12 +2526,14 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 		wdata->bytes = cur_len;
 		wdata->pagesz = PAGE_SIZE;
 		wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE);
+		wdata->credits = credits;
 
 		if (!wdata->cfile->invalidHandle ||
 		    !cifs_reopen_file(wdata->cfile, false))
 			rc = server->ops->async_writev(wdata,
 					cifs_uncached_writedata_release);
 		if (rc) {
+			add_credits_and_wake_if(server, wdata->credits, 0);
 			kref_put(&wdata->refcount,
 				 cifs_uncached_writedata_release);
 			if (rc == -EAGAIN) {
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 8a96342..5e8c22d 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -1025,6 +1025,7 @@ struct smb_version_operations smb1_operations = {
 	.set_credits = cifs_set_credits,
 	.get_credits_field = cifs_get_credits_field,
 	.get_credits = cifs_get_credits,
+	.wait_mtu_credits = cifs_wait_mtu_credits,
 	.get_next_mid = cifs_get_next_mid,
 	.read_data_offset = cifs_read_data_offset,
 	.read_data_length = cifs_read_data_length,
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index e35ce5b..fecc2de 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -112,6 +112,53 @@ smb2_get_credits(struct mid_q_entry *mid)
 	return le16_to_cpu(((struct smb2_hdr *)mid->resp_buf)->CreditRequest);
 }
 
+static int
+smb2_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
+		      unsigned int *num, unsigned int *credits)
+{
+	int rc = 0;
+	unsigned int scredits;
+
+	spin_lock(&server->req_lock);
+	while (1) {
+		if (server->credits <= 0) {
+			spin_unlock(&server->req_lock);
+			cifs_num_waiters_inc(server);
+			rc = wait_event_killable(server->request_q,
+					has_credits(server, &server->credits));
+			cifs_num_waiters_dec(server);
+			if (rc)
+				return rc;
+			spin_lock(&server->req_lock);
+		} else {
+			if (server->tcpStatus == CifsExiting) {
+				spin_unlock(&server->req_lock);
+				return -ENOENT;
+			}
+
+			scredits = server->credits;
+			/* can deadlock with reopen */
+			if (scredits == 1) {
+				*num = SMB2_MAX_BUFFER_SIZE;
+				*credits = 0;
+				break;
+			}
+
+			/* leave one credit for a possible reopen */
+			scredits--;
+			*num = min_t(unsigned int, size,
+				     scredits * SMB2_MAX_BUFFER_SIZE);
+
+			*credits = DIV_ROUND_UP(*num, SMB2_MAX_BUFFER_SIZE);
+			server->credits -= *credits;
+			server->in_flight++;
+			break;
+		}
+	}
+	spin_unlock(&server->req_lock);
+	return rc;
+}
+
 static __u64
 smb2_get_next_mid(struct TCP_Server_Info *server)
 {
@@ -182,8 +229,6 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
 	/* start with specified wsize, or default */
 	wsize = volume_info->wsize ? volume_info->wsize : CIFS_DEFAULT_IOSIZE;
 	wsize = min_t(unsigned int, wsize, server->max_write);
-	/* set it to the maximum buffer size value we can send with 1 credit */
-	wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
 
 	return wsize;
 }
@@ -1120,6 +1165,7 @@ struct smb_version_operations smb20_operations = {
 	.set_credits = smb2_set_credits,
 	.get_credits_field = smb2_get_credits_field,
 	.get_credits = smb2_get_credits,
+	.wait_mtu_credits = cifs_wait_mtu_credits,
 	.get_next_mid = smb2_get_next_mid,
 	.read_data_offset = smb2_read_data_offset,
 	.read_data_length = smb2_read_data_length,
@@ -1196,6 +1242,7 @@ struct smb_version_operations smb21_operations = {
 	.set_credits = smb2_set_credits,
 	.get_credits_field = smb2_get_credits_field,
 	.get_credits = smb2_get_credits,
+	.wait_mtu_credits = smb2_wait_mtu_credits,
 	.get_next_mid = smb2_get_next_mid,
 	.read_data_offset = smb2_read_data_offset,
 	.read_data_length = smb2_read_data_length,
@@ -1272,6 +1319,7 @@ struct smb_version_operations smb30_operations = {
 	.set_credits = smb2_set_credits,
 	.get_credits_field = smb2_get_credits_field,
 	.get_credits = smb2_get_credits,
+	.wait_mtu_credits = smb2_wait_mtu_credits,
 	.get_next_mid = smb2_get_next_mid,
 	.read_data_offset = smb2_read_data_offset,
 	.read_data_length = smb2_read_data_length,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 8f5754a..a1d89b7 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1902,15 +1902,25 @@ int
 smb2_async_writev(struct cifs_writedata *wdata,
 		  void (*release)(struct kref *kref))
 {
-	int rc = -EACCES;
+	int rc = -EACCES, flags = 0;
 	struct smb2_write_req *req = NULL;
 	struct cifs_tcon *tcon = tlink_tcon(wdata->cfile->tlink);
+	struct TCP_Server_Info *server = tcon->ses->server;
 	struct kvec iov;
 	struct smb_rqst rqst;
 
 	rc = small_smb2_init(SMB2_WRITE, tcon, (void **) &req);
-	if (rc)
+	if (rc) {
+		if (rc == -EAGAIN && wdata->credits) {
+			/* credits was reseted by reconnect */
+			wdata->credits = 0;
+			/* reduce in_flight value since we won't send the req */
+			spin_lock(&server->req_lock);
+			server->in_flight--;
+			spin_unlock(&server->req_lock);
+		}
 		goto async_writev_out;
+	}
 
 	req->hdr.ProcessId = cpu_to_le32(wdata->cfile->pid);
 
@@ -1943,9 +1953,20 @@ smb2_async_writev(struct cifs_writedata *wdata,
 
 	inc_rfc1001_len(&req->hdr, wdata->bytes - 1 /* Buffer */);
 
+	if (wdata->credits) {
+		req->hdr.CreditCharge = cpu_to_le16(DIV_ROUND_UP(wdata->bytes,
+						    SMB2_MAX_BUFFER_SIZE));
+		spin_lock(&server->req_lock);
+		server->credits += wdata->credits -
+					le16_to_cpu(req->hdr.CreditCharge);
+		spin_unlock(&server->req_lock);
+		wake_up(&server->request_q);
+		flags = CIFS_HAS_CREDITS;
+	}
+
 	kref_get(&wdata->refcount);
-	rc = cifs_call_async(tcon->ses->server, &rqst, NULL,
-				smb2_writev_callback, wdata, 0);
+	rc = cifs_call_async(server, &rqst, NULL, smb2_writev_callback, wdata,
+			     flags);
 
 	if (rc) {
 		kref_put(&wdata->refcount, release);
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index 59c748c..5111e72 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -466,7 +466,12 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
 static inline void
 smb2_seq_num_into_buf(struct TCP_Server_Info *server, struct smb2_hdr *hdr)
 {
+	unsigned int i, num = le16_to_cpu(hdr->CreditCharge);
+
 	hdr->MessageId = get_next_mid64(server);
+	/* skip message numbers according to CreditCharge field */
+	for (i = 1; i < num; i++)
+		get_next_mid(server);
 }
 
 static struct mid_q_entry *
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 18cd565..9d087f4 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -448,6 +448,15 @@ wait_for_free_request(struct TCP_Server_Info *server, const int timeout,
 	return wait_for_free_credits(server, timeout, val);
 }
 
+int
+cifs_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
+		      unsigned int *num, unsigned int *credits)
+{
+	*num = size;
+	*credits = 0;
+	return 0;
+}
+
 static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
 			struct mid_q_entry **ppmidQ)
 {
@@ -531,20 +540,23 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
 {
 	int rc, timeout, optype;
 	struct mid_q_entry *mid;
+	unsigned int credits = 0;
 
 	timeout = flags & CIFS_TIMEOUT_MASK;
 	optype = flags & CIFS_OP_MASK;
 
-	rc = wait_for_free_request(server, timeout, optype);
-	if (rc)
-		return rc;
+	if ((flags & CIFS_HAS_CREDITS) == 0) {
+		rc = wait_for_free_request(server, timeout, optype);
+		if (rc)
+			return rc;
+		credits = 1;
+	}
 
 	mutex_lock(&server->srv_mutex);
 	mid = server->ops->setup_async_request(server, rqst);
 	if (IS_ERR(mid)) {
 		mutex_unlock(&server->srv_mutex);
-		add_credits(server, 1, optype);
-		wake_up(&server->request_q);
+		add_credits_and_wake_if(server, credits, optype);
 		return PTR_ERR(mid);
 	}
 
@@ -572,8 +584,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
 		return 0;
 
 	cifs_delete_mid(mid);
-	add_credits(server, 1, optype);
-	wake_up(&server->request_q);
+	add_credits_and_wake_if(server, credits, optype);
 	return rc;
 }
 
-- 
1.8.1.2

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Shirish Pargaonkar]

Looks correct.

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>


Only  comment would be, wish there was a mnemonic/define for
a regular op  when calling add_credits_and_wake_if() for optype
such as CIFS_ECHO_OP or CIFS_OPBREAK_OP instead of 0.

Oh and super nitpick...  s/reseted/reset/.


On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If we negotiate SMB 2.1 and higher version of the protocol and
> a server supports large write buffer size, we need to consume 1
> credit per 65536 bytes. So, we need to know how many credits
> we have and obtain the required number of them before constructing
> a writedata structure in writepages and iovec write.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h      | 15 ++++++++++++++
>  fs/cifs/cifsproto.h     |  3 +++
>  fs/cifs/file.c          | 36 ++++++++++++++++++++++++++++------
>  fs/cifs/smb1ops.c       |  1 +
>  fs/cifs/smb2ops.c       | 52 +++++++++++++++++++++++++++++++++++++++++++++++--
>  fs/cifs/smb2pdu.c       | 29 +++++++++++++++++++++++----
>  fs/cifs/smb2transport.c |  5 +++++
>  fs/cifs/transport.c     | 25 +++++++++++++++++-------
>  8 files changed, 147 insertions(+), 19 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 8c53f20..54ca2b9 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -406,6 +406,9 @@ struct smb_version_operations {
>                         int);
>         /* writepages retry size */
>         unsigned int (*wp_retry_size)(struct inode *);
> +       /* get mtu credits */
> +       int (*wait_mtu_credits)(struct TCP_Server_Info *, unsigned int,
> +                               unsigned int *, unsigned int *);
>  };
>
>  struct smb_version_values {
> @@ -642,6 +645,16 @@ add_credits(struct TCP_Server_Info *server, const unsigned int add,
>  }
>
>  static inline void
> +add_credits_and_wake_if(struct TCP_Server_Info *server, const unsigned int add,
> +                       const int optype)
> +{
> +       if (add) {
> +               server->ops->add_credits(server, add, optype);
> +               wake_up(&server->request_q);
> +       }
> +}
> +
> +static inline void
>  set_credits(struct TCP_Server_Info *server, const int val)
>  {
>         server->ops->set_credits(server, val);
> @@ -1075,6 +1088,7 @@ struct cifs_writedata {
>         int                             result;
>         unsigned int                    pagesz;
>         unsigned int                    tailsz;
> +       unsigned int                    credits;
>         unsigned int                    nr_pages;
>         struct page                     *pages[];
>  };
> @@ -1400,6 +1414,7 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
>  #define   CIFS_OBREAK_OP   0x0100    /* oplock break request */
>  #define   CIFS_NEG_OP      0x0200    /* negotiate request */
>  #define   CIFS_OP_MASK     0x0380    /* mask request type */
> +#define   CIFS_HAS_CREDITS 0x0400    /* already has credits */
>
>  /* Security Flags: indicate type of session setup needed */
>  #define   CIFSSEC_MAY_SIGN     0x00001
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index ca7980a..009e5cb 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -89,6 +89,9 @@ extern struct mid_q_entry *cifs_setup_async_request(struct TCP_Server_Info *,
>                                                 struct smb_rqst *);
>  extern int cifs_check_receive(struct mid_q_entry *mid,
>                         struct TCP_Server_Info *server, bool log_error);
> +extern int cifs_wait_mtu_credits(struct TCP_Server_Info *server,
> +                                unsigned int size, unsigned int *num,
> +                                unsigned int *credits);
>  extern int SendReceive2(const unsigned int /* xid */ , struct cifs_ses *,
>                         struct kvec *, int /* nvec to send */,
>                         int * /* type of buf returned */ , const int flags);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index c9c4f5a..c79bdf3 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1670,8 +1670,8 @@ cifs_write(struct cifsFileInfo *open_file, __u32 pid, const char *write_data,
>                                         break;
>                         }
>
> -                       len = min((size_t)cifs_sb->wsize,
> -                                 write_size - total_written);
> +                       len = min(server->ops->wp_retry_size(dentry->d_inode),
> +                                 (unsigned int)write_size - total_written);
>                         /* iov[0] is reserved for smb header */
>                         iov[1].iov_base = (char *)write_data + total_written;
>                         iov[1].iov_len = len;
> @@ -2031,6 +2031,7 @@ static int cifs_writepages(struct address_space *mapping,
>                            struct writeback_control *wbc)
>  {
>         struct cifs_sb_info *cifs_sb = CIFS_SB(mapping->host->i_sb);
> +       struct TCP_Server_Info *server;
>         bool done = false, scanned = false, range_whole = false;
>         pgoff_t end, index;
>         struct cifs_writedata *wdata;
> @@ -2053,23 +2054,30 @@ static int cifs_writepages(struct address_space *mapping,
>                         range_whole = true;
>                 scanned = true;
>         }
> +       server = cifs_sb_master_tcon(cifs_sb)->ses->server;
>  retry:
>         while (!done && index <= end) {
> -               unsigned int i, nr_pages, found_pages;
> +               unsigned int i, nr_pages, found_pages, wsize, credits;
>                 pgoff_t next = 0, tofind, saved_index = index;
>
> -               tofind = min((cifs_sb->wsize / PAGE_CACHE_SIZE) - 1,
> -                               end - index) + 1;
> +               rc = server->ops->wait_mtu_credits(server, cifs_sb->wsize,
> +                                                  &wsize, &credits);
> +               if (rc)
> +                       break;
> +
> +               tofind = min((wsize / PAGE_CACHE_SIZE) - 1, end - index) + 1;
>
>                 wdata = wdata_alloc_and_fillpages(tofind, mapping, end, &index,
>                                                   &found_pages);
>                 if (!wdata) {
>                         rc = -ENOMEM;
> +                       add_credits_and_wake_if(server, credits, 0);
>                         break;
>                 }
>
>                 if (found_pages == 0) {
>                         kref_put(&wdata->refcount, cifs_writedata_release);
> +                       add_credits_and_wake_if(server, credits, 0);
>                         break;
>                 }
>
> @@ -2079,13 +2087,17 @@ retry:
>                 /* nothing to write? */
>                 if (nr_pages == 0) {
>                         kref_put(&wdata->refcount, cifs_writedata_release);
> +                       add_credits_and_wake_if(server, credits, 0);
>                         continue;
>                 }
>
> +               wdata->credits = credits;
> +
>                 rc = wdata_send_pages(wdata, nr_pages, mapping, wbc);
>
>                 /* send failure -- clean up the mess */
>                 if (rc != 0) {
> +                       add_credits_and_wake_if(server, wdata->credits, 0);
>                         for (i = 0; i < nr_pages; ++i) {
>                                 if (rc == -EAGAIN)
>                                         redirty_page_for_writepage(wbc,
> @@ -2466,17 +2478,26 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>         memcpy(&saved_from, from, sizeof(struct iov_iter));
>
>         do {
> -               nr_pages = get_numpages(cifs_sb->wsize, len, &cur_len);
> +               unsigned int wsize, credits;
> +
> +               rc = server->ops->wait_mtu_credits(server, cifs_sb->wsize,
> +                                                  &wsize, &credits);
> +               if (rc)
> +                       break;
> +
> +               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;
>                 }
>
>                 rc = cifs_write_allocate_pages(wdata->pages, nr_pages);
>                 if (rc) {
>                         kfree(wdata);
> +                       add_credits_and_wake_if(server, credits, 0);
>                         break;
>                 }
>
> @@ -2486,6 +2507,7 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>                         for (i = 0; i < nr_pages; i++)
>                                 put_page(wdata->pages[i]);
>                         kfree(wdata);
> +                       add_credits_and_wake_if(server, credits, 0);
>                         break;
>                 }
>
> @@ -2504,12 +2526,14 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
>                 wdata->bytes = cur_len;
>                 wdata->pagesz = PAGE_SIZE;
>                 wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE);
> +               wdata->credits = credits;
>
>                 if (!wdata->cfile->invalidHandle ||
>                     !cifs_reopen_file(wdata->cfile, false))
>                         rc = server->ops->async_writev(wdata,
>                                         cifs_uncached_writedata_release);
>                 if (rc) {
> +                       add_credits_and_wake_if(server, wdata->credits, 0);
>                         kref_put(&wdata->refcount,
>                                  cifs_uncached_writedata_release);
>                         if (rc == -EAGAIN) {
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 8a96342..5e8c22d 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -1025,6 +1025,7 @@ struct smb_version_operations smb1_operations = {
>         .set_credits = cifs_set_credits,
>         .get_credits_field = cifs_get_credits_field,
>         .get_credits = cifs_get_credits,
> +       .wait_mtu_credits = cifs_wait_mtu_credits,
>         .get_next_mid = cifs_get_next_mid,
>         .read_data_offset = cifs_read_data_offset,
>         .read_data_length = cifs_read_data_length,
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index e35ce5b..fecc2de 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -112,6 +112,53 @@ smb2_get_credits(struct mid_q_entry *mid)
>         return le16_to_cpu(((struct smb2_hdr *)mid->resp_buf)->CreditRequest);
>  }
>
> +static int
> +smb2_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
> +                     unsigned int *num, unsigned int *credits)
> +{
> +       int rc = 0;
> +       unsigned int scredits;
> +
> +       spin_lock(&server->req_lock);
> +       while (1) {
> +               if (server->credits <= 0) {
> +                       spin_unlock(&server->req_lock);
> +                       cifs_num_waiters_inc(server);
> +                       rc = wait_event_killable(server->request_q,
> +                                       has_credits(server, &server->credits));
> +                       cifs_num_waiters_dec(server);
> +                       if (rc)
> +                               return rc;
> +                       spin_lock(&server->req_lock);
> +               } else {
> +                       if (server->tcpStatus == CifsExiting) {
> +                               spin_unlock(&server->req_lock);
> +                               return -ENOENT;
> +                       }
> +
> +                       scredits = server->credits;
> +                       /* can deadlock with reopen */
> +                       if (scredits == 1) {
> +                               *num = SMB2_MAX_BUFFER_SIZE;
> +                               *credits = 0;
> +                               break;
> +                       }
> +
> +                       /* leave one credit for a possible reopen */
> +                       scredits--;
> +                       *num = min_t(unsigned int, size,
> +                                    scredits * SMB2_MAX_BUFFER_SIZE);
> +
> +                       *credits = DIV_ROUND_UP(*num, SMB2_MAX_BUFFER_SIZE);
> +                       server->credits -= *credits;
> +                       server->in_flight++;
> +                       break;
> +               }
> +       }
> +       spin_unlock(&server->req_lock);
> +       return rc;
> +}
> +
>  static __u64
>  smb2_get_next_mid(struct TCP_Server_Info *server)
>  {
> @@ -182,8 +229,6 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
>         /* start with specified wsize, or default */
>         wsize = volume_info->wsize ? volume_info->wsize : CIFS_DEFAULT_IOSIZE;
>         wsize = min_t(unsigned int, wsize, server->max_write);
> -       /* set it to the maximum buffer size value we can send with 1 credit */
> -       wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
>
>         return wsize;
>  }
> @@ -1120,6 +1165,7 @@ struct smb_version_operations smb20_operations = {
>         .set_credits = smb2_set_credits,
>         .get_credits_field = smb2_get_credits_field,
>         .get_credits = smb2_get_credits,
> +       .wait_mtu_credits = cifs_wait_mtu_credits,
>         .get_next_mid = smb2_get_next_mid,
>         .read_data_offset = smb2_read_data_offset,
>         .read_data_length = smb2_read_data_length,
> @@ -1196,6 +1242,7 @@ struct smb_version_operations smb21_operations = {
>         .set_credits = smb2_set_credits,
>         .get_credits_field = smb2_get_credits_field,
>         .get_credits = smb2_get_credits,
> +       .wait_mtu_credits = smb2_wait_mtu_credits,
>         .get_next_mid = smb2_get_next_mid,
>         .read_data_offset = smb2_read_data_offset,
>         .read_data_length = smb2_read_data_length,
> @@ -1272,6 +1319,7 @@ struct smb_version_operations smb30_operations = {
>         .set_credits = smb2_set_credits,
>         .get_credits_field = smb2_get_credits_field,
>         .get_credits = smb2_get_credits,
> +       .wait_mtu_credits = smb2_wait_mtu_credits,
>         .get_next_mid = smb2_get_next_mid,
>         .read_data_offset = smb2_read_data_offset,
>         .read_data_length = smb2_read_data_length,
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 8f5754a..a1d89b7 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1902,15 +1902,25 @@ int
>  smb2_async_writev(struct cifs_writedata *wdata,
>                   void (*release)(struct kref *kref))
>  {
> -       int rc = -EACCES;
> +       int rc = -EACCES, flags = 0;
>         struct smb2_write_req *req = NULL;
>         struct cifs_tcon *tcon = tlink_tcon(wdata->cfile->tlink);
> +       struct TCP_Server_Info *server = tcon->ses->server;
>         struct kvec iov;
>         struct smb_rqst rqst;
>
>         rc = small_smb2_init(SMB2_WRITE, tcon, (void **) &req);
> -       if (rc)
> +       if (rc) {
> +               if (rc == -EAGAIN && wdata->credits) {
> +                       /* credits was reseted by reconnect */
> +                       wdata->credits = 0;
> +                       /* reduce in_flight value since we won't send the req */
> +                       spin_lock(&server->req_lock);
> +                       server->in_flight--;
> +                       spin_unlock(&server->req_lock);
> +               }
>                 goto async_writev_out;
> +       }
>
>         req->hdr.ProcessId = cpu_to_le32(wdata->cfile->pid);
>
> @@ -1943,9 +1953,20 @@ smb2_async_writev(struct cifs_writedata *wdata,
>
>         inc_rfc1001_len(&req->hdr, wdata->bytes - 1 /* Buffer */);
>
> +       if (wdata->credits) {
> +               req->hdr.CreditCharge = cpu_to_le16(DIV_ROUND_UP(wdata->bytes,
> +                                                   SMB2_MAX_BUFFER_SIZE));
> +               spin_lock(&server->req_lock);
> +               server->credits += wdata->credits -
> +                                       le16_to_cpu(req->hdr.CreditCharge);
> +               spin_unlock(&server->req_lock);
> +               wake_up(&server->request_q);
> +               flags = CIFS_HAS_CREDITS;
> +       }
> +
>         kref_get(&wdata->refcount);
> -       rc = cifs_call_async(tcon->ses->server, &rqst, NULL,
> -                               smb2_writev_callback, wdata, 0);
> +       rc = cifs_call_async(server, &rqst, NULL, smb2_writev_callback, wdata,
> +                            flags);
>
>         if (rc) {
>                 kref_put(&wdata->refcount, release);
> diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
> index 59c748c..5111e72 100644
> --- a/fs/cifs/smb2transport.c
> +++ b/fs/cifs/smb2transport.c
> @@ -466,7 +466,12 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
>  static inline void
>  smb2_seq_num_into_buf(struct TCP_Server_Info *server, struct smb2_hdr *hdr)
>  {
> +       unsigned int i, num = le16_to_cpu(hdr->CreditCharge);
> +
>         hdr->MessageId = get_next_mid64(server);
> +       /* skip message numbers according to CreditCharge field */
> +       for (i = 1; i < num; i++)
> +               get_next_mid(server);
>  }
>
>  static struct mid_q_entry *
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 18cd565..9d087f4 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -448,6 +448,15 @@ wait_for_free_request(struct TCP_Server_Info *server, const int timeout,
>         return wait_for_free_credits(server, timeout, val);
>  }
>
> +int
> +cifs_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
> +                     unsigned int *num, unsigned int *credits)
> +{
> +       *num = size;
> +       *credits = 0;
> +       return 0;
> +}
> +
>  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
>                         struct mid_q_entry **ppmidQ)
>  {
> @@ -531,20 +540,23 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
>  {
>         int rc, timeout, optype;
>         struct mid_q_entry *mid;
> +       unsigned int credits = 0;
>
>         timeout = flags & CIFS_TIMEOUT_MASK;
>         optype = flags & CIFS_OP_MASK;
>
> -       rc = wait_for_free_request(server, timeout, optype);
> -       if (rc)
> -               return rc;
> +       if ((flags & CIFS_HAS_CREDITS) == 0) {
> +               rc = wait_for_free_request(server, timeout, optype);
> +               if (rc)
> +                       return rc;
> +               credits = 1;
> +       }
>
>         mutex_lock(&server->srv_mutex);
>         mid = server->ops->setup_async_request(server, rqst);
>         if (IS_ERR(mid)) {
>                 mutex_unlock(&server->srv_mutex);
> -               add_credits(server, 1, optype);
> -               wake_up(&server->request_q);
> +               add_credits_and_wake_if(server, credits, optype);
>                 return PTR_ERR(mid);
>         }
>
> @@ -572,8 +584,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
>                 return 0;
>
>         cifs_delete_mid(mid);
> -       add_credits(server, 1, optype);
> -       wake_up(&server->request_q);
> +       add_credits_and_wake_if(server, credits, optype);
>         return rc;
>  }
>
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Pavel Shilovsky]

2014-07-25 5:06 GMT+04:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Looks correct.
>
> Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

Thank you for reviewing the series!

> Only  comment would be, wish there was a mnemonic/define for
> a regular op  when calling add_credits_and_wake_if() for optype
> such as CIFS_ECHO_OP or CIFS_OPBREAK_OP instead of 0.

Yes, it can make sense but should be in another patch/series.

> Oh and super nitpick...  s/reseted/reset/.

Ok, will fix it in my smb2-dev branch on git.altlinux.org.

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Pavel Shilovsky]

2014-07-28 11:36 GMT+04:00 Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
> 2014-07-25 5:06 GMT+04:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> Looks correct.
>>
>> Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>
>
> Thank you for reviewing the series!
>
>> Only  comment would be, wish there was a mnemonic/define for
>> a regular op  when calling add_credits_and_wake_if() for optype
>> such as CIFS_ECHO_OP or CIFS_OPBREAK_OP instead of 0.
>
> Yes, it can make sense but should be in another patch/series.
>
>> Oh and super nitpick...  s/reseted/reset/.
>
> Ok, will fix it in my smb2-dev branch on git.altlinux.org.
>
> --
> Best regards,
> Pavel Shilovsky.

Shirish,

I am going to add a check (as well as removing an unnecessary comment)
to write part:

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 081529f..0dbd1de 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -230,6 +230,9 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon,
struct smb_vol *volume_info)
        wsize = volume_info->wsize ? volume_info->wsize : CIFS_DEFAULT_IOSIZE;
        wsize = min_t(unsigned int, wsize, server->max_write);

+       if (!(server->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU))
+               wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
+
        return wsize;
 }

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 3acef4b..5a6842c 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -108,7 +108,6 @@ smb2_hdr_assemble(struct smb2_hdr *hdr, __le16
smb2_cmd /* command */ ,
        if (!tcon)
                goto out;

-       /* BB FIXME when we do write > 64K add +1 for every 64K in req or rsp */
        /* GLOBAL_CAP_LARGE_MTU will only be set if dialect > SMB2.02 */
        /* See sections 2.2.4 and 3.2.4.1.5 of MS-SMB2 */
        if ((tcon->ses) &&

and the same check to read part:

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 0dbd1de..d0210a8 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -246,6 +246,9 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon,
struct smb_vol *volume_info)
        rsize = volume_info->rsize ? volume_info->rsize : CIFS_DEFAULT_IOSIZE;
        rsize = min_t(unsigned int, rsize, server->max_read);

+       if (!(server->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU))
+               rsize = min_t(unsigned int, rsize, SMB2_MAX_BUFFER_SIZE);
+
        return rsize;
 }

If you are ok with the changes I will update 10/16 and 16/16 patches
and leave your Reviewed-by tags in my git tree. Thoughts?

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Shirish Pargaonkar]

Pavel, that would be fine


On Tue, Jul 29, 2014 at 2:56 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> 2014-07-28 11:36 GMT+04:00 Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
>> 2014-07-25 5:06 GMT+04:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>> Looks correct.
>>>
>>> Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>
>>
>> Thank you for reviewing the series!
>>
>>> Only  comment would be, wish there was a mnemonic/define for
>>> a regular op  when calling add_credits_and_wake_if() for optype
>>> such as CIFS_ECHO_OP or CIFS_OPBREAK_OP instead of 0.
>>
>> Yes, it can make sense but should be in another patch/series.
>>
>>> Oh and super nitpick...  s/reseted/reset/.
>>
>> Ok, will fix it in my smb2-dev branch on git.altlinux.org.
>>
>> --
>> Best regards,
>> Pavel Shilovsky.
>
> Shirish,
>
> I am going to add a check (as well as removing an unnecessary comment)
> to write part:
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 081529f..0dbd1de 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -230,6 +230,9 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon,
> struct smb_vol *volume_info)
>         wsize = volume_info->wsize ? volume_info->wsize : CIFS_DEFAULT_IOSIZE;
>         wsize = min_t(unsigned int, wsize, server->max_write);
>
> +       if (!(server->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU))
> +               wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
> +
>         return wsize;
>  }
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 3acef4b..5a6842c 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -108,7 +108,6 @@ smb2_hdr_assemble(struct smb2_hdr *hdr, __le16
> smb2_cmd /* command */ ,
>         if (!tcon)
>                 goto out;
>
> -       /* BB FIXME when we do write > 64K add +1 for every 64K in req or rsp */
>         /* GLOBAL_CAP_LARGE_MTU will only be set if dialect > SMB2.02 */
>         /* See sections 2.2.4 and 3.2.4.1.5 of MS-SMB2 */
>         if ((tcon->ses) &&
>
> and the same check to read part:
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 0dbd1de..d0210a8 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -246,6 +246,9 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon,
> struct smb_vol *volume_info)
>         rsize = volume_info->rsize ? volume_info->rsize : CIFS_DEFAULT_IOSIZE;
>         rsize = min_t(unsigned int, rsize, server->max_read);
>
> +       if (!(server->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU))
> +               rsize = min_t(unsigned int, rsize, SMB2_MAX_BUFFER_SIZE);
> +
>         return rsize;
>  }
>
> If you are ok with the changes I will update 10/16 and 16/16 patches
> and leave your Reviewed-by tags in my git tree. Thoughts?
>
> --
> Best regards,
> Pavel Shilovsky.

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Pavel Shilovsky]

2014-07-30 6:06 GMT+04:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Pavel, that would be fine

Thank you - the branch is up to date now.

Steve, could you update your for-next branch with the recent version
of patches from my smb2-dev branch, please?
(http://git.altlinux.org/people/piastry/public/?p=cifs-2.6.git;a=shortlog;h=refs/heads/smb2-dev)

We haven't still decided if we need the patch "CIFS: Fix async reading
on reconnects" for stable. My opinion is yes since it fixes the real
problem that leads to lose of a data coherency. Also, the patch "CIFS:
Fix STATUS_CANNOT_DELETE error mapping for SMB2" is targeted for
stable as well.

Do we need to go ahead and send those two to v3.16 (and stable) before
it's released?

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Steve French]

I tried to pull your git tree, but get an error

remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

will try again tomorrow to see if it is back up again.

On Wed, Jul 30, 2014 at 2:41 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> 2014-07-30 6:06 GMT+04:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> Pavel, that would be fine
>
> Thank you - the branch is up to date now.
>
> Steve, could you update your for-next branch with the recent version
> of patches from my smb2-dev branch, please?
> (http://git.altlinux.org/people/piastry/public/?p=cifs-2.6.git;a=shortlog;h=refs/heads/smb2-dev)
>
> We haven't still decided if we need the patch "CIFS: Fix async reading
> on reconnects" for stable. My opinion is yes since it fixes the real
> problem that leads to lose of a data coherency. Also, the patch "CIFS:
> Fix STATUS_CANNOT_DELETE error mapping for SMB2" is targeted for
> stable as well.
>
> Do we need to go ahead and send those two to v3.16 (and stable) before
> it's released?
>
> --
> Best regards,
> Pavel Shilovsky.



-- 
Thanks,

Steve

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Pavel Shilovsky]

2014-07-31 8:23 GMT+04:00 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> I tried to pull your git tree, but get an error
>
> remote: aborting due to possible repository corruption on the remote side.
> fatal: protocol error: bad pack header
>
> will try again tomorrow to see if it is back up again.

Strange enough, it is working from my side.

Anyway, I mirrored the branch at git.etersoft.ru:
http://git.etersoft.ru/people/piastry/packages/?p=cifs-2.6.git;a=shortlog;h=refs/heads/smb2-dev

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Steve French]

merged into cifs-2.6.git

On Thu, Jul 31, 2014 at 1:38 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> 2014-07-31 8:23 GMT+04:00 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> I tried to pull your git tree, but get an error
>>
>> remote: aborting due to possible repository corruption on the remote side.
>> fatal: protocol error: bad pack header
>>
>> will try again tomorrow to see if it is back up again.
>
> Strange enough, it is working from my side.
>
> Anyway, I mirrored the branch at git.etersoft.ru:
> http://git.etersoft.ru/people/piastry/packages/?p=cifs-2.6.git;a=shortlog;h=refs/heads/smb2-dev
>
> --
> Best regards,
> Pavel Shilovsky.



-- 
Thanks,

Steve

Re: [PATCH v3 10/16] CIFS: Use multicredits for SMB 2.1/3 writes

[Pavel Shilovsky]

2014-08-01 9:58 GMT+04:00 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> merged into cifs-2.6.git

What's about the patch
http://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=500cdea99fbbb3a487dad7ce0beba8d44b93a0fa
? It seems like a good candidate for stable@ series. Thoughts?

-- 
Best regards,
Pavel Shilovsky.

[PATCH v3 11/16] CIFS: Separate page search from readpages

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 107 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 61 insertions(+), 46 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c79bdf3..bec48f1 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3340,6 +3340,63 @@ cifs_readpages_read_into_pages(struct TCP_Server_Info *server,
 	return total_read > 0 && result != -EAGAIN ? total_read : result;
 }
 
+static int
+readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
+		    unsigned int rsize, struct list_head *tmplist,
+		    unsigned int *nr_pages, loff_t *offset, unsigned int *bytes)
+{
+	struct page *page, *tpage;
+	unsigned int expected_index;
+	int rc;
+
+	page = list_entry(page_list->prev, struct page, lru);
+
+	/*
+	 * Lock the page and put it in the cache. Since no one else
+	 * should have access to this page, we're safe to simply set
+	 * PG_locked without checking it first.
+	 */
+	__set_page_locked(page);
+	rc = add_to_page_cache_locked(page, mapping,
+				      page->index, GFP_KERNEL);
+
+	/* give up if we can't stick it in the cache */
+	if (rc) {
+		__clear_page_locked(page);
+		return rc;
+	}
+
+	/* move first page to the tmplist */
+	*offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
+	*bytes = PAGE_CACHE_SIZE;
+	*nr_pages = 1;
+	list_move_tail(&page->lru, tmplist);
+
+	/* now try and add more pages onto the request */
+	expected_index = page->index + 1;
+	list_for_each_entry_safe_reverse(page, tpage, page_list, lru) {
+		/* discontinuity ? */
+		if (page->index != expected_index)
+			break;
+
+		/* would this page push the read over the rsize? */
+		if (*bytes + PAGE_CACHE_SIZE > rsize)
+			break;
+
+		__set_page_locked(page);
+		if (add_to_page_cache_locked(page, mapping, page->index,
+								GFP_KERNEL)) {
+			__clear_page_locked(page);
+			break;
+		}
+		list_move_tail(&page->lru, tmplist);
+		(*bytes) += PAGE_CACHE_SIZE;
+		expected_index++;
+		(*nr_pages)++;
+	}
+	return rc;
+}
+
 static int cifs_readpages(struct file *file, struct address_space *mapping,
 	struct list_head *page_list, unsigned num_pages)
 {
@@ -3394,57 +3451,15 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 	 * the rdata->pages, then we want them in increasing order.
 	 */
 	while (!list_empty(page_list)) {
-		unsigned int i;
-		unsigned int bytes = PAGE_CACHE_SIZE;
-		unsigned int expected_index;
-		unsigned int nr_pages = 1;
+		unsigned int i, nr_pages, bytes;
 		loff_t offset;
 		struct page *page, *tpage;
 		struct cifs_readdata *rdata;
 
-		page = list_entry(page_list->prev, struct page, lru);
-
-		/*
-		 * Lock the page and put it in the cache. Since no one else
-		 * should have access to this page, we're safe to simply set
-		 * PG_locked without checking it first.
-		 */
-		__set_page_locked(page);
-		rc = add_to_page_cache_locked(page, mapping,
-					      page->index, GFP_KERNEL);
-
-		/* give up if we can't stick it in the cache */
-		if (rc) {
-			__clear_page_locked(page);
+		rc = readpages_get_pages(mapping, page_list, rsize, &tmplist,
+					 &nr_pages, &offset, &bytes);
+		if (rc)
 			break;
-		}
-
-		/* move first page to the tmplist */
-		offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
-		list_move_tail(&page->lru, &tmplist);
-
-		/* now try and add more pages onto the request */
-		expected_index = page->index + 1;
-		list_for_each_entry_safe_reverse(page, tpage, page_list, lru) {
-			/* discontinuity ? */
-			if (page->index != expected_index)
-				break;
-
-			/* would this page push the read over the rsize? */
-			if (bytes + PAGE_CACHE_SIZE > rsize)
-				break;
-
-			__set_page_locked(page);
-			if (add_to_page_cache_locked(page, mapping,
-						page->index, GFP_KERNEL)) {
-				__clear_page_locked(page);
-				break;
-			}
-			list_move_tail(&page->lru, &tmplist);
-			bytes += PAGE_CACHE_SIZE;
-			expected_index++;
-			nr_pages++;
-		}
 
 		rdata = cifs_readdata_alloc(nr_pages, cifs_readv_complete);
 		if (!rdata) {
-- 
1.8.1.2

Re: [PATCH v3 11/16] CIFS: Separate page search from readpages

[Shirish Pargaonkar]

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 107 ++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 61 insertions(+), 46 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index c79bdf3..bec48f1 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -3340,6 +3340,63 @@ cifs_readpages_read_into_pages(struct TCP_Server_Info *server,
>         return total_read > 0 && result != -EAGAIN ? total_read : result;
>  }
>
> +static int
> +readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
> +                   unsigned int rsize, struct list_head *tmplist,
> +                   unsigned int *nr_pages, loff_t *offset, unsigned int *bytes)
> +{
> +       struct page *page, *tpage;
> +       unsigned int expected_index;
> +       int rc;
> +
> +       page = list_entry(page_list->prev, struct page, lru);
> +
> +       /*
> +        * Lock the page and put it in the cache. Since no one else
> +        * should have access to this page, we're safe to simply set
> +        * PG_locked without checking it first.
> +        */
> +       __set_page_locked(page);
> +       rc = add_to_page_cache_locked(page, mapping,
> +                                     page->index, GFP_KERNEL);
> +
> +       /* give up if we can't stick it in the cache */
> +       if (rc) {
> +               __clear_page_locked(page);
> +               return rc;
> +       }
> +
> +       /* move first page to the tmplist */
> +       *offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
> +       *bytes = PAGE_CACHE_SIZE;
> +       *nr_pages = 1;
> +       list_move_tail(&page->lru, tmplist);
> +
> +       /* now try and add more pages onto the request */
> +       expected_index = page->index + 1;
> +       list_for_each_entry_safe_reverse(page, tpage, page_list, lru) {
> +               /* discontinuity ? */
> +               if (page->index != expected_index)
> +                       break;
> +
> +               /* would this page push the read over the rsize? */
> +               if (*bytes + PAGE_CACHE_SIZE > rsize)
> +                       break;
> +
> +               __set_page_locked(page);
> +               if (add_to_page_cache_locked(page, mapping, page->index,
> +                                                               GFP_KERNEL)) {
> +                       __clear_page_locked(page);
> +                       break;
> +               }
> +               list_move_tail(&page->lru, tmplist);
> +               (*bytes) += PAGE_CACHE_SIZE;
> +               expected_index++;
> +               (*nr_pages)++;
> +       }
> +       return rc;
> +}
> +
>  static int cifs_readpages(struct file *file, struct address_space *mapping,
>         struct list_head *page_list, unsigned num_pages)
>  {
> @@ -3394,57 +3451,15 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>          * the rdata->pages, then we want them in increasing order.
>          */
>         while (!list_empty(page_list)) {
> -               unsigned int i;
> -               unsigned int bytes = PAGE_CACHE_SIZE;
> -               unsigned int expected_index;
> -               unsigned int nr_pages = 1;
> +               unsigned int i, nr_pages, bytes;
>                 loff_t offset;
>                 struct page *page, *tpage;

Looks correct except we do not need these two variables here now.

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

>                 struct cifs_readdata *rdata;
>
> -               page = list_entry(page_list->prev, struct page, lru);
> -
> -               /*
> -                * Lock the page and put it in the cache. Since no one else
> -                * should have access to this page, we're safe to simply set
> -                * PG_locked without checking it first.
> -                */
> -               __set_page_locked(page);
> -               rc = add_to_page_cache_locked(page, mapping,
> -                                             page->index, GFP_KERNEL);
> -
> -               /* give up if we can't stick it in the cache */
> -               if (rc) {
> -                       __clear_page_locked(page);
> +               rc = readpages_get_pages(mapping, page_list, rsize, &tmplist,
> +                                        &nr_pages, &offset, &bytes);
> +               if (rc)
>                         break;
> -               }
> -
> -               /* move first page to the tmplist */
> -               offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
> -               list_move_tail(&page->lru, &tmplist);
> -
> -               /* now try and add more pages onto the request */
> -               expected_index = page->index + 1;
> -               list_for_each_entry_safe_reverse(page, tpage, page_list, lru) {
> -                       /* discontinuity ? */
> -                       if (page->index != expected_index)
> -                               break;
> -
> -                       /* would this page push the read over the rsize? */
> -                       if (bytes + PAGE_CACHE_SIZE > rsize)
> -                               break;
> -
> -                       __set_page_locked(page);
> -                       if (add_to_page_cache_locked(page, mapping,
> -                                               page->index, GFP_KERNEL)) {
> -                               __clear_page_locked(page);
> -                               break;
> -                       }
> -                       list_move_tail(&page->lru, &tmplist);
> -                       bytes += PAGE_CACHE_SIZE;
> -                       expected_index++;
> -                       nr_pages++;
> -               }
>
>                 rdata = cifs_readdata_alloc(nr_pages, cifs_readv_complete);
>                 if (!rdata) {
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/16] CIFS: Separate page search from readpages

[Shirish Pargaonkar]

On Sat, Jul 26, 2014 at 11:40 AM, Shirish Pargaonkar
<shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> ---
>>  fs/cifs/file.c | 107 ++++++++++++++++++++++++++++++++-------------------------
>>  1 file changed, 61 insertions(+), 46 deletions(-)
>>
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index c79bdf3..bec48f1 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -3340,6 +3340,63 @@ cifs_readpages_read_into_pages(struct TCP_Server_Info *server,
>>         return total_read > 0 && result != -EAGAIN ? total_read : result;
>>  }
>>
>> +static int
>> +readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
>> +                   unsigned int rsize, struct list_head *tmplist,
>> +                   unsigned int *nr_pages, loff_t *offset, unsigned int *bytes)
>> +{
>> +       struct page *page, *tpage;
>> +       unsigned int expected_index;
>> +       int rc;
>> +
>> +       page = list_entry(page_list->prev, struct page, lru);
>> +
>> +       /*
>> +        * Lock the page and put it in the cache. Since no one else
>> +        * should have access to this page, we're safe to simply set
>> +        * PG_locked without checking it first.
>> +        */
>> +       __set_page_locked(page);
>> +       rc = add_to_page_cache_locked(page, mapping,
>> +                                     page->index, GFP_KERNEL);
>> +
>> +       /* give up if we can't stick it in the cache */
>> +       if (rc) {
>> +               __clear_page_locked(page);
>> +               return rc;
>> +       }
>> +
>> +       /* move first page to the tmplist */
>> +       *offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
>> +       *bytes = PAGE_CACHE_SIZE;
>> +       *nr_pages = 1;
>> +       list_move_tail(&page->lru, tmplist);
>> +
>> +       /* now try and add more pages onto the request */
>> +       expected_index = page->index + 1;
>> +       list_for_each_entry_safe_reverse(page, tpage, page_list, lru) {
>> +               /* discontinuity ? */
>> +               if (page->index != expected_index)
>> +                       break;
>> +
>> +               /* would this page push the read over the rsize? */
>> +               if (*bytes + PAGE_CACHE_SIZE > rsize)
>> +                       break;
>> +
>> +               __set_page_locked(page);
>> +               if (add_to_page_cache_locked(page, mapping, page->index,
>> +                                                               GFP_KERNEL)) {
>> +                       __clear_page_locked(page);
>> +                       break;
>> +               }
>> +               list_move_tail(&page->lru, tmplist);
>> +               (*bytes) += PAGE_CACHE_SIZE;
>> +               expected_index++;
>> +               (*nr_pages)++;
>> +       }
>> +       return rc;
>> +}
>> +
>>  static int cifs_readpages(struct file *file, struct address_space *mapping,
>>         struct list_head *page_list, unsigned num_pages)
>>  {
>> @@ -3394,57 +3451,15 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>>          * the rdata->pages, then we want them in increasing order.
>>          */
>>         while (!list_empty(page_list)) {
>> -               unsigned int i;
>> -               unsigned int bytes = PAGE_CACHE_SIZE;
>> -               unsigned int expected_index;
>> -               unsigned int nr_pages = 1;
>> +               unsigned int i, nr_pages, bytes;
>>                 loff_t offset;
>>                 struct page *page, *tpage;
>
> Looks correct except we do not need these two variables here now.

Ignore this comment.

>
> Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>
>
>>                 struct cifs_readdata *rdata;
>>
>> -               page = list_entry(page_list->prev, struct page, lru);
>> -
>> -               /*
>> -                * Lock the page and put it in the cache. Since no one else
>> -                * should have access to this page, we're safe to simply set
>> -                * PG_locked without checking it first.
>> -                */
>> -               __set_page_locked(page);
>> -               rc = add_to_page_cache_locked(page, mapping,
>> -                                             page->index, GFP_KERNEL);
>> -
>> -               /* give up if we can't stick it in the cache */
>> -               if (rc) {
>> -                       __clear_page_locked(page);
>> +               rc = readpages_get_pages(mapping, page_list, rsize, &tmplist,
>> +                                        &nr_pages, &offset, &bytes);
>> +               if (rc)
>>                         break;
>> -               }
>> -
>> -               /* move first page to the tmplist */
>> -               offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
>> -               list_move_tail(&page->lru, &tmplist);
>> -
>> -               /* now try and add more pages onto the request */
>> -               expected_index = page->index + 1;
>> -               list_for_each_entry_safe_reverse(page, tpage, page_list, lru) {
>> -                       /* discontinuity ? */
>> -                       if (page->index != expected_index)
>> -                               break;
>> -
>> -                       /* would this page push the read over the rsize? */
>> -                       if (bytes + PAGE_CACHE_SIZE > rsize)
>> -                               break;
>> -
>> -                       __set_page_locked(page);
>> -                       if (add_to_page_cache_locked(page, mapping,
>> -                                               page->index, GFP_KERNEL)) {
>> -                               __clear_page_locked(page);
>> -                               break;
>> -                       }
>> -                       list_move_tail(&page->lru, &tmplist);
>> -                       bytes += PAGE_CACHE_SIZE;
>> -                       expected_index++;
>> -                       nr_pages++;
>> -               }
>>
>>                 rdata = cifs_readdata_alloc(nr_pages, cifs_readv_complete);
>>                 if (!rdata) {
>> --
>> 1.8.1.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 12/16] CIFS: Fix rsize usage in readpages

[Pavel Shilovsky]

If a server changes maximum buffer size for read (rsize) requests
on reconnect we can fail on repeating with a big size buffer on
-EAGAIN error in readpages. Fix this by checking rsize all the
time before repeating requests.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index bec48f1..d627918 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3349,6 +3349,8 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
 	unsigned int expected_index;
 	int rc;
 
+	INIT_LIST_HEAD(tmplist);
+
 	page = list_entry(page_list->prev, struct page, lru);
 
 	/*
@@ -3404,19 +3406,10 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 	struct list_head tmplist;
 	struct cifsFileInfo *open_file = file->private_data;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
-	unsigned int rsize = cifs_sb->rsize;
+	struct TCP_Server_Info *server;
 	pid_t pid;
 
 	/*
-	 * Give up immediately if rsize is too small to read an entire page.
-	 * The VFS will fall back to readpage. We should never reach this
-	 * point however since we set ra_pages to 0 when the rsize is smaller
-	 * than a cache page.
-	 */
-	if (unlikely(rsize < PAGE_CACHE_SIZE))
-		return 0;
-
-	/*
 	 * Reads as many pages as possible from fscache. Returns -ENOBUFS
 	 * immediately if the cookie is negative
 	 *
@@ -3434,7 +3427,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 		pid = current->tgid;
 
 	rc = 0;
-	INIT_LIST_HEAD(&tmplist);
+	server = tlink_tcon(open_file->tlink)->ses->server;
 
 	cifs_dbg(FYI, "%s: file=%p mapping=%p num_pages=%u\n",
 		 __func__, file, mapping, num_pages);
@@ -3456,8 +3449,17 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 		struct page *page, *tpage;
 		struct cifs_readdata *rdata;
 
-		rc = readpages_get_pages(mapping, page_list, rsize, &tmplist,
-					 &nr_pages, &offset, &bytes);
+		/*
+		 * Give up immediately if rsize is too small to read an entire
+		 * page. The VFS will fall back to readpage. We should never
+		 * reach this point however since we set ra_pages to 0 when the
+		 * rsize is smaller than a cache page.
+		 */
+		if (unlikely(cifs_sb->rsize < PAGE_CACHE_SIZE))
+			return 0;
+
+		rc = readpages_get_pages(mapping, page_list, cifs_sb->rsize,
+					 &tmplist, &nr_pages, &offset, &bytes);
 		if (rc)
 			break;
 
@@ -3487,15 +3489,24 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 			rdata->pages[rdata->nr_pages++] = page;
 		}
 
-		rc = cifs_retry_async_readv(rdata);
-		if (rc != 0) {
+		if (!rdata->cfile->invalidHandle ||
+		    !cifs_reopen_file(rdata->cfile, true))
+			rc = server->ops->async_readv(rdata);
+		if (rc) {
 			for (i = 0; i < rdata->nr_pages; i++) {
 				page = rdata->pages[i];
 				lru_cache_add_file(page);
 				unlock_page(page);
 				page_cache_release(page);
+				if (rc == -EAGAIN)
+					list_add_tail(&page->lru, &tmplist);
 			}
 			kref_put(&rdata->refcount, cifs_readdata_release);
+			if (rc == -EAGAIN) {
+				/* Re-add pages to the page_list and retry */
+				list_splice(&tmplist, page_list);
+				continue;
+			}
 			break;
 		}
 
-- 
1.8.1.2

Re: [PATCH v3 12/16] CIFS: Fix rsize usage in readpages

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If a server changes maximum buffer size for read (rsize) requests
> on reconnect we can fail on repeating with a big size buffer on
> -EAGAIN error in readpages. Fix this by checking rsize all the
> time before repeating requests.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 41 ++++++++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index bec48f1..d627918 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -3349,6 +3349,8 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
>         unsigned int expected_index;
>         int rc;
>
> +       INIT_LIST_HEAD(tmplist);
> +
>         page = list_entry(page_list->prev, struct page, lru);
>
>         /*
> @@ -3404,19 +3406,10 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>         struct list_head tmplist;
>         struct cifsFileInfo *open_file = file->private_data;
>         struct cifs_sb_info *cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
> -       unsigned int rsize = cifs_sb->rsize;
> +       struct TCP_Server_Info *server;
>         pid_t pid;
>
>         /*
> -        * Give up immediately if rsize is too small to read an entire page.
> -        * The VFS will fall back to readpage. We should never reach this
> -        * point however since we set ra_pages to 0 when the rsize is smaller
> -        * than a cache page.
> -        */
> -       if (unlikely(rsize < PAGE_CACHE_SIZE))
> -               return 0;
> -
> -       /*
>          * Reads as many pages as possible from fscache. Returns -ENOBUFS
>          * immediately if the cookie is negative
>          *
> @@ -3434,7 +3427,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                 pid = current->tgid;
>
>         rc = 0;
> -       INIT_LIST_HEAD(&tmplist);
> +       server = tlink_tcon(open_file->tlink)->ses->server;
>
>         cifs_dbg(FYI, "%s: file=%p mapping=%p num_pages=%u\n",
>                  __func__, file, mapping, num_pages);
> @@ -3456,8 +3449,17 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                 struct page *page, *tpage;
>                 struct cifs_readdata *rdata;
>
> -               rc = readpages_get_pages(mapping, page_list, rsize, &tmplist,
> -                                        &nr_pages, &offset, &bytes);
> +               /*
> +                * Give up immediately if rsize is too small to read an entire
> +                * page. The VFS will fall back to readpage. We should never
> +                * reach this point however since we set ra_pages to 0 when the
> +                * rsize is smaller than a cache page.
> +                */
> +               if (unlikely(cifs_sb->rsize < PAGE_CACHE_SIZE))
> +                       return 0;
> +
> +               rc = readpages_get_pages(mapping, page_list, cifs_sb->rsize,
> +                                        &tmplist, &nr_pages, &offset, &bytes);
>                 if (rc)
>                         break;
>
> @@ -3487,15 +3489,24 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                         rdata->pages[rdata->nr_pages++] = page;
>                 }
>
> -               rc = cifs_retry_async_readv(rdata);
> -               if (rc != 0) {
> +               if (!rdata->cfile->invalidHandle ||
> +                   !cifs_reopen_file(rdata->cfile, true))
> +                       rc = server->ops->async_readv(rdata);
> +               if (rc) {
>                         for (i = 0; i < rdata->nr_pages; i++) {
>                                 page = rdata->pages[i];
>                                 lru_cache_add_file(page);
>                                 unlock_page(page);
>                                 page_cache_release(page);
> +                               if (rc == -EAGAIN)
> +                                       list_add_tail(&page->lru, &tmplist);
>                         }
>                         kref_put(&rdata->refcount, cifs_readdata_release);
> +                       if (rc == -EAGAIN) {
> +                               /* Re-add pages to the page_list and retry */
> +                               list_splice(&tmplist, page_list);
> +                               continue;
> +                       }
>                         break;
>                 }
>
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 13/16] CIFS: Separate page reading from user read

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 69 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index d627918..7df4e46 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2932,43 +2932,23 @@ cifs_uncached_read_into_pages(struct TCP_Server_Info *server,
 	return total_read > 0 && result != -EAGAIN ? total_read : result;
 }
 
-ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
+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)
 {
-	struct file *file = iocb->ki_filp;
-	ssize_t rc;
-	size_t len, cur_len;
-	ssize_t total_read = 0;
-	loff_t offset = iocb->ki_pos;
+	struct cifs_readdata *rdata;
 	unsigned int npages;
-	struct cifs_sb_info *cifs_sb;
-	struct cifs_tcon *tcon;
-	struct cifsFileInfo *open_file;
-	struct cifs_readdata *rdata, *tmp;
-	struct list_head rdata_list;
+	size_t cur_len;
+	int rc;
 	pid_t pid;
 
-	len = iov_iter_count(to);
-	if (!len)
-		return 0;
-
-	INIT_LIST_HEAD(&rdata_list);
-	cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
-	open_file = file->private_data;
-	tcon = tlink_tcon(open_file->tlink);
-
-	if (!tcon->ses->server->ops->async_readv)
-		return -ENOSYS;
-
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
 		pid = open_file->pid;
 	else
 		pid = current->tgid;
 
-	if ((file->f_flags & O_ACCMODE) == O_WRONLY)
-		cifs_dbg(FYI, "attempting read on write only file instance\n");
-
 	do {
-		cur_len = min_t(const size_t, len - total_read, cifs_sb->rsize);
+		cur_len = min_t(const size_t, len, cifs_sb->rsize);
 		npages = DIV_ROUND_UP(cur_len, PAGE_SIZE);
 
 		/* allocate a readdata struct */
@@ -2999,11 +2979,44 @@ error:
 			break;
 		}
 
-		list_add_tail(&rdata->list, &rdata_list);
+		list_add_tail(&rdata->list, rdata_list);
 		offset += cur_len;
 		len -= cur_len;
 	} while (len > 0);
 
+	return rc;
+}
+
+ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
+{
+	struct file *file = iocb->ki_filp;
+	ssize_t rc;
+	size_t len;
+	ssize_t total_read = 0;
+	loff_t offset = iocb->ki_pos;
+	struct cifs_sb_info *cifs_sb;
+	struct cifs_tcon *tcon;
+	struct cifsFileInfo *open_file;
+	struct cifs_readdata *rdata, *tmp;
+	struct list_head rdata_list;
+
+	len = iov_iter_count(to);
+	if (!len)
+		return 0;
+
+	INIT_LIST_HEAD(&rdata_list);
+	cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
+	open_file = file->private_data;
+	tcon = tlink_tcon(open_file->tlink);
+
+	if (!tcon->ses->server->ops->async_readv)
+		return -ENOSYS;
+
+	if ((file->f_flags & O_ACCMODE) == O_WRONLY)
+		cifs_dbg(FYI, "attempting read on write only file instance\n");
+
+	rc = cifs_send_async_read(offset, len, open_file, cifs_sb, &rdata_list);
+
 	/* if at least one read request send succeeded, then reset rc */
 	if (!list_empty(&rdata_list))
 		rc = 0;
-- 
1.8.1.2

Re: [PATCH v3 13/16] CIFS: Separate page reading from user read

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 69 ++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 41 insertions(+), 28 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index d627918..7df4e46 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2932,43 +2932,23 @@ cifs_uncached_read_into_pages(struct TCP_Server_Info *server,
>         return total_read > 0 && result != -EAGAIN ? total_read : result;
>  }
>
> -ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
> +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)
>  {
> -       struct file *file = iocb->ki_filp;
> -       ssize_t rc;
> -       size_t len, cur_len;
> -       ssize_t total_read = 0;
> -       loff_t offset = iocb->ki_pos;
> +       struct cifs_readdata *rdata;
>         unsigned int npages;
> -       struct cifs_sb_info *cifs_sb;
> -       struct cifs_tcon *tcon;
> -       struct cifsFileInfo *open_file;
> -       struct cifs_readdata *rdata, *tmp;
> -       struct list_head rdata_list;
> +       size_t cur_len;
> +       int rc;
>         pid_t pid;
>
> -       len = iov_iter_count(to);
> -       if (!len)
> -               return 0;
> -
> -       INIT_LIST_HEAD(&rdata_list);
> -       cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
> -       open_file = file->private_data;
> -       tcon = tlink_tcon(open_file->tlink);
> -
> -       if (!tcon->ses->server->ops->async_readv)
> -               return -ENOSYS;
> -
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
>                 pid = open_file->pid;
>         else
>                 pid = current->tgid;
>
> -       if ((file->f_flags & O_ACCMODE) == O_WRONLY)
> -               cifs_dbg(FYI, "attempting read on write only file instance\n");
> -
>         do {
> -               cur_len = min_t(const size_t, len - total_read, cifs_sb->rsize);
> +               cur_len = min_t(const size_t, len, cifs_sb->rsize);
>                 npages = DIV_ROUND_UP(cur_len, PAGE_SIZE);
>
>                 /* allocate a readdata struct */
> @@ -2999,11 +2979,44 @@ error:
>                         break;
>                 }
>
> -               list_add_tail(&rdata->list, &rdata_list);
> +               list_add_tail(&rdata->list, rdata_list);
>                 offset += cur_len;
>                 len -= cur_len;
>         } while (len > 0);
>
> +       return rc;
> +}
> +
> +ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
> +{
> +       struct file *file = iocb->ki_filp;
> +       ssize_t rc;
> +       size_t len;
> +       ssize_t total_read = 0;
> +       loff_t offset = iocb->ki_pos;
> +       struct cifs_sb_info *cifs_sb;
> +       struct cifs_tcon *tcon;
> +       struct cifsFileInfo *open_file;
> +       struct cifs_readdata *rdata, *tmp;
> +       struct list_head rdata_list;
> +
> +       len = iov_iter_count(to);
> +       if (!len)
> +               return 0;
> +
> +       INIT_LIST_HEAD(&rdata_list);
> +       cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
> +       open_file = file->private_data;
> +       tcon = tlink_tcon(open_file->tlink);
> +
> +       if (!tcon->ses->server->ops->async_readv)
> +               return -ENOSYS;
> +
> +       if ((file->f_flags & O_ACCMODE) == O_WRONLY)
> +               cifs_dbg(FYI, "attempting read on write only file instance\n");
> +
> +       rc = cifs_send_async_read(offset, len, open_file, cifs_sb, &rdata_list);
> +
>         /* if at least one read request send succeeded, then reset rc */
>         if (!list_empty(&rdata_list))
>                 rc = 0;
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 14/16] CIFS: Fix rsize usage in user read

[Pavel Shilovsky]

If a server changes maximum buffer size for read (rsize) requests
on reconnect we can fail on repeating with a big size buffer on
-EAGAIN error in user read. Fix this by checking rsize all the
time before repeating requests.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 7df4e46..bbc3859 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2828,26 +2828,6 @@ cifs_uncached_readdata_release(struct kref *refcount)
 	cifs_readdata_release(refcount);
 }
 
-static int
-cifs_retry_async_readv(struct cifs_readdata *rdata)
-{
-	int rc;
-	struct TCP_Server_Info *server;
-
-	server = tlink_tcon(rdata->cfile->tlink)->ses->server;
-
-	do {
-		if (rdata->cfile->invalidHandle) {
-			rc = cifs_reopen_file(rdata->cfile, true);
-			if (rc != 0)
-				continue;
-		}
-		rc = server->ops->async_readv(rdata);
-	} while (rc == -EAGAIN);
-
-	return rc;
-}
-
 /**
  * cifs_readdata_to_iov - copy data from pages in response to an iovec
  * @rdata:	the readdata response with list of pages holding data
@@ -2941,6 +2921,9 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
 	size_t cur_len;
 	int rc;
 	pid_t pid;
+	struct TCP_Server_Info *server;
+
+	server = tlink_tcon(open_file->tlink)->ses->server;
 
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
 		pid = open_file->pid;
@@ -2971,11 +2954,15 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
 		rdata->pagesz = PAGE_SIZE;
 		rdata->read_into_pages = cifs_uncached_read_into_pages;
 
-		rc = cifs_retry_async_readv(rdata);
+		if (!rdata->cfile->invalidHandle ||
+		    !cifs_reopen_file(rdata->cfile, true))
+			rc = server->ops->async_readv(rdata);
 error:
 		if (rc) {
 			kref_put(&rdata->refcount,
 				 cifs_uncached_readdata_release);
+			if (rc == -EAGAIN)
+				continue;
 			break;
 		}
 
@@ -3023,8 +3010,8 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
 
 	len = iov_iter_count(to);
 	/* the loop below should proceed in the order of increasing offsets */
+again:
 	list_for_each_entry_safe(rdata, tmp, &rdata_list, list) {
-	again:
 		if (!rc) {
 			/* FIXME: freezable sleep too? */
 			rc = wait_for_completion_killable(&rdata->done);
@@ -3034,7 +3021,19 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
 				rc = rdata->result;
 				/* resend call if it's a retryable error */
 				if (rc == -EAGAIN) {
-					rc = cifs_retry_async_readv(rdata);
+					struct list_head tmp_list;
+
+					list_del_init(&rdata->list);
+					INIT_LIST_HEAD(&tmp_list);
+
+					rc = cifs_send_async_read(rdata->offset,
+						rdata->bytes, rdata->cfile,
+						cifs_sb, &tmp_list);
+
+					list_splice(&tmp_list, &rdata_list);
+
+					kref_put(&rdata->refcount,
+						cifs_uncached_readdata_release);
 					goto again;
 				}
 			} else {
-- 
1.8.1.2

Re: [PATCH v3 14/16] CIFS: Fix rsize usage in user read

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If a server changes maximum buffer size for read (rsize) requests
> on reconnect we can fail on repeating with a big size buffer on
> -EAGAIN error in user read. Fix this by checking rsize all the
> time before repeating requests.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 45 ++++++++++++++++++++++-----------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 7df4e46..bbc3859 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2828,26 +2828,6 @@ cifs_uncached_readdata_release(struct kref *refcount)
>         cifs_readdata_release(refcount);
>  }
>
> -static int
> -cifs_retry_async_readv(struct cifs_readdata *rdata)
> -{
> -       int rc;
> -       struct TCP_Server_Info *server;
> -
> -       server = tlink_tcon(rdata->cfile->tlink)->ses->server;
> -
> -       do {
> -               if (rdata->cfile->invalidHandle) {
> -                       rc = cifs_reopen_file(rdata->cfile, true);
> -                       if (rc != 0)
> -                               continue;
> -               }
> -               rc = server->ops->async_readv(rdata);
> -       } while (rc == -EAGAIN);
> -
> -       return rc;
> -}
> -
>  /**
>   * cifs_readdata_to_iov - copy data from pages in response to an iovec
>   * @rdata:     the readdata response with list of pages holding data
> @@ -2941,6 +2921,9 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
>         size_t cur_len;
>         int rc;
>         pid_t pid;
> +       struct TCP_Server_Info *server;
> +
> +       server = tlink_tcon(open_file->tlink)->ses->server;
>
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
>                 pid = open_file->pid;
> @@ -2971,11 +2954,15 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
>                 rdata->pagesz = PAGE_SIZE;
>                 rdata->read_into_pages = cifs_uncached_read_into_pages;
>
> -               rc = cifs_retry_async_readv(rdata);
> +               if (!rdata->cfile->invalidHandle ||
> +                   !cifs_reopen_file(rdata->cfile, true))
> +                       rc = server->ops->async_readv(rdata);
>  error:
>                 if (rc) {
>                         kref_put(&rdata->refcount,
>                                  cifs_uncached_readdata_release);
> +                       if (rc == -EAGAIN)
> +                               continue;
>                         break;
>                 }
>
> @@ -3023,8 +3010,8 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
>
>         len = iov_iter_count(to);
>         /* the loop below should proceed in the order of increasing offsets */
> +again:
>         list_for_each_entry_safe(rdata, tmp, &rdata_list, list) {
> -       again:
>                 if (!rc) {
>                         /* FIXME: freezable sleep too? */
>                         rc = wait_for_completion_killable(&rdata->done);
> @@ -3034,7 +3021,19 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
>                                 rc = rdata->result;
>                                 /* resend call if it's a retryable error */
>                                 if (rc == -EAGAIN) {
> -                                       rc = cifs_retry_async_readv(rdata);
> +                                       struct list_head tmp_list;
> +
> +                                       list_del_init(&rdata->list);
> +                                       INIT_LIST_HEAD(&tmp_list);
> +
> +                                       rc = cifs_send_async_read(rdata->offset,
> +                                               rdata->bytes, rdata->cfile,
> +                                               cifs_sb, &tmp_list);
> +
> +                                       list_splice(&tmp_list, &rdata_list);
> +
> +                                       kref_put(&rdata->refcount,
> +                                               cifs_uncached_readdata_release);
>                                         goto again;
>                                 }
>                         } else {
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 15/16] CIFS: Fix rsize usage for sync read

[Pavel Shilovsky]

If a server changes maximum buffer size for read requests (rsize)
on reconnect we can fail on repeating with a big size buffer on
-EAGAIN error in cifs_read. Fix this by checking rsize all the
time before repeating requests.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/file.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index bbc3859..00b2a25 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3148,18 +3148,19 @@ cifs_read(struct file *file, char *read_data, size_t read_size, loff_t *offset)
 
 	for (total_read = 0, cur_offset = read_data; read_size > total_read;
 	     total_read += bytes_read, cur_offset += bytes_read) {
-		current_read_size = min_t(uint, read_size - total_read, rsize);
-		/*
-		 * For windows me and 9x we do not want to request more than it
-		 * negotiated since it will refuse the read then.
-		 */
-		if ((tcon->ses) && !(tcon->ses->capabilities &
+		do {
+			current_read_size = min_t(uint, read_size - total_read,
+						  rsize);
+			/*
+			 * For windows me and 9x we do not want to request more
+			 * than it negotiated since it will refuse the read
+			 * then.
+			 */
+			if ((tcon->ses) && !(tcon->ses->capabilities &
 				tcon->ses->server->vals->cap_large_files)) {
-			current_read_size = min_t(uint, current_read_size,
-					CIFSMaxBufSize);
-		}
-		rc = -EAGAIN;
-		while (rc == -EAGAIN) {
+				current_read_size = min_t(uint,
+					current_read_size, CIFSMaxBufSize);
+			}
 			if (open_file->invalidHandle) {
 				rc = cifs_reopen_file(open_file, true);
 				if (rc != 0)
@@ -3172,7 +3173,8 @@ cifs_read(struct file *file, char *read_data, size_t read_size, loff_t *offset)
 			rc = server->ops->sync_read(xid, open_file, &io_parms,
 						    &bytes_read, &cur_offset,
 						    &buf_type);
-		}
+		} while (rc == -EAGAIN);
+
 		if (rc || (bytes_read == 0)) {
 			if (total_read) {
 				break;
-- 
1.8.1.2

Re: [PATCH v3 15/16] CIFS: Fix rsize usage for sync read

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If a server changes maximum buffer size for read requests (rsize)
> on reconnect we can fail on repeating with a big size buffer on
> -EAGAIN error in cifs_read. Fix this by checking rsize all the
> time before repeating requests.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/file.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index bbc3859..00b2a25 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -3148,18 +3148,19 @@ cifs_read(struct file *file, char *read_data, size_t read_size, loff_t *offset)
>
>         for (total_read = 0, cur_offset = read_data; read_size > total_read;
>              total_read += bytes_read, cur_offset += bytes_read) {
> -               current_read_size = min_t(uint, read_size - total_read, rsize);
> -               /*
> -                * For windows me and 9x we do not want to request more than it
> -                * negotiated since it will refuse the read then.
> -                */
> -               if ((tcon->ses) && !(tcon->ses->capabilities &
> +               do {
> +                       current_read_size = min_t(uint, read_size - total_read,
> +                                                 rsize);
> +                       /*
> +                        * For windows me and 9x we do not want to request more
> +                        * than it negotiated since it will refuse the read
> +                        * then.
> +                        */
> +                       if ((tcon->ses) && !(tcon->ses->capabilities &
>                                 tcon->ses->server->vals->cap_large_files)) {
> -                       current_read_size = min_t(uint, current_read_size,
> -                                       CIFSMaxBufSize);
> -               }
> -               rc = -EAGAIN;
> -               while (rc == -EAGAIN) {
> +                               current_read_size = min_t(uint,
> +                                       current_read_size, CIFSMaxBufSize);
> +                       }
>                         if (open_file->invalidHandle) {
>                                 rc = cifs_reopen_file(open_file, true);
>                                 if (rc != 0)
> @@ -3172,7 +3173,8 @@ cifs_read(struct file *file, char *read_data, size_t read_size, loff_t *offset)
>                         rc = server->ops->sync_read(xid, open_file, &io_parms,
>                                                     &bytes_read, &cur_offset,
>                                                     &buf_type);
> -               }
> +               } while (rc == -EAGAIN);
> +
>                 if (rc || (bytes_read == 0)) {
>                         if (total_read) {
>                                 break;
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 16/16] CIFS: Use multicredits for SMB 2.1/3 reads

[Pavel Shilovsky]

If we negotiate SMB 2.1 and higher version of the protocol and
a server supports large read buffer size, we need to consume 1
credit per 65536 bytes. So, we need to know how many credits
we have and obtain the required number of them before constructing
a readdata structure in readpages and user read.

Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 fs/cifs/cifsglob.h |  1 +
 fs/cifs/file.c     | 35 ++++++++++++++++++++++++++++-------
 fs/cifs/smb2ops.c  |  2 --
 fs/cifs/smb2pdu.c  | 30 +++++++++++++++++++++++++++---
 4 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 54ca2b9..f33ff4c 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1068,6 +1068,7 @@ struct cifs_readdata {
 	struct kvec			iov;
 	unsigned int			pagesz;
 	unsigned int			tailsz;
+	unsigned int			credits;
 	unsigned int			nr_pages;
 	struct page			*pages[];
 };
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 00b2a25..ebdeb56 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2917,7 +2917,7 @@ 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)
 {
 	struct cifs_readdata *rdata;
-	unsigned int npages;
+	unsigned int npages, rsize, credits;
 	size_t cur_len;
 	int rc;
 	pid_t pid;
@@ -2931,13 +2931,19 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
 		pid = current->tgid;
 
 	do {
-		cur_len = min_t(const size_t, len, cifs_sb->rsize);
+		rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize,
+						   &rsize, &credits);
+		if (rc)
+			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,
 					    cifs_uncached_readv_complete);
 		if (!rdata) {
+			add_credits_and_wake_if(server, credits, 0);
 			rc = -ENOMEM;
 			break;
 		}
@@ -2953,12 +2959,14 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
 		rdata->pid = pid;
 		rdata->pagesz = PAGE_SIZE;
 		rdata->read_into_pages = cifs_uncached_read_into_pages;
+		rdata->credits = credits;
 
 		if (!rdata->cfile->invalidHandle ||
 		    !cifs_reopen_file(rdata->cfile, true))
 			rc = server->ops->async_readv(rdata);
 error:
 		if (rc) {
+			add_credits_and_wake_if(server, rdata->credits, 0);
 			kref_put(&rdata->refcount,
 				 cifs_uncached_readdata_release);
 			if (rc == -EAGAIN)
@@ -3458,10 +3466,16 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 	 * the rdata->pages, then we want them in increasing order.
 	 */
 	while (!list_empty(page_list)) {
-		unsigned int i, nr_pages, bytes;
+		unsigned int i, nr_pages, bytes, rsize;
 		loff_t offset;
 		struct page *page, *tpage;
 		struct cifs_readdata *rdata;
+		unsigned credits;
+
+		rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize,
+						   &rsize, &credits);
+		if (rc)
+			break;
 
 		/*
 		 * Give up immediately if rsize is too small to read an entire
@@ -3469,13 +3483,17 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 		 * reach this point however since we set ra_pages to 0 when the
 		 * rsize is smaller than a cache page.
 		 */
-		if (unlikely(cifs_sb->rsize < PAGE_CACHE_SIZE))
+		if (unlikely(rsize < PAGE_CACHE_SIZE)) {
+			add_credits_and_wake_if(server, credits, 0);
 			return 0;
+		}
 
-		rc = readpages_get_pages(mapping, page_list, cifs_sb->rsize,
-					 &tmplist, &nr_pages, &offset, &bytes);
-		if (rc)
+		rc = readpages_get_pages(mapping, page_list, rsize, &tmplist,
+					 &nr_pages, &offset, &bytes);
+		if (rc) {
+			add_credits_and_wake_if(server, credits, 0);
 			break;
+		}
 
 		rdata = cifs_readdata_alloc(nr_pages, cifs_readv_complete);
 		if (!rdata) {
@@ -3487,6 +3505,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 				page_cache_release(page);
 			}
 			rc = -ENOMEM;
+			add_credits_and_wake_if(server, credits, 0);
 			break;
 		}
 
@@ -3497,6 +3516,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 		rdata->pid = pid;
 		rdata->pagesz = PAGE_CACHE_SIZE;
 		rdata->read_into_pages = cifs_readpages_read_into_pages;
+		rdata->credits = credits;
 
 		list_for_each_entry_safe(page, tpage, &tmplist, lru) {
 			list_del(&page->lru);
@@ -3507,6 +3527,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 		    !cifs_reopen_file(rdata->cfile, true))
 			rc = server->ops->async_readv(rdata);
 		if (rc) {
+			add_credits_and_wake_if(server, rdata->credits, 0);
 			for (i = 0; i < rdata->nr_pages; i++) {
 				page = rdata->pages[i];
 				lru_cache_add_file(page);
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index fecc2de..081529f 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -242,8 +242,6 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
 	/* start with specified rsize, or default */
 	rsize = volume_info->rsize ? volume_info->rsize : CIFS_DEFAULT_IOSIZE;
 	rsize = min_t(unsigned int, rsize, server->max_read);
-	/* set it to the maximum buffer size value we can send with 1 credit */
-	rsize = min_t(unsigned int, rsize, SMB2_MAX_BUFFER_SIZE);
 
 	return rsize;
 }
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index a1d89b7..26624ee 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1758,11 +1758,12 @@ smb2_readv_callback(struct mid_q_entry *mid)
 int
 smb2_async_readv(struct cifs_readdata *rdata)
 {
-	int rc;
+	int rc, flags = 0;
 	struct smb2_hdr *buf;
 	struct cifs_io_parms io_parms;
 	struct smb_rqst rqst = { .rq_iov = &rdata->iov,
 				 .rq_nvec = 1 };
+	struct TCP_Server_Info *server;
 
 	cifs_dbg(FYI, "%s: offset=%llu bytes=%u\n",
 		 __func__, rdata->offset, rdata->bytes);
@@ -1773,18 +1774,41 @@ smb2_async_readv(struct cifs_readdata *rdata)
 	io_parms.persistent_fid = rdata->cfile->fid.persistent_fid;
 	io_parms.volatile_fid = rdata->cfile->fid.volatile_fid;
 	io_parms.pid = rdata->pid;
+
+	server = io_parms.tcon->ses->server;
+
 	rc = smb2_new_read_req(&rdata->iov, &io_parms, 0, 0);
-	if (rc)
+	if (rc) {
+		if (rc == -EAGAIN && rdata->credits) {
+			/* credits was reseted by reconnect */
+			rdata->credits = 0;
+			/* reduce in_flight value since we won't send the req */
+			spin_lock(&server->req_lock);
+			server->in_flight--;
+			spin_unlock(&server->req_lock);
+		}
 		return rc;
+	}
 
 	buf = (struct smb2_hdr *)rdata->iov.iov_base;
 	/* 4 for rfc1002 length field */
 	rdata->iov.iov_len = get_rfc1002_length(rdata->iov.iov_base) + 4;
 
+	if (rdata->credits) {
+		buf->CreditCharge = cpu_to_le16(DIV_ROUND_UP(rdata->bytes,
+						SMB2_MAX_BUFFER_SIZE));
+		spin_lock(&server->req_lock);
+		server->credits += rdata->credits -
+						le16_to_cpu(buf->CreditCharge);
+		spin_unlock(&server->req_lock);
+		wake_up(&server->request_q);
+		flags = CIFS_HAS_CREDITS;
+	}
+
 	kref_get(&rdata->refcount);
 	rc = cifs_call_async(io_parms.tcon->ses->server, &rqst,
 			     cifs_readv_receive, smb2_readv_callback,
-			     rdata, 0);
+			     rdata, flags);
 	if (rc) {
 		kref_put(&rdata->refcount, cifs_readdata_release);
 		cifs_stats_fail_inc(io_parms.tcon, SMB2_READ_HE);
-- 
1.8.1.2

Re: [PATCH v3 16/16] CIFS: Use multicredits for SMB 2.1/3 reads

[Shirish Pargaonkar]

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Mon, Jul 21, 2014 at 10:45 AM, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> If we negotiate SMB 2.1 and higher version of the protocol and
> a server supports large read buffer size, we need to consume 1
> credit per 65536 bytes. So, we need to know how many credits
> we have and obtain the required number of them before constructing
> a readdata structure in readpages and user read.
>
> Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |  1 +
>  fs/cifs/file.c     | 35 ++++++++++++++++++++++++++++-------
>  fs/cifs/smb2ops.c  |  2 --
>  fs/cifs/smb2pdu.c  | 30 +++++++++++++++++++++++++++---
>  4 files changed, 56 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 54ca2b9..f33ff4c 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1068,6 +1068,7 @@ struct cifs_readdata {
>         struct kvec                     iov;
>         unsigned int                    pagesz;
>         unsigned int                    tailsz;
> +       unsigned int                    credits;
>         unsigned int                    nr_pages;
>         struct page                     *pages[];
>  };
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 00b2a25..ebdeb56 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2917,7 +2917,7 @@ 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)
>  {
>         struct cifs_readdata *rdata;
> -       unsigned int npages;
> +       unsigned int npages, rsize, credits;
>         size_t cur_len;
>         int rc;
>         pid_t pid;
> @@ -2931,13 +2931,19 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
>                 pid = current->tgid;
>
>         do {
> -               cur_len = min_t(const size_t, len, cifs_sb->rsize);
> +               rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize,
> +                                                  &rsize, &credits);
> +               if (rc)
> +                       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,
>                                             cifs_uncached_readv_complete);
>                 if (!rdata) {
> +                       add_credits_and_wake_if(server, credits, 0);
>                         rc = -ENOMEM;
>                         break;
>                 }
> @@ -2953,12 +2959,14 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
>                 rdata->pid = pid;
>                 rdata->pagesz = PAGE_SIZE;
>                 rdata->read_into_pages = cifs_uncached_read_into_pages;
> +               rdata->credits = credits;
>
>                 if (!rdata->cfile->invalidHandle ||
>                     !cifs_reopen_file(rdata->cfile, true))
>                         rc = server->ops->async_readv(rdata);
>  error:
>                 if (rc) {
> +                       add_credits_and_wake_if(server, rdata->credits, 0);
>                         kref_put(&rdata->refcount,
>                                  cifs_uncached_readdata_release);
>                         if (rc == -EAGAIN)
> @@ -3458,10 +3466,16 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>          * the rdata->pages, then we want them in increasing order.
>          */
>         while (!list_empty(page_list)) {
> -               unsigned int i, nr_pages, bytes;
> +               unsigned int i, nr_pages, bytes, rsize;
>                 loff_t offset;
>                 struct page *page, *tpage;
>                 struct cifs_readdata *rdata;
> +               unsigned credits;
> +
> +               rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize,
> +                                                  &rsize, &credits);
> +               if (rc)
> +                       break;
>
>                 /*
>                  * Give up immediately if rsize is too small to read an entire
> @@ -3469,13 +3483,17 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                  * reach this point however since we set ra_pages to 0 when the
>                  * rsize is smaller than a cache page.
>                  */
> -               if (unlikely(cifs_sb->rsize < PAGE_CACHE_SIZE))
> +               if (unlikely(rsize < PAGE_CACHE_SIZE)) {
> +                       add_credits_and_wake_if(server, credits, 0);
>                         return 0;
> +               }
>
> -               rc = readpages_get_pages(mapping, page_list, cifs_sb->rsize,
> -                                        &tmplist, &nr_pages, &offset, &bytes);
> -               if (rc)
> +               rc = readpages_get_pages(mapping, page_list, rsize, &tmplist,
> +                                        &nr_pages, &offset, &bytes);
> +               if (rc) {
> +                       add_credits_and_wake_if(server, credits, 0);
>                         break;
> +               }
>
>                 rdata = cifs_readdata_alloc(nr_pages, cifs_readv_complete);
>                 if (!rdata) {
> @@ -3487,6 +3505,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                                 page_cache_release(page);
>                         }
>                         rc = -ENOMEM;
> +                       add_credits_and_wake_if(server, credits, 0);
>                         break;
>                 }
>
> @@ -3497,6 +3516,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                 rdata->pid = pid;
>                 rdata->pagesz = PAGE_CACHE_SIZE;
>                 rdata->read_into_pages = cifs_readpages_read_into_pages;
> +               rdata->credits = credits;
>
>                 list_for_each_entry_safe(page, tpage, &tmplist, lru) {
>                         list_del(&page->lru);
> @@ -3507,6 +3527,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                     !cifs_reopen_file(rdata->cfile, true))
>                         rc = server->ops->async_readv(rdata);
>                 if (rc) {
> +                       add_credits_and_wake_if(server, rdata->credits, 0);
>                         for (i = 0; i < rdata->nr_pages; i++) {
>                                 page = rdata->pages[i];
>                                 lru_cache_add_file(page);
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index fecc2de..081529f 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -242,8 +242,6 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
>         /* start with specified rsize, or default */
>         rsize = volume_info->rsize ? volume_info->rsize : CIFS_DEFAULT_IOSIZE;
>         rsize = min_t(unsigned int, rsize, server->max_read);
> -       /* set it to the maximum buffer size value we can send with 1 credit */
> -       rsize = min_t(unsigned int, rsize, SMB2_MAX_BUFFER_SIZE);
>
>         return rsize;
>  }
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index a1d89b7..26624ee 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1758,11 +1758,12 @@ smb2_readv_callback(struct mid_q_entry *mid)
>  int
>  smb2_async_readv(struct cifs_readdata *rdata)
>  {
> -       int rc;
> +       int rc, flags = 0;
>         struct smb2_hdr *buf;
>         struct cifs_io_parms io_parms;
>         struct smb_rqst rqst = { .rq_iov = &rdata->iov,
>                                  .rq_nvec = 1 };
> +       struct TCP_Server_Info *server;
>
>         cifs_dbg(FYI, "%s: offset=%llu bytes=%u\n",
>                  __func__, rdata->offset, rdata->bytes);
> @@ -1773,18 +1774,41 @@ smb2_async_readv(struct cifs_readdata *rdata)
>         io_parms.persistent_fid = rdata->cfile->fid.persistent_fid;
>         io_parms.volatile_fid = rdata->cfile->fid.volatile_fid;
>         io_parms.pid = rdata->pid;
> +
> +       server = io_parms.tcon->ses->server;
> +
>         rc = smb2_new_read_req(&rdata->iov, &io_parms, 0, 0);
> -       if (rc)
> +       if (rc) {
> +               if (rc == -EAGAIN && rdata->credits) {
> +                       /* credits was reseted by reconnect */
> +                       rdata->credits = 0;
> +                       /* reduce in_flight value since we won't send the req */
> +                       spin_lock(&server->req_lock);
> +                       server->in_flight--;
> +                       spin_unlock(&server->req_lock);
> +               }
>                 return rc;
> +       }
>
>         buf = (struct smb2_hdr *)rdata->iov.iov_base;
>         /* 4 for rfc1002 length field */
>         rdata->iov.iov_len = get_rfc1002_length(rdata->iov.iov_base) + 4;
>
> +       if (rdata->credits) {
> +               buf->CreditCharge = cpu_to_le16(DIV_ROUND_UP(rdata->bytes,
> +                                               SMB2_MAX_BUFFER_SIZE));
> +               spin_lock(&server->req_lock);
> +               server->credits += rdata->credits -
> +                                               le16_to_cpu(buf->CreditCharge);
> +               spin_unlock(&server->req_lock);
> +               wake_up(&server->request_q);
> +               flags = CIFS_HAS_CREDITS;
> +       }
> +
>         kref_get(&rdata->refcount);
>         rc = cifs_call_async(io_parms.tcon->ses->server, &rqst,
>                              cifs_readv_receive, smb2_readv_callback,
> -                            rdata, 0);
> +                            rdata, flags);
>         if (rc) {
>                 kref_put(&rdata->refcount, cifs_readdata_release);
>                 cifs_stats_fail_inc(io_parms.tcon, SMB2_READ_HE);
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 16/16] CIFS: Use multicredits for SMB 2.1/3 reads

[Pavel Shilovsky]

2014-07-27 17:05 GMT+04:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

Looks like the patch has the same 's/reseted/reset/' typo - will fix
in the git tree.

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 00/16] SMB 2.1/3 multicredit requests for reading/writing

[Stefan Metzmacher]

Hi Pavel,

> The goal of this patchset is to add large MTU/multicredits support for reading and writing from the CIFS client to servers > that support SMB 2.1 and higher versions of the protocol.

Are looking at the returned capabilities from the server or just the
dialect?
The answer should really be the capability, it's wrong to rely on the
dialect...

metze

Re: [PATCH v3 00/16] SMB 2.1/3 multicredit requests for reading/writing

[Pavel Shilovsky]

2014-07-29 0:14 GMT+04:00 Stefan Metzmacher <metze-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
> Hi Pavel,
>
>> The goal of this patchset is to add large MTU/multicredits support for reading and writing from the CIFS client to servers > that support SMB 2.1 and higher versions of the protocol.
>
> Are looking at the returned capabilities from the server or just the
> dialect?
> The answer should really be the capability, it's wrong to rely on the
> dialect...
>
> metze

Hi Stefan,

The code of the patchset is looking at the returned
MaxReadSize/MaxWriteSize values from the server. If they are greater
than 64K, the module uses multicredit requests. Also it only works for
SMB 2.1 and higher versions and is disabled for SMB2.0.

The spec says (http://msdn.microsoft.com/en-us/library/cc246768.aspx):

"If the common dialect is SMB 2.1 or 3.x dialect family and the
underlying connection is either TCP port 445 or RDMA,
Connection.SupportsMultiCreditMUST be set to TRUE; otherwise, it MUST
be set to FALSE."

So, it seems like the server must should multicredit requests if
negotiating SMB 2.1 or 3.x protocols.

Anyway, if the server sets MaxRead(Write)Size value to more than 64K
and does not support multicredit requests, this server looks like a
broken one.

I can add a check for MTU capability bit for SMB 2.1 and 3.x protocols
before the check of MaxRead(Write)Size value and let the code fall
back to the 64K per request if this bit is missed.

Thoughts?

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 00/16] SMB 2.1/3 multicredit requests for reading/writing

[Stefan (metze) Metzmacher]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

Hi Pavel,

> 2014-07-29 0:14 GMT+04:00 Stefan Metzmacher <metze-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
>> Hi Pavel,
>>
>>> The goal of this patchset is to add large MTU/multicredits support for reading and writing from the CIFS client to servers > that support SMB 2.1 and higher versions of the protocol.
>>
>> Are looking at the returned capabilities from the server or just the
>> dialect?
>> The answer should really be the capability, it's wrong to rely on the
>> dialect...
>>
>> metze
> 
> Hi Stefan,
> 
> The code of the patchset is looking at the returned
> MaxReadSize/MaxWriteSize values from the server. If they are greater
> than 64K, the module uses multicredit requests. Also it only works for
> SMB 2.1 and higher versions and is disabled for SMB2.0.
> 
> The spec says (http://msdn.microsoft.com/en-us/library/cc246768.aspx):
> 
> "If the common dialect is SMB 2.1 or 3.x dialect family and the
> underlying connection is either TCP port 445 or RDMA,
> Connection.SupportsMultiCreditMUST be set to TRUE; otherwise, it MUST
> be set to FALSE."
> 
> So, it seems like the server must should multicredit requests if
> negotiating SMB 2.1 or 3.x protocols.
> 
> Anyway, if the server sets MaxRead(Write)Size value to more than 64K
> and does not support multicredit requests, this server looks like a
> broken one.
> 
> I can add a check for MTU capability bit for SMB 2.1 and 3.x protocols
> before the check of MaxRead(Write)Size value and let the code fall
> back to the 64K per request if this bit is missed.

I'd prefer to have this extra check, you never know if some vendors are
crazy
enough to run Samba 4.0.0alpha* release. Samba got SMB2.1 support in
September 2011
and multi-credit support in February 2012.

metze



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 246 bytes --]

Re: [PATCH v3 00/16] SMB 2.1/3 multicredit requests for reading/writing

[Pavel Shilovsky]

2014-07-29 11:40 GMT+04:00 Stefan (metze) Metzmacher <metze-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
> Hi Pavel,
>
> I'd prefer to have this extra check, you never know if some vendors are
> crazy
> enough to run Samba 4.0.0alpha* release. Samba got SMB2.1 support in
> September 2011
> and multi-credit support in February 2012.

Ok - will update my patches. Thank you for pointing it out!

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v3 00/16] SMB 2.1/3 multicredit requests for reading/writing

[Stefan (metze) Metzmacher]

[-- Attachment #1: Type: text/plain, Size: 484 bytes --]

Am 29.07.2014 10:02, schrieb Pavel Shilovsky:
> 2014-07-29 11:40 GMT+04:00 Stefan (metze) Metzmacher <metze-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
>> Hi Pavel,
>>
>> I'd prefer to have this extra check, you never know if some vendors are
>> crazy
>> enough to run Samba 4.0.0alpha* release. Samba got SMB2.1 support in
>> September 2011
>> and multi-credit support in February 2012.
> 
> Ok - will update my patches. Thank you for pointing it out!

Thanks!

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 246 bytes --]