Skip to contentSkip to conversation

struct super_block cleanup - msdos/vfat

PATCH In review Brian Gerst 4 participants 11 messages Mar 14, 2002 – Mar 15, 2002

Subsystems (2)
VFS VFAT/FAT/msdos filesystem
lore
Share & tools
lore

Brian Gerst

author
#0 lore raw

PATCH 1/1 struct super_block cleanup - msdos/vfat

Seperates msdos_sb_info from struct super_block for msdos and vfat. Umsdos is terminally broken and is not included.

Signature · 1 line
						Brian Gerst
fs/fat/fatfs_syms.c +1 -1
diff -urN linux-2.5.7-pre1/fs/fat/fatfs_syms.c linux/fs/fat/fatfs_syms.c
@@ -24,7 +24,7 @@
 EXPORT_SYMBOL(fat_attach);
 EXPORT_SYMBOL(fat_detach);
 EXPORT_SYMBOL(fat_build_inode);
-EXPORT_SYMBOL(fat_read_super);
+EXPORT_SYMBOL(fat_fill_super);
 EXPORT_SYMBOL(fat_search_long);
 EXPORT_SYMBOL(fat_readdir);
 EXPORT_SYMBOL(fat_scan);
fs/fat/inode.c +34 -25

114 lines not on this page · raw message

fs/msdos/namei.c +4 -7
diff -urN linux-2.5.7-pre1/fs/msdos/namei.c linux/fs/msdos/namei.c
@@ -603,17 +603,14 @@
 
 int msdos_fill_super(struct super_block *sb,void *data, int silent)
 {
-	struct super_block *res;
+	int res;
 
-	MSDOS_SB(sb)->options.isvfat = 0;
-	res = fat_read_super(sb, data, silent, &msdos_dir_inode_operations);
-	if (IS_ERR(res))
-		return PTR_ERR(res);
-	if (res == NULL) {
+	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
+	if (res) {
 		if (!silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);
-		return -EINVAL;
+		return res;
 	}
 
 	sb->s_root->d_op = &msdos_dentry_operations;
fs/vfat/namei.c +12 -12
diff -urN linux-2.5.7-pre1/fs/vfat/namei.c linux/fs/vfat/namei.c
@@ -1285,25 +1285,25 @@
 
 int vfat_fill_super(struct super_block *sb, void *data, int silent)
 {
-	struct super_block *res;
+	int res;
+	struct msdos_sb_info *sbi;
   
-	MSDOS_SB(sb)->options.isvfat = 1;
-	res = fat_read_super(sb, data, silent, &vfat_dir_inode_operations);
-	if (IS_ERR(res))
-		return PTR_ERR(res);
-	if (res == NULL) {
+	res = fat_fill_super(sb, data, silent, &vfat_dir_inode_operations, 1);
+	if (res) {
 		if (!silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " VFAT filesystem on dev %s.\n", sb->s_id);
-		return -EINVAL;
+		return res;
 	}
 
-	if (parse_options((char *) data, &(MSDOS_SB(sb)->options))) {
-		MSDOS_SB(sb)->options.dotsOK = 0;
-		if (MSDOS_SB(sb)->options.posixfs) {
-			MSDOS_SB(sb)->options.name_check = 's';
+	sbi = MSDOS_SB(sb);
+
+	if (parse_options((char *) data, &(sbi->options))) {
+		sbi->options.dotsOK = 0;
+		if (sbi->options.posixfs) {
+			sbi->options.name_check = 's';
 		}
-		if (MSDOS_SB(sb)->options.name_check != 's') {
+		if (sbi->options.name_check != 's') {
 			sb->s_root->d_op = &vfat_dentry_ops[0];
 		} else {
 			sb->s_root->d_op = &vfat_dentry_ops[2];
include/linux/fs.h +0 -2
diff -urN linux-2.5.7-pre1/include/linux/fs.h linux/include/linux/fs.h
@@ -652,7 +652,6 @@
 #include <linux/ext3_fs_sb.h>
 #include <linux/hpfs_fs_sb.h>
 #include <linux/ntfs_fs_sb.h>
-#include <linux/msdos_fs_sb.h>
 #include <linux/iso_fs_sb.h>
 #include <linux/nfs_fs_sb.h>
 #include <linux/sysv_fs_sb.h>

@@ -708,7 +707,6 @@
 		struct ext3_sb_info	ext3_sb;
 		struct hpfs_sb_info	hpfs_sb;
 		struct ntfs_sb_info	ntfs_sb;
-		struct msdos_sb_info	msdos_sb;
 		struct isofs_sb_info	isofs_sb;
 		struct nfs_sb_info	nfs_sb;
 		struct sysv_sb_info	sysv_sb;
include/linux/msdos_fs.h +8 -4
diff -urN linux-2.5.7-pre1/include/linux/msdos_fs.h linux/include/linux/msdos_fs.h
@@ -5,6 +5,7 @@
  * The MS-DOS filesystem constants/structures
  */
 #include <linux/msdos_fs_i.h>
+#include <linux/msdos_fs_sb.h>
 
 #include <asm/byteorder.h>
 

@@ -53,7 +54,11 @@
 #define MSDOS_VALID_MODE (S_IFREG | S_IFDIR | S_IRWXU | S_IRWXG | S_IRWXO)
 	/* valid file mode bits */
 
-#define MSDOS_SB(s) (&((s)->u.msdos_sb))
+static inline struct msdos_sb_info *MSDOS_SB(struct super_block *sb)
+{
+	return sb->u.generic_sbp;
+}
+
 static inline struct msdos_inode_info *MSDOS_I(struct inode *inode)
 {
 	return list_entry(inode, struct msdos_inode_info, vfs_inode);

@@ -282,9 +287,8 @@
 extern void fat_delete_inode(struct inode *inode);
 extern void fat_clear_inode(struct inode *inode);
 extern void fat_put_super(struct super_block *sb);
-extern struct super_block *
-fat_read_super(struct super_block *sb, void *data, int silent,
-	       struct inode_operations *fs_dir_inode_ops);
+int fat_fill_super(struct super_block *sb, void *data, int silent,
+		   struct inode_operations *fs_dir_inode_ops, int isvfat);
 extern int fat_statfs(struct super_block *sb, struct statfs *buf);
 extern void fat_write_inode(struct inode *inode, int wait);
 extern int fat_notify_change(struct dentry * dentry, struct iattr * attr);
#1 lore raw
Quoted 2 lines · Brian Gerst

Seperates msdos_sb_info from struct super_block for msdos and vfat. Umsdos is terminally broken and is not included.

We have everything needed to fix^Wrewrite umsdos and I hope to do that this week. Main idea of the rewrite: turn it into proper layered filesystem (i.e. let the underlying layer have its own superblock, inodes, etc.)...

#2 lore raw
Quoted 8 lines · Alexander Viro
Quoted 2 lines · Brian Gerst

Seperates msdos_sb_info from struct super_block for msdos and vfat. Umsdos is terminally broken and is not included.

We have everything needed to fix^Wrewrite umsdos and I hope to do that this week. Main idea of the rewrite: turn it into proper layered filesystem (i.e. let the underlying layer have its own superblock, inodes, etc.)...

Does this mean umsdos can be layered atop of wider range of filesystems than just msdos? That would be cool.

Also, would it be possible to mount both underlying msdos fs and umsdos fs layered on top of it at the same time (on different mountpoints)? --
vda

#3 lore raw
Quoted 2 lines · Denis Vlasenko

Does this mean umsdos can be layered atop of wider range of filesystems than just msdos? That would be cool.

Yes, but what's cool about it? If not for the fact that there are weird setups that actually use umsdos (i.e. compatibility reasons), the best way to deal with it would be rm -rf... If underlying filesystem has normal semantics - you don't need anything, if it doesn't... I'd suggest to use combination of tar(1) and ramfs. At least that way you get full Unix semantics - no mess with rename breaking links, etc.

Quoted 2 lines

Also, would it be possible to mount both underlying msdos fs and umsdos fs layered on top of it at the same time (on different mountpoints)?

No. That stuff is ugly as it is and trying to make it deal with unexpected changes of underlying fs... <shudder>

#4 lore raw
Quoted 10 lines · Alexander Viro
Quoted 2 lines · Denis Vlasenko

Does this mean umsdos can be layered atop of wider range of filesystems than just msdos? That would be cool.

Yes, but what's cool about it? If not for the fact that there are weird setups that actually use umsdos (i.e. compatibility reasons), the best way to deal with it would be rm -rf... If underlying filesystem has normal semantics - you don't need anything, if it doesn't... I'd suggest to use combination of tar(1) and ramfs. At least that way you get full Unix semantics - no mess with rename breaking links, etc.

Well, I initially come here from DOS/Win world and in fact actually used umsdos for some time (heck, it's still installed on one abandoned box). But I presume there are other worlds (maybe Mac?) with filesystems unsuited for Linux root fs (like fat), why invent u[fs] for them too?

OTOH it means extra effort in umsdos rewrite, and since I don't do that effort, I'd better shut up now.
--
vda

maintainer
#5 lore raw
In reply to Brian Gerst

Hi,

Quoted 10 lines · Brian Gerst

diff -urN linux-2.5.7-pre1/fs/msdos/namei.c linux/fs/msdos/namei.c

fs/msdos/namei.c +1 -1
@@ -603,17 +603,14 @@
 
 int msdos_fill_super(struct super_block *sb,void *data, int silent)
 {
-	struct super_block *res;
+	int res;

fs/msdos/namei.c line 612

Quoted 10 lines
fs/msdos/namei.c +2 -5
-	MSDOS_SB(sb)->options.isvfat = 0;
-	res = fat_read_super(sb, data, silent, &msdos_dir_inode_operations);
-	if (IS_ERR(res))
-		return PTR_ERR(res);
-	if (res == NULL) {
+	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
+	if (res) {
 		if (!silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);

If the error is I/O error, I think we shouldn't output this message. What do you think about this?

Quoted 5 lines
fs/msdos/namei.c +1 -1
-		return -EINVAL;
+		return res;
 	}
 
 	sb->s_root->d_op = &msdos_dentry_operations;
Signature · 1 line
OGAWA Hirofumi <[email protected]>
maintainer
#6 lore raw
In reply to Brian Gerst
Quoted 14 lines · OGAWA Hirofumi

Hi,

Quoted 10 lines · Brian Gerst

diff -urN linux-2.5.7-pre1/fs/msdos/namei.c linux/fs/msdos/namei.c

fs/msdos/namei.c +1 -1
@@ -603,17 +603,14 @@
 
 int msdos_fill_super(struct super_block *sb,void *data, int silent)
 {
-	struct super_block *res;
+	int res;
Quoted 12 lines

fs/msdos/namei.c line 612

Quoted 10 lines
fs/msdos/namei.c +2 -5
-	MSDOS_SB(sb)->options.isvfat = 0;
-	res = fat_read_super(sb, data, silent, &msdos_dir_inode_operations);
-	if (IS_ERR(res))
-		return PTR_ERR(res);
-	if (res == NULL) {
+	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
+	if (res) {
 		if (!silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);

If the error is I/O error, I think we shouldn't output this message.

^^^^^^^^^^^^^^^^^^^^^^^^^^
If the error is except -EINVAL,

Sorry.

Quoted 1 line

What do you think about this?

Signature · 1 line
OGAWA Hirofumi <[email protected]>
author
#7 lore raw
In reply to Brian Gerst
Quoted 31 lines · OGAWA Hirofumi
Quoted 15 lines · OGAWA Hirofumi

Hi,

Quoted 10 lines · Brian Gerst

diff -urN linux-2.5.7-pre1/fs/msdos/namei.c linux/fs/msdos/namei.c

fs/msdos/namei.c +1 -1
@@ -603,17 +603,14 @@
 
 int msdos_fill_super(struct super_block *sb,void *data, int silent)
 {
-	struct super_block *res;
+	int res;
Quoted 12 lines

fs/msdos/namei.c line 612

Quoted 10 lines
fs/msdos/namei.c +2 -5
-	MSDOS_SB(sb)->options.isvfat = 0;
-	res = fat_read_super(sb, data, silent, &msdos_dir_inode_operations);
-	if (IS_ERR(res))
-		return PTR_ERR(res);
-	if (res == NULL) {
+	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
+	if (res) {
 		if (!silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);

If the error is I/O error, I think we shouldn't output this message.

Quoted 8 lines

^^^^^^^^^^^^^^^^^^^^^^^^^^
If the error is except -EINVAL,

Sorry.

Quoted 1 line

What do you think about this?

Why not? The statement is true, and other filesystems do complain when there is an I/O error.

Signature · 1 line
						Brian Gerst
maintainer
#8 lore raw
In reply to Brian Gerst
Quoted 32 lines · Brian Gerst
Quoted 31 lines · OGAWA Hirofumi
Quoted 17 lines · OGAWA Hirofumi

Hi,

Quoted 12 lines · Brian Gerst

diff -urN linux-2.5.7-pre1/fs/msdos/namei.c linux/fs/msdos/namei.c

fs/msdos/namei.c +1 -5
@@ -603,17 +603,14 @@
 int msdos_fill_super(struct super_block *sb,void *data, int silent)
 {
-	struct super_block *res;
+	int res;
-	MSDOS_SB(sb)->options.isvfat = 0;
-	res = fat_read_super(sb, data, silent, &msdos_dir_inode_operations);
-	if (IS_ERR(res))
-		return PTR_ERR(res);
Quoted 8 lines

fs/msdos/namei.c line 612

Quoted 6 lines
fs/msdos/namei.c +2 -1
-	if (res == NULL) {
+	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
+	if (res) {
 		if (!silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);

If the error is I/O error, I think we shouldn't output this message.

^^^^^^^^^^^^^^^^^^^^^^^^^^
If the error is except -EINVAL,
Sorry.

Quoted 5 lines
Quoted 2 lines
Quoted 1 line

What do you think about this?

Why not? The statement is true, and other filesystems do complain when there is an I/O error.

Umm, almost all filesystems doesn't output this message when the I/O error occurs, AFAIK.

I think that this message indicate that a device isn't a FAT filesystem. And, of course, if error is the I/O error, fat_full_super() can't detect whether it is FAT filesystem or not.

Signature · 1 line
OGAWA Hirofumi <[email protected]>
author
#9 lore raw
In reply to Brian Gerst
Quoted 46 lines · OGAWA Hirofumi
Quoted 37 lines · Brian Gerst
Quoted 35 lines · OGAWA Hirofumi
Quoted 18 lines · OGAWA Hirofumi

Hi,

Quoted 12 lines · Brian Gerst

diff -urN linux-2.5.7-pre1/fs/msdos/namei.c linux/fs/msdos/namei.c

fs/msdos/namei.c +1 -5
@@ -603,17 +603,14 @@
 int msdos_fill_super(struct super_block *sb,void *data, int silent)
 {
-	struct super_block *res;
+	int res;
-	MSDOS_SB(sb)->options.isvfat = 0;
-	res = fat_read_super(sb, data, silent, &msdos_dir_inode_operations);
-	if (IS_ERR(res))
-		return PTR_ERR(res);
Quoted 8 lines

fs/msdos/namei.c line 610

Quoted 6 lines
fs/msdos/namei.c +2 -1
-	if (res == NULL) {
+	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
+	if (res) {
 		if (!silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);

If the error is I/O error, I think we shouldn't output this message.

^^^^^^^^^^^^^^^^^^^^^^^^^^
If the error is except -EINVAL,
Sorry.

Quoted 4 lines
Quoted 2 lines
Quoted 1 line

What do you think about this?

Why not? The statement is true, and other filesystems do complain when there is an I/O error.

Quoted 6 lines

Umm, almost all filesystems doesn't output this message when the I/O error occurs, AFAIK.

I think that this message indicate that a device isn't a FAT filesystem. And, of course, if error is the I/O error, fat_full_super() can't detect whether it is FAT filesystem or not.

Patch attached.

Signature · 1 line
						Brian Gerst
fs/msdos/namei.c +1 -1
diff -urN linux/fs/msdos/namei.c linux2/fs/msdos/namei.c
@@ -607,7 +607,7 @@
 
 	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
 	if (res) {
-		if (!silent)
+		if (res == -EINVAL && !silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);
 		return res;
fs/vfat/namei.c +1 -1
diff -urN linux/fs/vfat/namei.c linux2/fs/vfat/namei.c
@@ -1290,7 +1290,7 @@
   
 	res = fat_fill_super(sb, data, silent, &vfat_dir_inode_operations, 1);
 	if (res) {
-		if (!silent)
+		if (res == -EINVAL && !silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " VFAT filesystem on dev %s.\n", sb->s_id);
 		return res;
maintainer
#10 lore raw
In reply to Brian Gerst
Quoted 5 lines · Brian Gerst

Patch attached.

Signature · 1 line
						Brian Gerst

Thanks a lot!

Quoted 24 lines

diff -urN linux/fs/msdos/namei.c linux2/fs/msdos/namei.c

fs/msdos/namei.c +1 -1
@@ -607,7 +607,7 @@
 
 	res = fat_fill_super(sb, data, silent, &msdos_dir_inode_operations, 0);
 	if (res) {
-		if (!silent)
+		if (res == -EINVAL && !silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " MSDOS filesystem on dev %s.\n", sb->s_id);
 		return res;

diff -urN linux/fs/vfat/namei.c linux2/fs/vfat/namei.c

fs/vfat/namei.c +1 -1
@@ -1290,7 +1290,7 @@
 
 	res = fat_fill_super(sb, data, silent, &vfat_dir_inode_operations, 1);
 	if (res) {
-		if (!silent)
+		if (res == -EINVAL && !silent)
 			printk(KERN_INFO "VFS: Can't find a valid"
 			       " VFAT filesystem on dev %s.\n", sb->s_id);
 		return res;
Signature · 1 line
OGAWA Hirofumi <[email protected]>