[PATCH 2/2] HFS: clear dirty flags on remount read-only

[PATCH 2/2] HFS: clear dirty flags on remount read-only

[Timo Schlüßler]

Remounting a HFS filesystem read-only doesn't clear the "DIRTY"-flags (not
HFS_SB_ATTRIB_UNMNT and HFS_SB_ATTRIB_INCNSTNT) correctly. Subsequent
mounts then report a dirty filesystem and enforce a check before willing to
mount it read-write again.
Signed-off-by: Timo Schlüßler <timo@schluessler.org>
---

--- linux/fs/hfs/super.c.orig   2015-11-29 14:42:22.000000000 +0100
+++ linux/fs/hfs/super.c        2015-11-29 15:22:03.000000000 +0100
@@ -113,11 +113,13 @@ static int hfs_statfs(struct dentry *den

 static int hfs_remount(struct super_block *sb, int *flags, char *data)
 {
-       sync_filesystem(sb);
        *flags |= MS_NODIRATIME;
-       if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
+       if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY)) {
+               sync_filesystem(sb);
                return 0;
+       }
        if (!(*flags & MS_RDONLY)) {
+               sync_filesystem(sb);
                if (!(HFS_SB(sb)->mdb->drAtrb & cpu_to_be16(HFS_SB_ATTRIB_UNMNT))) {
                        pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended.  leaving read-only.\n");
                        sb->s_flags |= MS_RDONLY;
@@ -127,6 +129,9 @@ static int hfs_remount(struct super_bloc
                        sb->s_flags |= MS_RDONLY;
                        *flags |= MS_RDONLY;
                }
+       } else {
+               hfs_mdb_close(sb);
+               sync_filesystem(sb);
        }
        return 0;
 }

Re: [PATCH 2/2] HFS: clear dirty flags on remount read-only

[Vyacheslav Dubeyko]

Hi Timo,

On Sun, 2015-11-29 at 15:59 +0100, Timo Schlüßler wrote:
> Remounting a HFS filesystem read-only doesn't clear the "DIRTY"-flags (not
> HFS_SB_ATTRIB_UNMNT and HFS_SB_ATTRIB_INCNSTNT) correctly. Subsequent
> mounts then report a dirty filesystem and enforce a check before willing to
> mount it read-write again.
> Signed-off-by: Timo Schlüßler <timo@schluessler.org>
> ---
> 
> --- linux/fs/hfs/super.c.orig   2015-11-29 14:42:22.000000000 +0100
> +++ linux/fs/hfs/super.c        2015-11-29 15:22:03.000000000 +0100
> @@ -113,11 +113,13 @@ static int hfs_statfs(struct dentry *den
> 
>  static int hfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> -       sync_filesystem(sb);

I think that this is the right placement for sync_filesystem() call.
It's much better to have such call in one place. Anyway, you need to set
HFS_SB_ATTRIB_UNMNT and to clear HFS_SB_ATTRIB_INCNSTNT for remount from
READ-WRITE to READ-ONLY state. And vou need to do opposite operation for
remount from READ-ONLY to READ-WRITE state. So, it's much better ti
leave sync_filesystem() call in the same place but to add additional
logic before this call.

Thanks,
Vyacheslav Dubeyko.

>         *flags |= MS_NODIRATIME;
> -       if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
> +       if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY)) {
> +               sync_filesystem(sb);
>                 return 0;
> +       }
>         if (!(*flags & MS_RDONLY)) {
> +               sync_filesystem(sb);
>                 if (!(HFS_SB(sb)->mdb->drAtrb & cpu_to_be16(HFS_SB_ATTRIB_UNMNT))) {
>                         pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended.  leaving read-only.\n");
>                         sb->s_flags |= MS_RDONLY;
> @@ -127,6 +129,9 @@ static int hfs_remount(struct super_bloc
>                         sb->s_flags |= MS_RDONLY;
>                         *flags |= MS_RDONLY;
>                 }
> +       } else {
> +               hfs_mdb_close(sb);
> +               sync_filesystem(sb);
>         }
>         return 0;
>  }
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] HFS: clear dirty flags on remount read-only

[Timo Schlüßler]

Hello Vyacheslav,

On 30.11.2015 00:03, Vyacheslav Dubeyko wrote:
> Hi Timo,
>
> I think that this is the right placement for sync_filesystem() call.
> It's much better to have such call in one place. Anyway, you need to set
> HFS_SB_ATTRIB_UNMNT and to clear HFS_SB_ATTRIB_INCNSTNT for remount from
> READ-WRITE to READ-ONLY state. And vou need to do opposite operation for
> remount from READ-ONLY to READ-WRITE state. So, it's much better ti
> leave sync_filesystem() call in the same place but to add additional
> logic before this call.
>
> Thanks,
> Vyacheslav Dubeyko.
>

I agree with you and will provide an updated patch for this.

Best regards
Timo Schlüßler