Skip to contentSkip to conversation

HFS: clear dirty flags on remount read-only

PATCH In review Timo Schlüßler 2 participants 3 messages Nov 29, 2015 – Dec 3, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

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.

---

fs/hfs/super.c 2015-11-29 15:22:03.000000000 +0100 renamed from fs/hfs/super.c.orig 2015-11-29 14:42:22.000000000 +0100 +7 -2
@@ -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;
 }
#1 lore raw

Hi Timo,

Quoted 10 lines · 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.

---

fs/hfs/super.c 2015-11-29 15:22:03.000000000 +0100 static int hfs_statfs(struct dentry *den · line 116

Quoted 4 lines
fs/hfs/super.c 2015-11-29 15:22:03.000000000 +0100 renamed from fs/hfs/super.c.orig 2015-11-29 14:42:22.000000000 +0100 +0 -1
@@ -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.

Quoted 27 lines
fs/hfs/super.c 2015-11-29 15:22:03.000000000 +0100 renamed from fs/hfs/super.c.orig 2015-11-29 14:42:22.000000000 +0100 +7 -2
        *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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

Hello Vyacheslav,

Quoted 10 lines · Vyacheslav Dubeyko

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.

Quoted 3 lines

Thanks,
Vyacheslav Dubeyko.

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

Best regards
Timo Schlüßler