[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[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.

    --rusty

Index: src/super.c
===================================================================
--- src/super.c	(revision 812)
+++ src/super.c	(working copy)
@@ -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()

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Joel Becker]

On Thu, Mar 25, 2004 at 06:59:49PM -0800, Rusty Lynch wrote:
> 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

-- 

"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: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Rusty Lynch]

On Fri, Mar 26, 2004 at 06:34:54AM -0800, Joel Becker wrote:
> On Thu, Mar 25, 2004 at 06:59:49PM -0800, Rusty Lynch wrote:
> > 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


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


Index: src/super.c
===================================================================
--- src/super.c	(revision 812)
+++ src/super.c	(working copy)
@@ -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()
  *
Index: src/inc/io.h
===================================================================
--- src/inc/io.h	(revision 812)
+++ src/inc/io.h	(working copy)
@@ -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, 

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Mark Fasheh]

On Fri, Mar 26, 2004 at 12:46:38PM -0800, Rusty Lynch wrote:
> 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
mark.fasheh@oracle.com

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Rusty Lynch]

On Mon, Mar 29, 2004 at 01:29:47PM -0800, Mark Fasheh wrote:
> On Fri, Mar 26, 2004 at 12:46:38PM -0800, Rusty Lynch wrote:
> > 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

> 
> 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.

> 	--Mark
> 
> --
> Mark Fasheh
> Software Developer, Oracle Corp
> mark.fasheh@oracle.com

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Rusty Lynch]

On Mon, Mar 29, 2004 at 01:31:58PM -0800, Rusty Lynch wrote:
> On Mon, Mar 29, 2004 at 01:29:47PM -0800, Mark Fasheh wrote:
> > On Fri, Mar 26, 2004 at 12:46:38PM -0800, Rusty Lynch wrote:
> > > 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
> 
> > 
> > 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 

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Mark Fasheh]

On Mon, Mar 29, 2004 at 02:37:54PM -0800, Rusty Lynch wrote:
> 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

> 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
mark.fasheh@oracle.com

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Rusty Lynch]

On Mon, Mar 29, 2004 at 03:06:09PM -0800, Mark Fasheh wrote:
> On Mon, Mar 29, 2004 at 02:37:54PM -0800, Rusty Lynch wrote:
> > 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
> 
> > 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
> 

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


Index: src/super.c
===================================================================
--- src/super.c	(revision 820)
+++ src/super.c	(working copy)
@@ -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()
  *
Index: src/inc/ocfs.h
===================================================================
--- src/inc/ocfs.h	(revision 820)
+++ src/inc/ocfs.h	(working copy)
@@ -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;

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Mark Fasheh]

On Mon, Mar 29, 2004 at 03:46:03PM -0800, Rusty Lynch wrote:
> 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
mark.fasheh@oracle.com

[Ocfs2-devel] [PATCH]remove ocfs_fill_super and port ocfs_read_super

[Rusty Lynch]

On Mon, Mar 29, 2004 at 06:44:21PM -0800, Mark Fasheh wrote:
> On Mon, Mar 29, 2004 at 03:46:03PM -0800, Rusty Lynch wrote:
> > 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.
> 
> 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)
> 
> Let me know if I messed anything up for 2.6, but I seriously doubt it :)
> 	--Mark

Works for me 
> 
> --
> Mark Fasheh
> Software Developer, Oracle Corp
> mark.fasheh@oracle.com