[PATCH] ext4: fix handling mount -o remount,nolazytime

[PATCH] ext4: fix handling mount -o remount,nolazytime

[Konstantin Khlebnikov]

Tool "mount" from util-linux >= 2.27 knows about flag MS_LAZYTIME and
handles options "lazytime" and "nolazytime" as fs-independent.

For ext4 it works for enabling lazytime: mount(MS_REMOUNT | MS_LAZYTIME),
but does not work for disabling: mount(MS_REMOUNT).

Currently ext4 has performance issue in lazytime implementation caused by
contention around inode_hash_lock in ext4_update_other_inodes_time().

Fortunately lazytime still could be disabled without unmounting by passing
"nolazytime" as fs-specific mount option: mount(MS_REMOUNT, "nolazytime").
But modern versions of tool "mount" cannot do that.

This patch fixes remount for modern tool and keeps backward compatibility.

Fixes: a2fd66d069d8 ("ext4: set lazytime on remount if MS_LAZYTIME is set by mount")
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Link: https://lore.kernel.org/lkml/158040603451.1879.7954684107752709143.stgit@buzz/
---
 fs/ext4/super.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f464dff09774..c901dc957b97 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5339,6 +5339,9 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	if (sbi->s_journal && sbi->s_journal->j_task->io_context)
 		journal_ioprio = sbi->s_journal->j_task->io_context->ioprio;
 
+	if (!(*flags & SB_LAZYTIME))
+		sb->s_flags &= ~SB_LAZYTIME;
+
 	if (!parse_options(data, sb, NULL, &journal_ioprio, 1)) {
 		err = -EINVAL;
 		goto restore_opts;

Re: [PATCH] ext4: fix handling mount -o remount,nolazytime

[Theodore Y. Ts'o]

On Wed, Feb 19, 2020 at 12:19:52PM +0300, Konstantin Khlebnikov wrote:
> Tool "mount" from util-linux >= 2.27 knows about flag MS_LAZYTIME and
> handles options "lazytime" and "nolazytime" as fs-independent.
> 
> For ext4 it works for enabling lazytime: mount(MS_REMOUNT | MS_LAZYTIME),
> but does not work for disabling: mount(MS_REMOUNT).
> 
> Currently ext4 has performance issue in lazytime implementation caused by
> contention around inode_hash_lock in ext4_update_other_inodes_time().
> 
> Fortunately lazytime still could be disabled without unmounting by passing
> "nolazytime" as fs-specific mount option: mount(MS_REMOUNT, "nolazytime").
> But modern versions of tool "mount" cannot do that.
> 
> This patch fixes remount for modern tool and keeps backward compatibility.

Actually, if you are using ancient versions of mount that don't know
about MS_LAZYTIME, then when you do something like mount -o
remount,usrquota /dev/sdb" with your patch, it will disable
MS_LAZYTIME, which would be a backwards incompatible change.

So if we make this change, and there is someone who wants to use
lazytime on some ancient enterprise linux system which is still using
an old version of util-linux, and then take a kernel with this change,
then it will result in a change in the behavior they will see.  The
good news is that RHEL 8 is using util-linux 2.32, but RHEL 7 is still
using util-linux 2.23.

Lazytime is not enabled by default, so this issue is really only a
problem for someone which explicitly enables lazytime using a newer
version of util-linux, and then disables lazytime with a newer version
of util-linux.  So the behaviour of a2fd66d069d8 ("ext4: set lazytime
on remount if MS_LAZYTIME is set by mount") was in fact an explicit
decision to do things in that way.

So maybe we might want to change things, assuming that it's unlikely
users will try to be running new kernels on ancient distros.  But I
really wouldn't want to add a Fixes tag, and I would want to make sure
this doesn't get backported to older kernels, since the change does
*not* keep backwards compatibility.

Unfortunately, it's not possible to do this without breaking
compatibility for at least some systems.  The question is whether or
not we think systems running util-linux less than 2.27 is something we
care about for new kernels.  Times may have changed since
a2fd66d069d8.

So I might be willing to take this patch (I invite comments from
others), but there will need to be a DO NOT BACKPORT warning in the
commit description.

Cheers,

						- Ted

Re: [PATCH] ext4: fix handling mount -o remount,nolazytime

[Konstantin Khlebnikov]

On 19/02/2020 19.22, Theodore Y. Ts'o wrote:
> On Wed, Feb 19, 2020 at 12:19:52PM +0300, Konstantin Khlebnikov wrote:
>> Tool "mount" from util-linux >= 2.27 knows about flag MS_LAZYTIME and
>> handles options "lazytime" and "nolazytime" as fs-independent.
>>
>> For ext4 it works for enabling lazytime: mount(MS_REMOUNT | MS_LAZYTIME),
>> but does not work for disabling: mount(MS_REMOUNT).
>>
>> Currently ext4 has performance issue in lazytime implementation caused by
>> contention around inode_hash_lock in ext4_update_other_inodes_time().
>>
>> Fortunately lazytime still could be disabled without unmounting by passing
>> "nolazytime" as fs-specific mount option: mount(MS_REMOUNT, "nolazytime").
>> But modern versions of tool "mount" cannot do that.
>>
>> This patch fixes remount for modern tool and keeps backward compatibility.
> 
> Actually, if you are using ancient versions of mount that don't know
> about MS_LAZYTIME, then when you do something like mount -o
> remount,usrquota /dev/sdb" with your patch, it will disable
> MS_LAZYTIME, which would be a backwards incompatible change.
> 
> So if we make this change, and there is someone who wants to use
> lazytime on some ancient enterprise linux system which is still using
> an old version of util-linux, and then take a kernel with this change,
> then it will result in a change in the behavior they will see.  The
> good news is that RHEL 8 is using util-linux 2.32, but RHEL 7 is still
> using util-linux 2.23.
> 
> Lazytime is not enabled by default, so this issue is really only a
> problem for someone which explicitly enables lazytime using a newer
> version of util-linux, and then disables lazytime with a newer version
> of util-linux.  So the behaviour of a2fd66d069d8 ("ext4: set lazytime
> on remount if MS_LAZYTIME is set by mount") was in fact an explicit
> decision to do things in that way.
> 
> So maybe we might want to change things, assuming that it's unlikely
> users will try to be running new kernels on ancient distros.  But I
> really wouldn't want to add a Fixes tag, and I would want to make sure
> this doesn't get backported to older kernels, since the change does
> *not* keep backwards compatibility.
> 
> Unfortunately, it's not possible to do this without breaking
> compatibility for at least some systems.  The question is whether or
> not we think systems running util-linux less than 2.27 is something we
> care about for new kernels.  Times may have changed since
> a2fd66d069d8.
> 
> So I might be willing to take this patch (I invite comments from
> others), but there will need to be a DO NOT BACKPORT warning in the
> commit description.

Usually all these options are saved in /etc/fstab and
mount -o remount,... includes them into line passed into syscall.
In this case remounting any other option will not disable lazytime.

But there might be implementations of /bin/mount which doesn't do that.

> 
> Cheers,
> 
> 						- Ted
> 

Re: [PATCH] ext4: fix handling mount -o remount,nolazytime

[Theodore Y. Ts'o]

On Thu, Feb 20, 2020 at 06:11:04PM +0300, Konstantin Khlebnikov wrote:
> Usually all these options are saved in /etc/fstab and
> mount -o remount,... includes them into line passed into syscall.
> In this case remounting any other option will not disable lazytime.

This assumes that there *is* an /etc/fstab entry.  Sometimes system
administrators will perform ad hoc mounts of devices that aren't
mentioned in /etc/fstab.

For example, consider how xfstests works; it will run mounts commands
mentioning the test and scratch devices which are not mentioned on
/etc/fstab.


						- Ted