All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Steve French <smfrench@gmail.com>
Cc: dhowells@redhat.com, Vishal Moola <vishal.moola@gmail.com>,
	Shyam Prasad N <nspmangalore@gmail.com>,
	Rohith Surabattula <rohiths.msft@gmail.com>,
	Tom Talpey <tom@talpey.com>, Stefan Metzmacher <metze@samba.org>,
	Paulo Alcantara <pc@cjr.nz>, Jeff Layton <jlayton@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Marc Dionne <marc.dionne@auristor.com>,
	linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: cifs test patch to make cifs use its own version of write_cache_pages()
Date: Thu, 02 Mar 2023 23:41:39 +0000	[thread overview]
Message-ID: <522532.1677800499@warthog.procyon.org.uk> (raw)
In-Reply-To: <522331.1677800209@warthog.procyon.org.uk>

David Howells <dhowells@redhat.com> wrote:

> And then CIFS. ...
> 
>   Base + Own write_cache_pages():
> 	WRITE: bw=451MiB/s (473MB/s), 113MiB/s-113MiB/s (118MB/s-118MB/s)
> 	WRITE: bw=455MiB/s (478MB/s), 114MiB/s-114MiB/s (119MB/s-120MB/s)
> 	WRITE: bw=453MiB/s (475MB/s), 113MiB/s-113MiB/s (119MB/s-119MB/s)
> 	WRITE: bw=459MiB/s (481MB/s), 115MiB/s-115MiB/s (120MB/s-120MB/s)

Here's my patch to give cifs its own copy of write_cache_pages() so that the
function pointer can be eliminated in case some sort of spectre thing is
causing a slowdown.

This goes on top of "cifs test patch to convert to using write_cache_pages()".

David
---
 fs/cifs/file.c |  137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 136 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 04e2466609d9..c33c7db729c7 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2802,6 +2802,141 @@ static int cifs_writepages_add_folio(struct folio *folio,
 	return 0;
 }
 
+static int cifs_write_cache_pages(struct address_space *mapping,
+				  struct writeback_control *wbc,
+				  struct cifs_writepages_context *ctx)
+{
+	int ret = 0;
+	int done = 0;
+	int error;
+	struct folio_batch fbatch;
+	int nr_folios;
+	pgoff_t index;
+	pgoff_t end;		/* Inclusive */
+	pgoff_t done_index;
+	int range_whole = 0;
+	xa_mark_t tag;
+
+	folio_batch_init(&fbatch);
+	if (wbc->range_cyclic) {
+		index = mapping->writeback_index; /* prev offset */
+		end = -1;
+	} else {
+		index = wbc->range_start >> PAGE_SHIFT;
+		end = wbc->range_end >> PAGE_SHIFT;
+		if (wbc->range_start == 0 && wbc->range_end == LLONG_MAX)
+			range_whole = 1;
+	}
+	if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages) {
+		tag_pages_for_writeback(mapping, index, end);
+		tag = PAGECACHE_TAG_TOWRITE;
+	} else {
+		tag = PAGECACHE_TAG_DIRTY;
+	}
+	done_index = index;
+	while (!done && (index <= end)) {
+		int i;
+
+		nr_folios = filemap_get_folios_tag(mapping, &index, end,
+				tag, &fbatch);
+
+		if (nr_folios == 0)
+			break;
+
+		for (i = 0; i < nr_folios; i++) {
+			struct folio *folio = fbatch.folios[i];
+
+			done_index = folio->index;
+
+			folio_lock(folio);
+
+			/*
+			 * Page truncated or invalidated. We can freely skip it
+			 * then, even for data integrity operations: the page
+			 * has disappeared concurrently, so there could be no
+			 * real expectation of this data integrity operation
+			 * even if there is now a new, dirty page at the same
+			 * pagecache address.
+			 */
+			if (unlikely(folio->mapping != mapping)) {
+continue_unlock:
+				folio_unlock(folio);
+				continue;
+			}
+
+			if (!folio_test_dirty(folio)) {
+				/* someone wrote it for us */
+				goto continue_unlock;
+			}
+
+			if (folio_test_writeback(folio)) {
+				if (wbc->sync_mode != WB_SYNC_NONE)
+					folio_wait_writeback(folio);
+				else
+					goto continue_unlock;
+			}
+
+			BUG_ON(folio_test_writeback(folio));
+			if (!folio_clear_dirty_for_io(folio))
+				goto continue_unlock;
+
+			error = cifs_writepages_add_folio(folio, wbc, ctx);
+			if (unlikely(error)) {
+				/*
+				 * Handle errors according to the type of
+				 * writeback. There's no need to continue for
+				 * background writeback. Just push done_index
+				 * past this page so media errors won't choke
+				 * writeout for the entire file. For integrity
+				 * writeback, we must process the entire dirty
+				 * set regardless of errors because the fs may
+				 * still have state to clear for each page. In
+				 * that case we continue processing and return
+				 * the first error.
+				 */
+				if (error == AOP_WRITEPAGE_ACTIVATE) {
+					folio_unlock(folio);
+					error = 0;
+				} else if (wbc->sync_mode != WB_SYNC_ALL) {
+					ret = error;
+					done_index = folio->index +
+						folio_nr_pages(folio);
+					done = 1;
+					break;
+				}
+				if (!ret)
+					ret = error;
+			}
+
+			/*
+			 * We stop writing back only if we are not doing
+			 * integrity sync. In case of integrity sync we have to
+			 * keep going until we have written all the pages
+			 * we tagged for writeback prior to entering this loop.
+			 */
+			if (--wbc->nr_to_write <= 0 &&
+			    wbc->sync_mode == WB_SYNC_NONE) {
+				done = 1;
+				break;
+			}
+		}
+		folio_batch_release(&fbatch);
+		cond_resched();
+	}
+
+	/*
+	 * If we hit the last page and there is more work to be done: wrap
+	 * back the index back to the start of the file for the next
+	 * time we are called.
+	 */
+	if (wbc->range_cyclic && !done)
+		done_index = 0;
+	if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
+		mapping->writeback_index = done_index;
+
+	return ret;
+}
+
 /*
  * Write some of the pending data back to the server
  */
@@ -2816,7 +2951,7 @@ static int cifs_writepages(struct address_space *mapping,
 	 * to prevent it.
 	 */
 
-	ret = write_cache_pages(mapping, wbc, cifs_writepages_add_folio, &ctx);
+	ret = cifs_write_cache_pages(mapping, wbc, &ctx);
 	if (ret >= 0 && ctx.begun) {
 		ret = cifs_writepages_submit(mapping, wbc, &ctx);
 		if (ret < 0)


      reply	other threads:[~2023-03-02 23:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-02 23:16 [PATCH 0/3] smb3, afs: Revert changes to {cifs,afs}_writepages_region() David Howells
2023-03-02 23:16 ` [PATCH 1/3] mm: Add a function to get a single tagged folio from a file David Howells
2023-03-02 23:21   ` Matthew Wilcox
2023-03-02 23:16 ` [PATCH 2/3] afs: Partially revert and use filemap_get_folio_tag() David Howells
2023-03-02 23:16 ` [PATCH 3/3] cifs: " David Howells
2023-03-02 23:20 ` [PATCH 0/3] smb3, afs: Revert changes to {cifs,afs}_writepages_region() David Howells
2023-03-02 23:23 ` Test patch to remove per-page dirty region tracking from afs David Howells
2023-03-02 23:29   ` Test patch to make afs use write_cache_pages() David Howells
2023-03-02 23:32     ` Test patch to make afs use its own version of write_cache_pages() David Howells
2023-03-02 23:36 ` cifs test patch to convert to using write_cache_pages() David Howells
2023-03-02 23:41   ` David Howells [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=522532.1677800499@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=jlayton@kernel.org \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    --cc=metze@samba.org \
    --cc=nspmangalore@gmail.com \
    --cc=pc@cjr.nz \
    --cc=rohiths.msft@gmail.com \
    --cc=smfrench@gmail.com \
    --cc=tom@talpey.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vishal.moola@gmail.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.