On Mon, Dec 20, 2021 at 12:53 PM Lukas Czerner <lczerner@xxxxxxxxxx> wrote: > > On Fri, Dec 17, 2021 at 07:26:30PM +0100, Heiner Kallweit wrote: > > On 17.12.2021 18:02, Heiner Kallweit wrote: > > > On 17.12.2021 16:34, Heiner Kallweit wrote: > > >> On 17.12.2021 16:24, Lukas Czerner wrote: > > >>> On Fri, Dec 17, 2021 at 04:11:32PM +0100, Heiner Kallweit wrote: > > >>>> On linux-next systemd-remount-fs complains about an invalid mount option > > >>>> here, resulting in a r/o root fs. After playing with the mount options > > >>>> it turned out that data=ordered causes the problem. linux-next from Dec > > >>>> 1st was ok, so it seems to be related to the new mount API patches. > > >>>> > > >>>> At a first glance I saw no obvious problem, the following looks good. > > >>>> Maybe you have an idea where to look .. > > >>>> > > >>>> static const struct constant_table ext4_param_data[] = { > > >>>> {"journal", EXT4_MOUNT_JOURNAL_DATA}, > > >>>> {"ordered", EXT4_MOUNT_ORDERED_DATA}, > > >>>> {"writeback", EXT4_MOUNT_WRITEBACK_DATA}, > > >>>> {} > > >>>> }; > > >>>> > > >>>> fsparam_enum ("data", Opt_data, ext4_param_data), > > >>>> > > >>> > > >>> Thank you for the report! > > >>> > > >>> The ext4 mount has been reworked to use the new mount api and the work > > >>> has been applied to linux-next couple of days ago so I definitelly > > >>> assume there is a bug in there that I've missed. I will be looking into > > >>> it. > > >>> > > >>> Can you be a little bit more specific about how to reproduce the problem > > >>> as well as the error it generates in the logs ? Any other mount options > > >>> used simultaneously, non-default file system features, or mount options > > >>> stored within the superblock ? > > >>> > > >>> Can you reproduce it outside of the systemd unit, say a script ? > > >>> > > >> Yes: > > >> > > >> [root@zotac ~]# mount -o remount,data=ordered / > > >> mount: /: mount point not mounted or bad option. > > >> [root@zotac ~]# mount -o remount,discard / > > >> [root@zotac ~]# > > >> > > >> "systemctl status systemd-remount-fs" shows the same error. > > >> > > >> Following options I had in my fstab (ext4 fs): > > >> rw,relatime,data=ordered,discard > > >> > > >> No non-default system features. > > >> > > >>> Thanks! > > >>> -Lukas > > >>> > > >> Heiner > > > > > > Sorry, should have looked at dmesg earlier. There I see: > > > EXT4-fs: Cannot change data mode on remount > > > Message seems to be triggered from ext4_check_opt_consistency(). > > > Not sure why this error doesn't occur with old mount API. > > > And actually I don't change the data mode. > > > > Based on the old API code: Maybe we need something like this? > > At least it works for me. > > > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > index b72d989b7..9ec7e526c 100644 > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > @@ -2821,7 +2821,9 @@ static int ext4_check_opt_consistency(struct fs_context *fc, > > "Remounting file system with no journal " > > "so ignoring journalled data option"); > > ctx_clear_mount_opt(ctx, EXT4_MOUNT_DATA_FLAGS); > > - } else if (ctx->mask_s_mount_opt & EXT4_MOUNT_DATA_FLAGS) { > > + } else if (ctx->mask_s_mount_opt & EXT4_MOUNT_DATA_FLAGS && > > + (ctx->vals_s_mount_opt & EXT4_MOUNT_DATA_FLAGS) != > > + (sbi->s_mount_opt & EXT4_MOUNT_DATA_FLAGS)) { > > Hi, > > indeed that's where the problem is. It's not enogh to check whether > we have a data= mount options set, we also have to check whether it's > the same as it already is set on the file system during remount. In > which case we just ignore it, rather then error out. > > Thanks for tracking it down. I think the condition can be simplified a > bit. I also have to update the xfstest test to check for plain remount > without changing anything to catch errors like these. I'll send patch > soon. > Is "ext4: don't fail remount if journalling mode didn't change" the fix for the issue reported by Heiner? - Sedat - [1] https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?h=dev&id=4c2467287779f744cdd70c8ec70903034d6584f0 > Thanks! > -Lukas > > > ext4_msg(NULL, KERN_ERR, "Cannot change data mode " > > "on remount"); > > return -EINVAL; > > >