linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Czerner <lczerner@redhat.com>
To: linux-ext4@vger.kernel.org
Cc: Eric Sandeen <sandeen@redhat.com>
Subject: [PATCH v2] mmp: do not use O_DIRECT when working with regular file
Date: Thu, 18 Feb 2021 10:51:46 +0100	[thread overview]
Message-ID: <20210218095146.265302-1-lczerner@redhat.com> (raw)
In-Reply-To: <20210212093719.162065-1-lczerner@redhat.com>

Currently the mmp block is read using O_DIRECT to avoid any caching that
may be done by the VM. However when working with regular files this
creates alignment issues when the device of the host file system has
sector size larger than the blocksize of the file system in the file
we're working with.

This can be reproduced with t_mmp_fail test when run on the device with
4k sector size because the mke2fs fails when trying to read the mmp
block.

Fix it by disabling O_DIRECT when working with regular files. I don't
think there is any risk of doing so since the file system layer, unlike
shared block device, should guarantee cache consistency.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
---
v2: Fix comment - it avoids problems when the sector size is larger not
    smaller than blocksize

 lib/ext2fs/mmp.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/ext2fs/mmp.c b/lib/ext2fs/mmp.c
index c21ae272..cca2873b 100644
--- a/lib/ext2fs/mmp.c
+++ b/lib/ext2fs/mmp.c
@@ -57,21 +57,21 @@ errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf)
 	 * regardless of how the io_manager is doing reads, to avoid caching of
 	 * the MMP block by the io_manager or the VM.  It needs to be fresh. */
 	if (fs->mmp_fd <= 0) {
+		struct stat st;
 		int flags = O_RDWR | O_DIRECT;
 
-retry:
+		/*
+		 * There is no reason for using O_DIRECT if we're working with
+		 * regular file. Disabling it also avoids problems with
+		 * alignment when the device of the host file system has sector
+		 * size larger than blocksize of the fs we're working with.
+		 */
+		if (stat(fs->device_name, &st) == 0 &&
+		    S_ISREG(st.st_mode))
+			flags &= ~O_DIRECT;
+
 		fs->mmp_fd = open(fs->device_name, flags);
 		if (fs->mmp_fd < 0) {
-			struct stat st;
-
-			/* Avoid O_DIRECT for filesystem image files if open
-			 * fails, since it breaks when running on tmpfs. */
-			if (errno == EINVAL && (flags & O_DIRECT) &&
-			    stat(fs->device_name, &st) == 0 &&
-			    S_ISREG(st.st_mode)) {
-				flags &= ~O_DIRECT;
-				goto retry;
-			}
 			retval = EXT2_ET_MMP_OPEN_DIRECT;
 			goto out;
 		}
-- 
2.26.2


  parent reply	other threads:[~2021-02-18 11:45 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-12  9:37 [PATCH] mmp: do not use O_DIRECT when working with regular file Lukas Czerner
2021-02-16 21:24 ` Eric Sandeen
2021-02-16 21:51   ` Lukas Czerner
2021-02-18  9:51 ` Lukas Czerner [this message]
2021-02-18 22:20   ` [PATCH v2] " Andreas Dilger
2021-02-19 10:08     ` Alexey Lyashkov
2021-02-19 10:57       ` Lukas Czerner
2021-02-19 11:49         ` Alexey Lyashkov
2021-02-19 13:34           ` Lukas Czerner
2021-02-19 13:53             ` Alexey Lyashkov
2021-02-19 14:41               ` Lukas Czerner
2021-02-19 16:18               ` Theodore Ts'o
2021-02-20 13:21                 ` Alexey Lyashkov

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=20210218095146.265302-1-lczerner@redhat.com \
    --to=lczerner@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=sandeen@redhat.com \
    /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).