Skip to contentSkip to conversation

ufs fix sun state

PATCH New Evgeniy Dushistov 2 participants 2 messages Aug 19, 2007 – Sep 5, 2007

lore
Share & tools
lore

Evgeniy Dushistov

author
#0 lore raw

PATCH 1/1 ufs fix sun state

Different types of ufs hold state in different places, to hide complexity of this, there is ufs_get_fs_state, it returns state according to "UFS_SB(sb)->s_flags", but during mount ufs_get_fs_state is called, before setting s_flags, this cause message for ufs types like sun ufs: "fs need fsck",
and remount in readonly state.

This patch depend on other patches that now in -mm branch.

Signed-off-by Evgeniy Dushistov <[email protected]>

---

fs/ufs/super.c +1 -3
Index: linux-2.6.23-rc3-git1/fs/ufs/super.c
===================================================================
@@ -934,7 +934,7 @@ magic_found:
 		flags |=  UFS_ST_SUN;
 	}
 
-
+	sbi->s_flags = flags;/*after that line some functions use s_flags*/
 	ufs_print_super_stuff(sb, usb1, usb2, usb3);
 
 	/*

@@ -1065,8 +1065,6 @@ magic_found:
 	    UFS_MOUNT_UFSTYPE_44BSD)
 		uspi->s_maxsymlinklen =
 		    fs32_to_cpu(sb, usb3->fs_un2.fs_44.fs_maxsymlinklen);
-	
-	sbi->s_flags = flags;
 
 	inode = iget(sb, UFS_ROOTINO);
 	if (!inode || is_bad_inode(inode))
Signature · 1 line
/Evgeniy
#1 lore raw

PATCH 1/1 ufs: Fix mount check in ufs_fill_super()

Hi Evgeniy,

Quoted 8 lines · Evgeniy Dushistov

Different types of ufs hold state in different places, to hide complexity of this, there is ufs_get_fs_state, it returns state according to "UFS_SB(sb)->s_flags", but during mount ufs_get_fs_state is called, before setting s_flags, this cause message for ufs types like sun ufs: "fs need fsck",
and remount in readonly state.

I noticed another strange thing with that same compound condition in ufs_fill_super(). In the present code, if (flags & UFS_ST_MASK) == UFS_ST_44BSD, then the whole ufs_get_fs_state() == UFS_FSOK check gets completely skipped (!)

Is this intentional? If so, I'd recommend plonking a comment in there. But doesn't look that way to me, because ufs_get_fs_state() does handle the UFS_ST_44BSD case also ... Does the patch below look correct to you?

Satyam

[PATCH -mm] ufs: Fix mount check in ufs_fill_super()

The current code skips the check to verify whether the filesystem was previously cleanly unmounted, if (flags & UFS_ST_MASK) == UFS_ST_44BSD or UFS_ST_OLD. This looks like an inadvertent bug that slipped in due to parantheses in the compound conditional to me, especially given that ufs_get_fs_state() handles the UFS_ST_44BSD case perfectly well. So, let's fix the compound condition appropriately.

Signed-off-by Satyam Sharma <[email protected]>

---

 fs/ufs/super.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
fs/ufs/super.c renamed from fs/ufs/super.c~fix +8 -7
@@ -934,19 +934,20 @@ magic_found:
 		flags |=  UFS_ST_SUN;
 	}
 
-	sbi->s_flags = flags;/*after that line some functions use s_flags*/
+	/* Set sbi->s_flags here, used by ufs_get_fs_state() below */
+	sbi->s_flags = flags;
 	ufs_print_super_stuff(sb, usb1, usb2, usb3);
 
 	/*
 	 * Check, if file system was correctly unmounted.
 	 * If not, make it read only.
 	 */
-	if (((flags & UFS_ST_MASK) == UFS_ST_44BSD) ||
-	  ((flags & UFS_ST_MASK) == UFS_ST_OLD) ||
-	  (((flags & UFS_ST_MASK) == UFS_ST_SUN || 
-	    (flags & UFS_ST_MASK) == UFS_ST_SUNOS ||
-	  (flags & UFS_ST_MASK) == UFS_ST_SUNx86) && 
-	  (ufs_get_fs_state(sb, usb1, usb3) == (UFS_FSOK - fs32_to_cpu(sb, usb1->fs_time))))) {
+	if ((((flags & UFS_ST_MASK) == UFS_ST_44BSD)	||
+	     ((flags & UFS_ST_MASK) == UFS_ST_OLD)	||
+	     ((flags & UFS_ST_MASK) == UFS_ST_SUN)	||
+	     ((flags & UFS_ST_MASK) == UFS_ST_SUNOS)	||
+	     ((flags & UFS_ST_MASK) == UFS_ST_SUNx86))	&&
+	    (ufs_get_fs_state(sb, usb1, usb3) == (UFS_FSOK - fs32_to_cpu(sb, usb1->fs_time)))) {
 		switch(usb1->fs_clean) {
 		case UFS_FSCLEAN:
 			UFSD("fs is clean\n");