| Message ID | 158210399258.5335.3994877510070204710.stgit@buzz |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: fix handling mount -o remount,nolazytime | expand |
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
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 >
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
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;
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(+)