linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mauricio Faria de Oliveira <mfo@canonical.com>
To: linux-ext4@vger.kernel.org, "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: dann frazier <dann.frazier@canonical.com>,
	Andreas Dilger <adilger@dilger.ca>, Jan Kara <jack@suse.com>
Subject: [RFC PATCH 03/11] ext4: data=journal: call ext4_force_commit() in ext4_writepages() for msync()
Date: Thu, 23 Apr 2020 20:36:57 -0300	[thread overview]
Message-ID: <20200423233705.5878-4-mfo@canonical.com> (raw)
In-Reply-To: <20200423233705.5878-1-mfo@canonical.com>

The data-integrity syscalls (not memory-cleansing writeback)
use file_write_and_wait_range() that wait_on_page_writeback()
for every page in the range after do_writepages().

If any of these pages is mmap'ed pagecache, i.e., goes into
__ext4_journalled_writepage(), with the last couple patches
end_page_writeback() will be done on (or, not be done until)
transaction commit, which can take seconds (commit interval,
max commit age), which delays msync().

Let's fix this so that msync() syscall should just return
quickly without a delay of up to a few seconds by default.

For data=journal the next thing these syscalls do anyway is
ext4_force_commit() (see ext4_sync_file()), which is needed
for the buffered pagecache, as __filemap_fdatawrite_range()
doesn't do anything: the buffers are clean, so it returns
early without calling do_writepages() / ext4_write_pages().
So it's not possible to just move/replace that call here.

(This is better/more correct than to use ext4_handle_sync()
for mmap'ed pagecache, which triggers one commit per page,
as synchronous transaction batching in jbd2 targets other,
concurrent tasks, but in this case one single task writes
all pages back serially.)

Now for memory-cleansing writeback, even though it is not
supposed to wait, we should not wait for seconds either,
as it could delay an upcoming data-integrity syscall in
write_cache_pages() on a call to wait_on_page_writeback().
(another fix is needed for such calls to ext4_writepage()).

So just do not check for wbc->sync_mode to cover it too.

Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
---
 fs/ext4/inode.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d385a11ba31e..574a062b8bcd 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2709,7 +2709,37 @@ static int ext4_writepages(struct address_space *mapping,
 		goto out_writepages;
 
 	if (ext4_should_journal_data(inode)) {
+		journal_t *journal = sbi->s_journal;
+
 		ret = generic_writepages(mapping, wbc);
+		/*
+		 * On the data-integrity syscalls, file_write_and_wait_range()
+		 * will wait on page writeback after calling ext4_writepages().
+		 * For mmaped pagecache that only ends on transaction commit,
+		 * which may take up to commit interval (seconds!) to happen.
+		 *
+		 * So, ensure that ext4_force_commit() happens before return,
+		 * and after all pages in the range are set_page_writeback(),
+		 * but only if needed (i.e. check for datasync transaction
+		 * set in the inode by __ext4_journalled_writepage().)
+		 *
+		 * Do it for memory-cleasing writeback too, because it might
+		 * delay another data-integrity syscall in write_cache_pages()
+		 * on wait_on_page_writeback().
+		 */
+		if (!ret && journal) {
+			bool force_commit = false;
+
+			read_lock(&journal->j_state_lock);
+			if (journal->j_running_transaction &&
+			    journal->j_running_transaction->t_tid ==
+				EXT4_I(inode)->i_datasync_tid)
+				force_commit = true;
+			read_unlock(&journal->j_state_lock);
+
+			if (force_commit)
+				ret = ext4_force_commit(inode->i_sb);
+		}
 		goto out_writepages;
 	}
 
-- 
2.20.1


  parent reply	other threads:[~2020-04-23 23:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-23 23:36 [RFC PATCH 00/11] ext4: data=journal: writeback mmap'ed pagecache Mauricio Faria de Oliveira
2020-04-23 23:36 ` [RFC PATCH 01/11] ext4: data=journal: introduce struct/kmem_cache ext4_journalled_wb_page/_cachep Mauricio Faria de Oliveira
2020-04-23 23:36 ` [RFC PATCH 02/11] ext4: data=journal: handle page writeback in __ext4_journalled_writepage() Mauricio Faria de Oliveira
2020-04-23 23:36 ` Mauricio Faria de Oliveira [this message]
2020-04-23 23:36 ` [RFC PATCH 04/11] ext4: data=journal: introduce helpers for journalled writeback deadlock Mauricio Faria de Oliveira
2020-04-23 23:36 ` [RFC PATCH 05/11] ext4: data=journal: prevent journalled writeback deadlock in __ext4_journalled_writepage() Mauricio Faria de Oliveira
2020-04-23 23:37 ` [RFC PATCH 06/11] ext4: data=journal: prevent journalled writeback deadlock in ext4_write_begin() Mauricio Faria de Oliveira
2020-04-23 23:37 ` [RFC PATCH 07/11] ext4: grab page before starting transaction handle in ext4_convert_inline_data_to_extent() Mauricio Faria de Oliveira
2020-04-23 23:37 ` [RFC PATCH 08/11] ext4: data=journal: prevent journalled writeback deadlock " Mauricio Faria de Oliveira
2020-04-23 23:37 ` [RFC PATCH 09/11] ext4: grab page before starting transaction handle in ext4_try_to_write_inline_data() Mauricio Faria de Oliveira
2020-04-23 23:37 ` [RFC PATCH 10/11] ext4: deduplicate code with error legs " Mauricio Faria de Oliveira
2020-04-23 23:37 ` [RFC PATCH 11/11] ext4: data=journal: prevent journalled writeback deadlock " Mauricio Faria de Oliveira
2020-05-15 18:39 ` [RFC PATCH 00/11] ext4: data=journal: writeback mmap'ed pagecache Mauricio Faria de Oliveira
2020-05-17  7:40   ` Andreas Dilger
2020-06-10 13:21 ` Jan Kara
2020-06-10 15:15   ` Mauricio Faria de Oliveira

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=20200423233705.5878-4-mfo@canonical.com \
    --to=mfo@canonical.com \
    --cc=adilger@dilger.ca \
    --cc=dann.frazier@canonical.com \
    --cc=jack@suse.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).