On Feb 18, 2021, at 2:51 AM, Lukas Czerner <lczerner@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
> ---
> 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
>
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
