remove ocfs_fill_super and port ocfs_read_super

PATCH New Rusty Lynch 3 participants 10 messages Mar 25, 2004 – Mar 30, 2004 OCFS2

Share & tools

Rusty Lynch

author

2004-03-25 21:00 UTC #0 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

The 2.6 version of ocfs_read_super (known as ocfs_fill_super) had a bug where it wasn't unmounting on error. There really isn't a lot different between the 2.4 and 2.5 functions, so here is a patch that adds 2.6 support to the existing ocfs_read_super, and nukes the old ocfs_fill_super.

    --rusty

super.c +63 ~~-105~~

Index: src/super.c
===================================================================
@@ -154,108 +154,15 @@
 
 };
 
-
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
-
-static int ocfs_fill_super (struct super_block *sb, void *data, int silent)
-{
-	struct dentry *root_dentry;
-	int status = -1;
-	struct inode *root_inode = NULL;
-	__u32 uid = current->fsuid;
-	__u32 gid = current->fsgid;
-	bool reclaim_id;
-        ocfs_super *osb;
-
-	LOG_ENTRY ();
-	
-	if (ocfs_parse_options (data, &uid, &gid, &reclaim_id) != 0) {
-		LOG_ERROR_STR ("bad mount option");
-		status=-EINVAL;
-		goto read_super_error;
-	}
-
-	sb->s_magic = OCFS_MAGIC;
-	sb->s_op = &ocfs_sops;
-	sb->s_flags |= MS_NOATIME;
-
-	/* this is needed to support O_LARGE_FILE */
-	sb->s_maxbytes = OCFS_LINUX_MAX_FILE_SIZE;
-
-	if (!sb_set_blocksize(sb, 512)) {
-		LOG_ERROR_STR("Could not set block size");
-		status=-EIO;
-		goto read_super_error;
-	}
-
-	status = ocfs_mount_volume (sb, reclaim_id, NULL);
-	if (status < 0) {
-		goto read_super_error;
-	}
-	osb = ((ocfs_super *)(sb->s_fs_info));
-	if (!osb) {
-		status=-EINVAL;
-		goto read_super_error;
-	}
-
-	root_inode = ocfs_iget(sb, NULL);
-	if (!root_inode) {
-		status=-EIO;
-		LOG_ERROR_STATUS (status);
-		goto read_super_error;
-	}
-
-	root_dentry = d_alloc_root (root_inode);
-	if (!root_dentry) {
-		status=-ENOMEM;
-		LOG_ERROR_STATUS (status);
-		goto read_super_error;
-	}
-
-	sb->s_root = root_dentry;
-	printk ("ocfs2: Mounting device (%u,%u) on %s (node %d)\n",
-		MAJOR(sb->s_dev), MINOR(sb->s_dev),
-		osb->node_cfg_info[osb->node_num]->node_name, osb->node_num);
-
-	status = 0;
-	LOG_EXIT_STATUS (status);
-	return status;
-
-read_super_error:
-	if (root_inode != NULL) {
-		iput (root_inode);
-		root_inode = NULL;
-	}
-
-	LOG_EXIT_STATUS (status);
-	return status;
-}				/* ocfs_fill_super */
-
-static struct super_block *ocfs_get_sb(struct file_system_type *fs_type, int flags, const char *dev_name, void *data)
-{
-	return get_sb_bdev(fs_type, flags, dev_name, data, ocfs_fill_super);
-}
-
-static struct file_system_type ocfs_fs_type = {
-        .owner          = THIS_MODULE,
-        .name           = "ocfs2",
-        .get_sb         = ocfs_get_sb, /* is this called when we mount
-					* the fs? */
-        .kill_sb        = kill_block_super, /* set to the generic one
-					     * right now, but do we
-					     * need to change that? */
-        .fs_flags       = FS_REQUIRES_DEV,
-	.next           = NULL
-};
-
-#else  /* We're a 2.4 kernel */
-
-
 /*
  * ocfs_read_super()
  *
  */
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+static int ocfs_read_super (struct super_block *sb, void *data, int silent)
+#else
 static struct super_block *ocfs_read_super (struct super_block *sb, void *data, int silent)
+#endif
 {
 	struct dentry *root;
 	int status;

@@ -265,15 +172,24 @@
 	bool reclaim_id;
 	ocfs_super *osb = NULL;
 
-	LOG_ENTRY ();
+	LOG_ENTRY_ARGS ("%p, %p, %i", sb, data, silent);
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)	
 	MOD_INC_USE_COUNT;
-
+#endif
 	if (ocfs_parse_options (data, &uid, &gid, &reclaim_id) != 0) {
+		status = -EINVAL;
 		LOG_ERROR_STR ("ocfs_read_super: bad mount option");
 		goto read_super_error;
 	}
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+	if (!sb_set_blocksize(sb, 512)) {
+		LOG_ERROR_STR("Could not set block size");
+		status=-EIO;
+		goto read_super_error;
+	}
+#else
 	/* TODO: fix this */
 	sb->s_blocksize = 512;
 	sb->s_blocksize_bits = 9;

@@ -285,7 +201,8 @@
 	}
 #else
 	set_blocksize (sb->s_dev, 512);
-#endif
+#endif /* < 2.4.18 */
+#endif /* < 2.6.0 */
 
 	sb->s_magic = OCFS_MAGIC;
 	sb->s_op = &ocfs_sops;

@@ -295,20 +212,32 @@
 	sb->s_maxbytes = OCFS_LINUX_MAX_FILE_SIZE;
 
 	status = ocfs_mount_volume (sb, reclaim_id, NULL);
+	if (status < 0)
+		goto read_super_error;
+
         osb = (ocfs_super *) OCFS_GENERIC_SB_P(sb);
-	if (status < 0 || !osb)
+	if (!osb) {
+		status = -EINVAL;
 		goto read_super_error;
+	}
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+	inode = ocfs_iget(sb, NULL);
+#else	
 	inode = iget4 (sb, OCFS_ROOT_INODE_NUMBER, 0, NULL);
+#endif
 	if (!inode) {
+		status = -EIO;
 		LOG_ERROR_STATUS (status);
 		goto read_super_error;
 	}
 
 	root = d_alloc_root (inode);
 	if (!root) {
+		status = -ENOMEM;
 		LOG_ERROR_STATUS (status);
 		iput (inode);
+		/* should we be setting inode to null?? */
 		goto read_super_error;
 	}

@@ -318,28 +247,57 @@
 		MAJOR(sb->s_dev), MINOR(sb->s_dev),
 		osb->node_cfg_info[osb->node_num]->node_name, osb->node_num);
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+	LOG_EXIT_STATUS(status);
+	return status;		
+#else
 	LOG_EXIT_PTR (sb);
 	return sb;
-
+#endif
 read_super_error:
 	if (osb)
 		ocfs_dismount_volume (sb);
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
 	MOD_DEC_USE_COUNT;
+#endif
+
 	if (inode != NULL) {
 		iput (inode);
 		inode = NULL;
 	}
 
-	LOG_EXIT_PTR (0);
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+	LOG_EXIT_STATUS(status);
+	return status;
+#else
+	LOG_EXIT_PTR (NULL);
 	return NULL;
+#endif
 }				/* ocfs_read_super */
 
 
-static DECLARE_FSTYPE (ocfs_fs_type, "ocfs2", ocfs_read_super, FS_REQUIRES_DEV);
-#endif /* #if 2.6 kernel ... #else */
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+static struct super_block *ocfs_get_sb(struct file_system_type *fs_type, int flags, const char *dev_name, void *data)
+{
+	return get_sb_bdev(fs_type, flags, dev_name, data, ocfs_read_super);
+}
 
+static struct file_system_type ocfs_fs_type = {
+        .owner          = THIS_MODULE,
+        .name           = "ocfs2",
+        .get_sb         = ocfs_get_sb, /* is this called when we mount
+					* the fs? */
+        .kill_sb        = kill_block_super, /* set to the generic one
+					     * right now, but do we
+					     * need to change that? */
+        .fs_flags       = FS_REQUIRES_DEV,
+	.next           = NULL
+};
+#else
+static DECLARE_FSTYPE (ocfs_fs_type, "ocfs2", ocfs_read_super, FS_REQUIRES_DEV);
+#endif
 
 /*
  * ocfs_parse_options()

Joel Becker

2004-03-26 08:35 UTC #1 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 5 lines · Rusty Lynch

The 2.6 version of ocfs_read_super (known as ocfs_fill_super) had a bug where it wasn't unmounting on error. There really isn't a lot different between the 2.4 and 2.5 functions, so here is a patch that adds 2.6 support to the existing ocfs_read_super, and nukes the old ocfs_fill_super.

Ugh ugh ugh. I see what you are trying to do here, but you are
peppering one function with multiple #ifdefs. Better to make the guts be an #ifdef ocfs_generic_fill_super(), with:

#if KERNEL_VERSION < LINUX_VERSION_CODE(2,6,0)
static int ocfs_read_super(...)

{
    /* 2.6 specific */
    ocfs_generic_fill_super();
    /* 2.6 specific */
}
#else
static struct super_block *ocfs_read_super(...)
{
    /* 2.4 specific */
    ocfs_generic_fill_super();
    /* 2.4 specific */
}

#endif

Joel

Signature · 9 lines

"Under capitalism, man exploits man.  Under Communism, it's just 
   the opposite."
				 - John Kenneth Galbraith

Joel Becker
Senior Member of Technical Staff
Oracle Corporation
E-mail: [email protected]
Phone: (650) 506-8127

Rusty Lynch

author

2004-03-26 14:47 UTC #2 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 25 lines · Joel Becker

Quoted 5 lines · Rusty Lynch
The 2.6 version of ocfs_read_super (known as ocfs_fill_super) had a bug where it wasn't unmounting on error. There really isn't a lot different between the 2.4 and 2.5 functions, so here is a patch that adds 2.6 support to the existing ocfs_read_super, and nukes the old ocfs_fill_super.
Ugh ugh ugh. I see what you are trying to do here, but you are
peppering one function with multiple #ifdefs. Better to make the guts be an #ifdef ocfs_generic_fill_super(), with:
#if KERNEL_VERSION < LINUX_VERSION_CODE(2,6,0)
static int ocfs_read_super(...)
{
    /* 2.6 specific */
    ocfs_generic_fill_super();
    /* 2.6 specific */
}
#else
static struct super_block *ocfs_read_super(...)
{
    /* 2.4 specific */
    ocfs_generic_fill_super();
    /* 2.4 specific */
}

Quoted 1 line

#endif

How about something like...

static int ocfs_set_blocksize(struct super_block *sb)

{
	int status = 0;

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
	if (!sb_set_blocksize(sb, 512))
		status = -EIO;
#else
	/* TODO: fix this */
	sb->s_blocksize = 512;
	sb->s_blocksize_bits = 9;
#if LINUX_VERSION_CODE >= LinuxVersionCode(2,4,18)
	status = set_blocksize (sb->s_dev, 512);
#else
	set_blocksize (sb->s_dev, 512);

#endif /* < 2.4.18 */
#endif /* < 2.6.0 */

	return status;
}

/*
* __ocfs_read_super()
*
*/
static int __ocfs_read_super(struct super_block *sb, void *data, int silent)

{
	struct dentry *root;
	int status;
	struct inode *inode = NULL;
	__u32 uid = current->fsuid;
	__u32 gid = current->fsgid;
	bool reclaim_id;
	ocfs_super *osb = NULL;

	LOG_ENTRY_ARGS ("%p, %p, %i", sb, data, silent);

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)	
	MOD_INC_USE_COUNT;
#endif
	if (ocfs_parse_options (data, &uid, &gid, &reclaim_id) != 0) {
		status = -EINVAL;
		LOG_ERROR_STR ("ocfs_read_super: bad mount option");
		goto read_super_error;
	}

	status = ocfs_set_blocksize(sb);
	if (status < 0) {
		LOG_ERROR_STR("unable to set blocksize");
		goto read_super_error;
	}

	sb->s_magic = OCFS_MAGIC;
	sb->s_op = &ocfs_sops;
	sb->s_flags |= MS_NOATIME;

	/* this is needed to support O_LARGE_FILE */
	sb->s_maxbytes = OCFS_LINUX_MAX_FILE_SIZE;

	status = ocfs_mount_volume (sb, reclaim_id, NULL);
	if (status < 0)
		goto read_super_error;

        osb = (ocfs_super *) OCFS_GENERIC_SB_P(sb);
	if (!osb) {
		status = -EINVAL;
		goto read_super_error;
	}

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
	inode = ocfs_iget(sb, NULL);
#else	
	inode = iget4 (sb, OCFS_ROOT_INODE_NUMBER, 0, NULL);
#endif
	if (!inode) {
		status = -EIO;
		LOG_ERROR_STATUS (status);
		goto read_super_error;
	}

	root = d_alloc_root (inode);
	if (!root) {
		status = -ENOMEM;
		LOG_ERROR_STATUS (status);
		iput (inode);
		/* should we be setting inode to null?? */
		goto read_super_error;
	}

	sb->s_root = root;

	printk ("ocfs2: Mounting device (%u,%u) on %s (node %d)\n",
		MAJOR(sb->s_dev), MINOR(sb->s_dev),
		osb->node_cfg_info[osb->node_num]->node_name, osb->node_num);

	LOG_EXIT_STATUS(status);
	return status;

read_super_error:

	if (osb)
		ocfs_dismount_volume (sb);

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
	MOD_DEC_USE_COUNT;

#endif

	if (inode != NULL) {
		iput (inode);
		inode = NULL;
	}

	LOG_EXIT_STATUS(status);
	return status;
}

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
static int ocfs_read_super (struct super_block *sb, void *data, int silent)

{
	return __ocfs_read_super(sb, data, silent);
}
#else
static struct super_block *ocfs_read_super (struct super_block *sb, void *data, int silent)
{
	if (__ocfs_read_super(sb, data, silent) < 0)
		return NULL;
	return sb;
}

#endif

The __ocfs_read_super() still has to keep some #if/#else/#endif's for 2.4/2.6 code, so it really isn't generic, so I thought __ocfs_read_super() was more accurate. The biggest part of __ocfs_read_super that made it hard on the eyes was the stuff about setting the block size, so I went ahead and seperated that into a ocfs_set_blocksize() function that does the right thing for a given kernel version. (It adds another function call, but does it really matter if a mount takes a sliver of time longer?)

Take a look@this and see if this is better.

--rusty

super.c +76 ~~-113~~

Index: src/super.c
===================================================================
@@ -154,108 +154,32 @@
 
 };
 
+static int ocfs_set_blocksize(struct super_block *sb)
+{
+	int status = 0;
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+	if (!sb_set_blocksize(sb, 512))
+		status = -EIO;
+#else
+	/* TODO: fix this */
+	sb->s_blocksize = 512;
+	sb->s_blocksize_bits = 9;
+#if LINUX_VERSION_CODE >= LinuxVersionCode(2,4,18)
+	status = set_blocksize (sb->s_dev, 512);
+#else
+	set_blocksize (sb->s_dev, 512);
+#endif /* < 2.4.18 */
+#endif /* < 2.6.0 */
 
-static int ocfs_fill_super (struct super_block *sb, void *data, int silent)
-{
-	struct dentry *root_dentry;
-	int status = -1;
-	struct inode *root_inode = NULL;
-	__u32 uid = current->fsuid;
-	__u32 gid = current->fsgid;
-	bool reclaim_id;
-        ocfs_super *osb;
-
-	LOG_ENTRY ();
-	
-	if (ocfs_parse_options (data, &uid, &gid, &reclaim_id) != 0) {
-		LOG_ERROR_STR ("bad mount option");
-		status=-EINVAL;
-		goto read_super_error;
-	}
-
-	sb->s_magic = OCFS_MAGIC;
-	sb->s_op = &ocfs_sops;
-	sb->s_flags |= MS_NOATIME;
-
-	/* this is needed to support O_LARGE_FILE */
-	sb->s_maxbytes = OCFS_LINUX_MAX_FILE_SIZE;
-
-	if (!sb_set_blocksize(sb, 512)) {
-		LOG_ERROR_STR("Could not set block size");
-		status=-EIO;
-		goto read_super_error;
-	}
-
-	status = ocfs_mount_volume (sb, reclaim_id, NULL);
-	if (status < 0) {
-		goto read_super_error;
-	}
-	osb = ((ocfs_super *)(sb->s_fs_info));
-	if (!osb) {
-		status=-EINVAL;
-		goto read_super_error;
-	}
-
-	root_inode = ocfs_iget(sb, NULL);
-	if (!root_inode) {
-		status=-EIO;
-		LOG_ERROR_STATUS (status);
-		goto read_super_error;
-	}
-
-	root_dentry = d_alloc_root (root_inode);
-	if (!root_dentry) {
-		status=-ENOMEM;
-		LOG_ERROR_STATUS (status);
-		goto read_super_error;
-	}
-
-	sb->s_root = root_dentry;
-	printk ("ocfs2: Mounting device (%u,%u) on %s (node %d)\n",
-		MAJOR(sb->s_dev), MINOR(sb->s_dev),
-		osb->node_cfg_info[osb->node_num]->node_name, osb->node_num);
-
-	status = 0;
-	LOG_EXIT_STATUS (status);
 	return status;
-
-read_super_error:
-	if (root_inode != NULL) {
-		iput (root_inode);
-		root_inode = NULL;
-	}
-
-	LOG_EXIT_STATUS (status);
-	return status;
-}				/* ocfs_fill_super */
-
-static struct super_block *ocfs_get_sb(struct file_system_type *fs_type, int flags, const char *dev_name, void *data)
-{
-	return get_sb_bdev(fs_type, flags, dev_name, data, ocfs_fill_super);
 }
 
-static struct file_system_type ocfs_fs_type = {
-        .owner          = THIS_MODULE,
-        .name           = "ocfs2",
-        .get_sb         = ocfs_get_sb, /* is this called when we mount
-					* the fs? */
-        .kill_sb        = kill_block_super, /* set to the generic one
-					     * right now, but do we
-					     * need to change that? */
-        .fs_flags       = FS_REQUIRES_DEV,
-	.next           = NULL
-};
-
-#else  /* We're a 2.4 kernel */
-
-
 /*
- * ocfs_read_super()
+ * __ocfs_read_super()
  *
  */
-static struct super_block *ocfs_read_super (struct super_block *sb, void *data, int silent)
+static int __ocfs_read_super(struct super_block *sb, void *data, int silent)
 {
 	struct dentry *root;
 	int status;

@@ -265,27 +189,22 @@
 	bool reclaim_id;
 	ocfs_super *osb = NULL;
 
-	LOG_ENTRY ();
+	LOG_ENTRY_ARGS ("%p, %p, %i", sb, data, silent);
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)	
 	MOD_INC_USE_COUNT;
-
+#endif
 	if (ocfs_parse_options (data, &uid, &gid, &reclaim_id) != 0) {
+		status = -EINVAL;
 		LOG_ERROR_STR ("ocfs_read_super: bad mount option");
 		goto read_super_error;
 	}
 
-	/* TODO: fix this */
-	sb->s_blocksize = 512;
-	sb->s_blocksize_bits = 9;
-#if LINUX_VERSION_CODE >= LinuxVersionCode(2,4,18)
-	status = set_blocksize (sb->s_dev, 512);
+	status = ocfs_set_blocksize(sb);
 	if (status < 0) {
-		LOG_ERROR_STR ("ocfs_read_super: set_blocksize failed!");
+		LOG_ERROR_STR("unable to set blocksize");
 		goto read_super_error;
 	}
-#else
-	set_blocksize (sb->s_dev, 512);
-#endif
 
 	sb->s_magic = OCFS_MAGIC;
 	sb->s_op = &ocfs_sops;

@@ -295,20 +214,32 @@
 	sb->s_maxbytes = OCFS_LINUX_MAX_FILE_SIZE;
 
 	status = ocfs_mount_volume (sb, reclaim_id, NULL);
+	if (status < 0)
+		goto read_super_error;
+
         osb = (ocfs_super *) OCFS_GENERIC_SB_P(sb);
-	if (status < 0 || !osb)
+	if (!osb) {
+		status = -EINVAL;
 		goto read_super_error;
+	}
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+	inode = ocfs_iget(sb, NULL);
+#else	
 	inode = iget4 (sb, OCFS_ROOT_INODE_NUMBER, 0, NULL);
+#endif
 	if (!inode) {
+		status = -EIO;
 		LOG_ERROR_STATUS (status);
 		goto read_super_error;
 	}
 
 	root = d_alloc_root (inode);
 	if (!root) {
+		status = -ENOMEM;
 		LOG_ERROR_STATUS (status);
 		iput (inode);
+		/* should we be setting inode to null?? */
 		goto read_super_error;
 	}

@@ -318,29 +249,61 @@
 		MAJOR(sb->s_dev), MINOR(sb->s_dev),
 		osb->node_cfg_info[osb->node_num]->node_name, osb->node_num);
 
-	LOG_EXIT_PTR (sb);
-	return sb;
+	LOG_EXIT_STATUS(status);
+	return status;		
 
 read_super_error:
 	if (osb)
 		ocfs_dismount_volume (sb);
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
 	MOD_DEC_USE_COUNT;
+#endif
+
 	if (inode != NULL) {
 		iput (inode);
 		inode = NULL;
 	}
 
-	LOG_EXIT_PTR (0);
-	return NULL;
-}				/* ocfs_read_super */
+	LOG_EXIT_STATUS(status);
+	return status;
+}
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+static int ocfs_read_super (struct super_block *sb, void *data, int silent)
+{
+	return __ocfs_read_super(sb, data, silent);
+}
+#else
+static struct super_block *ocfs_read_super (struct super_block *sb, void *data, int silent)
+{
+	if (__ocfs_read_super(sb, data, silent) < 0)
+		return NULL;
+	return sb;
+}
+#endif
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+static struct super_block *ocfs_get_sb(struct file_system_type *fs_type, int flags, const char *dev_name, void *data)
+{
+	return get_sb_bdev(fs_type, flags, dev_name, data, ocfs_read_super);
+}
+
+static struct file_system_type ocfs_fs_type = {
+        .owner          = THIS_MODULE,
+        .name           = "ocfs2",
+        .get_sb         = ocfs_get_sb, /* is this called when we mount
+					* the fs? */
+        .kill_sb        = kill_block_super, /* set to the generic one
+					     * right now, but do we
+					     * need to change that? */
+        .fs_flags       = FS_REQUIRES_DEV,
+	.next           = NULL
+};
+#else
 static DECLARE_FSTYPE (ocfs_fs_type, "ocfs2", ocfs_read_super, FS_REQUIRES_DEV);
-#endif /* #if 2.6 kernel ... #else */
+#endif
 
-
-
 /*
  * ocfs_parse_options()
  *

inc/io.h +0 -1

Index: src/inc/io.h
===================================================================
@@ -41,7 +41,6 @@
 				  struct buffer_head **bh, 
 				  int                  flags, 
 				  struct inode        *inode);
-
 int ocfs_write_bhs (ocfs_super          *osb,
 		    struct buffer_head  *bh[], 
 		    int                  nr,

Mark Fasheh

2004-03-29 15:29 UTC #3 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 1 line · Rusty Lynch

How about something like...

Could you move the MOD_INC_USE_COUNT / MOD_DEC_USE_COUNT macros out of the generic function too?

We could make ocfs_set_blocksize an inline, unless you think it's too big for that...

	--Mark

--
Mark Fasheh
Software Developer, Oracle Corp
[email protected]

Rusty Lynch

author

2004-03-29 15:32 UTC #4 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 4 lines · Mark Fasheh

Quoted 1 line · Rusty Lynch
How about something like...
Could you move the MOD_INC_USE_COUNT / MOD_DEC_USE_COUNT macros out of the generic function too?

Can do

Quoted 3 lines

We could make ocfs_set_blocksize an inline, unless you think it's too big for that...

I should be inline. Let me change that.

Quoted 6 lines

	--Mark
--
Mark Fasheh
Software Developer, Oracle Corp
[email protected]

Rusty Lynch

author

2004-03-29 16:42 UTC #5 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 6 lines · Rusty Lynch

Quoted 4 lines · Mark Fasheh
Quoted 1 line · Rusty Lynch
How about something like...
Could you move the MOD_INC_USE_COUNT / MOD_DEC_USE_COUNT macros out of the generic function too?

Quoted 7 lines

Can do
Quoted 3 lines
We could make ocfs_set_blocksize an inline, unless you think it's too big for that...
I should be inline. Let me change that.

ah... first of all, s/I should be inline/It should be inlined/ hey, it's not like englihs is first (and only) language or anything :->

The only thing left in __ocfs_read_super that requires #if LINUX.../#endif's is the ocfs_iget/iget4 stuff, which is one of those areas that I have been meaning to deal with... ocfs_iget() should be what get's called regardless of 2.4/2.6, and then deal with 2.4 verses 2.6 specifics in that function instead of sprinkled over super/namei/hash.

You mind if I tackle this in the same patch, or do you want two patches? (It's just that ocfs_iget needs to be fixed before __ocfs_read_super can be made clean.)

    --rusty

Mark Fasheh

2004-03-29 17:06 UTC #6 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 2 lines · Rusty Lynch

ah... first of all, s/I should be inline/It should be inlined/ hey, it's not like englihs is first (and only) language or anything :->

heh, i dont' now wat your taking abuot

Quoted 8 lines

The only thing left in __ocfs_read_super that requires #if LINUX.../#endif's is the ocfs_iget/iget4 stuff, which is one of those areas that I have been meaning to deal with... ocfs_iget() should be what get's called regardless of 2.4/2.6, and then deal with 2.4 verses 2.6 specifics in that function instead of sprinkled over super/namei/hash.
You mind if I tackle this in the same patch, or do you want two patches? (It's just that ocfs_iget needs to be fixed before __ocfs_read_super can be made clean.)

Actually, two patches please. I don't mind the extra over #ifdef, and it'll give us time to run with the new read_super while you deal with the inode stuff (which I'd like to test seperately anyway).

While we're on the topic of iget, here's a thought. Theoretically we don't need iget4/iget5 anymore, as our inode numbers are no longer tied to 64 bit disk offsets and instead are taken from iunique, which, as the name indicates, gives you a unique inode number. We should be able to just always use iget instead, which is much simpler.

Of course, eliminating the #ifdef ... iget4 #else ocfs_iget #endif stuff is a step in a good direction. However, if you want to give moving to iget a shot, I'd be more than happy to help out with at least the 2.4 side of things (testing, etc)

	--Mark

--
Mark Fasheh
Software Developer, Oracle Corp
[email protected]

Rusty Lynch

author

2004-03-29 17:46 UTC #7 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 23 lines · Mark Fasheh

Quoted 2 lines · Rusty Lynch
ah... first of all, s/I should be inline/It should be inlined/ hey, it's not like englihs is first (and only) language or anything :->
heh, i dont' now wat your taking abuot
Quoted 8 lines
The only thing left in __ocfs_read_super that requires #if LINUX.../#endif's is the ocfs_iget/iget4 stuff, which is one of those areas that I have been meaning to deal with... ocfs_iget() should be what get's called regardless of 2.4/2.6, and then deal with 2.4 verses 2.6 specifics in that function instead of sprinkled over super/namei/hash.
You mind if I tackle this in the same patch, or do you want two patches? (It's just that ocfs_iget needs to be fixed before __ocfs_read_super can be made clean.)
Actually, two patches please. I don't mind the extra over #ifdef, and it'll give us time to run with the new read_super while you deal with the inode stuff (which I'd like to test seperately anyway).
While we're on the topic of iget, here's a thought. Theoretically we don't need iget4/iget5 anymore, as our inode numbers are no longer tied to 64 bit disk offsets and instead are taken from iunique, which, as the name indicates, gives you a unique inode number. We should be able to just always use iget instead, which is much simpler.

Quoted 6 lines

Of course, eliminating the #ifdef ... iget4 #else ocfs_iget #endif stuff is a step in a good direction. However, if you want to give moving to iget a shot, I'd be more than happy to help out with at least the 2.4 side of things (testing, etc)
	--Mark

hmm... yea, switching over to pure iget's sounds like a good idea. Let me take a look into it.

Here is my previous patch with the ocfs_set_blocksize make inline, and the addition of OCFS_MOD_[INC|DEC]_USE_COUNT

super.c +75 ~~-115~~

Index: src/super.c
===================================================================
@@ -154,108 +154,32 @@
 
 };
 
+static inline int ocfs_set_blocksize(struct super_block *sb)
+{
+	int status = 0;
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+	if (!sb_set_blocksize(sb, 512))
+		status = -EIO;
+#else
+	/* TODO: fix this */
+	sb->s_blocksize = 512;
+	sb->s_blocksize_bits = 9;
+#if LINUX_VERSION_CODE >= LinuxVersionCode(2,4,18)
+	status = set_blocksize (sb->s_dev, 512);
+#else
+	set_blocksize (sb->s_dev, 512);
+#endif /* < 2.4.18 */
+#endif /* < 2.6.0 */
 
-static int ocfs_fill_super (struct super_block *sb, void *data, int silent)
-{
-	struct dentry *root_dentry;
-	int status = -1;
-	struct inode *root_inode = NULL;
-	__u32 uid = current->fsuid;
-	__u32 gid = current->fsgid;
-	bool reclaim_id;
-        ocfs_super *osb;
-
-	LOG_ENTRY ();
-	
-	if (ocfs_parse_options (data, &uid, &gid, &reclaim_id) != 0) {
-		LOG_ERROR_STR ("bad mount option");
-		status=-EINVAL;
-		goto read_super_error;
-	}
-
-	sb->s_magic = OCFS_MAGIC;
-	sb->s_op = &ocfs_sops;
-	sb->s_flags |= MS_NOATIME;
-
-	/* this is needed to support O_LARGE_FILE */
-	sb->s_maxbytes = OCFS_LINUX_MAX_FILE_SIZE;
-
-	if (!sb_set_blocksize(sb, 512)) {
-		LOG_ERROR_STR("Could not set block size");
-		status=-EIO;
-		goto read_super_error;
-	}
-
-	status = ocfs_mount_volume (sb, reclaim_id, NULL);
-	if (status < 0) {
-		goto read_super_error;
-	}
-	osb = ((ocfs_super *)(sb->s_fs_info));
-	if (!osb) {
-		status=-EINVAL;
-		goto read_super_error;
-	}
-
-	root_inode = ocfs_iget(sb, NULL);
-	if (!root_inode) {
-		status=-EIO;
-		LOG_ERROR_STATUS (status);
-		goto read_super_error;
-	}
-
-	root_dentry = d_alloc_root (root_inode);
-	if (!root_dentry) {
-		status=-ENOMEM;
-		LOG_ERROR_STATUS (status);
-		goto read_super_error;
-	}
-
-	sb->s_root = root_dentry;
-	printk ("ocfs2: Mounting device (%u,%u) on %s (node %d)\n",
-		MAJOR(sb->s_dev), MINOR(sb->s_dev),
-		osb->node_cfg_info[osb->node_num]->node_name, osb->node_num);
-
-	status = 0;
-	LOG_EXIT_STATUS (status);
 	return status;
-
-read_super_error:
-	if (root_inode != NULL) {
-		iput (root_inode);
-		root_inode = NULL;
-	}
-
-	LOG_EXIT_STATUS (status);
-	return status;
-}				/* ocfs_fill_super */
-
-static struct super_block *ocfs_get_sb(struct file_system_type *fs_type, int flags, const char *dev_name, void *data)
-{
-	return get_sb_bdev(fs_type, flags, dev_name, data, ocfs_fill_super);
 }
 
-static struct file_system_type ocfs_fs_type = {
-        .owner          = THIS_MODULE,
-        .name           = "ocfs2",
-        .get_sb         = ocfs_get_sb, /* is this called when we mount
-					* the fs? */
-        .kill_sb        = kill_block_super, /* set to the generic one
-					     * right now, but do we
-					     * need to change that? */
-        .fs_flags       = FS_REQUIRES_DEV,
-	.next           = NULL
-};
-
-#else  /* We're a 2.4 kernel */
-
-
 /*
- * ocfs_read_super()
+ * __ocfs_read_super()
  *
  */
-static struct super_block *ocfs_read_super (struct super_block *sb, void *data, int silent)
+static int __ocfs_read_super(struct super_block *sb, void *data, int silent)
 {
 	struct dentry *root;
 	int status;

@@ -265,27 +189,21 @@
 	bool reclaim_id;
 	ocfs_super *osb = NULL;
 
-	LOG_ENTRY ();
+	LOG_ENTRY_ARGS ("%p, %p, %i", sb, data, silent);
 
-	MOD_INC_USE_COUNT;
+	OCFS_MOD_INC_USE_COUNT;
 
 	if (ocfs_parse_options (data, &uid, &gid, &reclaim_id) != 0) {
+		status = -EINVAL;
 		LOG_ERROR_STR ("ocfs_read_super: bad mount option");
 		goto read_super_error;
 	}
 
-	/* TODO: fix this */
-	sb->s_blocksize = 512;
-	sb->s_blocksize_bits = 9;
-#if LINUX_VERSION_CODE >= LinuxVersionCode(2,4,18)
-	status = set_blocksize (sb->s_dev, 512);
+	status = ocfs_set_blocksize(sb);
 	if (status < 0) {
-		LOG_ERROR_STR ("ocfs_read_super: set_blocksize failed!");
+		LOG_ERROR_STR("unable to set blocksize");
 		goto read_super_error;
 	}
-#else
-	set_blocksize (sb->s_dev, 512);
-#endif
 
 	sb->s_magic = OCFS_MAGIC;
 	sb->s_op = &ocfs_sops;

@@ -295,20 +213,32 @@
 	sb->s_maxbytes = OCFS_LINUX_MAX_FILE_SIZE;
 
 	status = ocfs_mount_volume (sb, reclaim_id, NULL);
+	if (status < 0)
+		goto read_super_error;
+
         osb = (ocfs_super *) OCFS_GENERIC_SB_P(sb);
-	if (status < 0 || !osb)
+	if (!osb) {
+		status = -EINVAL;
 		goto read_super_error;
+	}
 
-	inode = iget4 (sb, OCFS_ROOT_INODE_NUMBER, 0, NULL);
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)	
+	inode = ocfs_iget(sb, NULL);
+#else
+	inode = iget4(sb, OCFS_ROOT_INODE_NUMBER, 0, NULL);
+#endif
 	if (!inode) {
+		status = -EIO;
 		LOG_ERROR_STATUS (status);
 		goto read_super_error;
 	}
 
 	root = d_alloc_root (inode);
 	if (!root) {
+		status = -ENOMEM;
 		LOG_ERROR_STATUS (status);
 		iput (inode);
+		/* should we be setting inode to null?? */
 		goto read_super_error;
 	}

@@ -318,29 +248,59 @@
 		MAJOR(sb->s_dev), MINOR(sb->s_dev),
 		osb->node_cfg_info[osb->node_num]->node_name, osb->node_num);
 
-	LOG_EXIT_PTR (sb);
-	return sb;
+	LOG_EXIT_STATUS(status);
+	return status;		
 
 read_super_error:
 	if (osb)
 		ocfs_dismount_volume (sb);
 
-	MOD_DEC_USE_COUNT;
+	OCFS_MOD_DEC_USE_COUNT;
+
 	if (inode != NULL) {
 		iput (inode);
 		inode = NULL;
 	}
 
-	LOG_EXIT_PTR (0);
-	return NULL;
-}				/* ocfs_read_super */
+	LOG_EXIT_STATUS(status);
+	return status;
+}
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+static int ocfs_read_super (struct super_block *sb, void *data, int silent)
+{
+	return __ocfs_read_super(sb, data, silent);
+}
+#else
+static struct super_block *ocfs_read_super (struct super_block *sb, void *data, int silent)
+{
+	if (__ocfs_read_super(sb, data, silent) < 0)
+		return NULL;
+	return sb;
+}
+#endif
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+static struct super_block *ocfs_get_sb(struct file_system_type *fs_type, int flags, const char *dev_name, void *data)
+{
+	return get_sb_bdev(fs_type, flags, dev_name, data, ocfs_read_super);
+}
+
+static struct file_system_type ocfs_fs_type = {
+        .owner          = THIS_MODULE,
+        .name           = "ocfs2",
+        .get_sb         = ocfs_get_sb, /* is this called when we mount
+					* the fs? */
+        .kill_sb        = kill_block_super, /* set to the generic one
+					     * right now, but do we
+					     * need to change that? */
+        .fs_flags       = FS_REQUIRES_DEV,
+	.next           = NULL
+};
+#else
 static DECLARE_FSTYPE (ocfs_fs_type, "ocfs2", ocfs_read_super, FS_REQUIRES_DEV);
-#endif /* #if 2.6 kernel ... #else */
+#endif
 
-
-
 /*
  * ocfs_parse_options()
  *

inc/ocfs.h +8 -0

Index: src/inc/ocfs.h
===================================================================
@@ -1159,6 +1159,14 @@
 #define OCFS_GENERIC_SB_P(sb)	((ocfs_super *)(sb->u.generic_sbp))
 #endif
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+#define OCFS_MOD_INC_USE_COUNT
+#define OCFS_MOD_DEC_USE_COUNT
+#else
+#define OCFS_MOD_INC_USE_COUNT        MOD_INC_USE_COUNT
+#define OCFS_MOD_DEC_USE_COUNT        MOD_INC_USE_COUNT
+#endif
+
 extern __u32 debug_context;
 extern __u32 debug_level;
 extern __u32 debug_exclude;

Mark Fasheh

2004-03-29 20:44 UTC #8 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 2 lines · Rusty Lynch

Here is my previous patch with the ocfs_set_blocksize make inline, and the addition of OCFS_MOD_[INC|DEC]_USE_COUNT

Ok, this is in. I fixed a couple of things, one of them being the whole OCFS_DEC/INC_INODE_COUNT which I removed, and just put the kernel macros in the 2.4 read_super.

You'll prolly get conflicts when updating :(

Let me know if I messed anything up for 2.6, but I seriously doubt it :)

	--Mark

--
Mark Fasheh
Software Developer, Oracle Corp
[email protected]

Rusty Lynch

author

2004-03-30 11:28 UTC #9 lore raw

PATCH 1/1 remove ocfs_fill_super and port ocfs_read_super

Quoted 6 lines · Mark Fasheh

Quoted 2 lines · Rusty Lynch
Here is my previous patch with the ocfs_set_blocksize make inline, and the addition of OCFS_MOD_[INC|DEC]_USE_COUNT
Ok, this is in. I fixed a couple of things, one of them being the whole OCFS_DEC/INC_INODE_COUNT which I removed, and just put the kernel macros in the 2.4 read_super.

Ahh... good idea on where to put the mod inc/dec.

Quoted 2 lines

You'll prolly get conflicts when updating :(

Nope. I make a point of starting from a fresh tree when ever possible (less chance of munging up future patches)

Quoted 3 lines

Let me know if I messed anything up for 2.6, but I seriously doubt it :)
	--Mark

Works for me

Quoted 5 lines

--
Mark Fasheh
Software Developer, Oracle Corp
[email protected]