Add online resize support for ocfs2, take 3

PATCH New Tao Ma 3 participants 9 messages Dec 13, 2007 – Dec 17, 2007

Subsystems (2)

Documentation OCFS2

lore

Share & tools

lore

Tao Ma

author

2007-12-13 23:40 UTC #0 lore raw

cover Add online resize support for ocfs2, take 3

Change from V2 to V3 can be found in the patches themselves.

User can do offline resize using tunefs.ocfs2 when a volume isn't mounted. Now the support for online resize is added into ocfs2.

Please note that the node where online resize goes must already has the volume mounted. We don't mount it behind the user and the operation would fail if we find it isn't mounted. As for other nodes, we don't care whether the volume is mounted or not.

global_bitmap, super block and all the backups will be updated in the kernel. And if super block or backup's update fails, we just output some error message in dmesg and continue the work.

Tao Ma

author

2007-12-13 23:54 UTC #1 lore raw

PATCH 1/4 Initalize bitmap_cpg of ocfs2_super to be the maximum,take 3

This value is initialized from global_bitmap->id2.i_chain.cl_cpg. If there is only 1 group, it will be equal to the total clusters in the volume. So as for online resize, it should change for all the nodes in the cluster. It isn't easy and there is no corresponding lock for it.

bitmap_cpg is only used in 2 areas:
1. Check whether the suballoc is too large for us to allocate from the global bitmap, so it is little used. And now the suballoc size is 2048, it rarely meet this situation and the check is almost useless.
2. Calculate which group a cluster belongs to. We use it during truncate to figure out which cluster group an extent belongs too. But we should be OK if we increase it though as the cluster group calculated shouldn't change and we only ever have a small bitmap_cpg on file systems with a single cluster group.

Signed-off-by Tao Ma <[email protected]>

Signed-off-by Mark Fasheh <[email protected]>

---
 fs/ocfs2/super.c |   19 +------------------
 1 files changed, 1 insertions(+), 18 deletions(-)

fs/ocfs2/super.c +1 ~~-18~~

index 5ee7754..192c7db 100644
@@ -1315,7 +1315,6 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	int i, cbits, bbits;
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data;
 	struct inode *inode = NULL;
-	struct buffer_head *bitmap_bh = NULL;
 	struct ocfs2_journal *journal;
 	__le32 uuid_net_key;
 	struct ocfs2_super *osb;

@@ -1539,25 +1538,9 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	}
 
 	osb->bitmap_blkno = OCFS2_I(inode)->ip_blkno;
-
-	/* We don't have a cluster lock on the bitmap here because
-	 * we're only interested in static information and the extra
-	 * complexity at mount time isn't worht it. Don't pass the
-	 * inode in to the read function though as we don't want it to
-	 * be put in the cache. */
-	status = ocfs2_read_block(osb, osb->bitmap_blkno, &bitmap_bh, 0,
-				  NULL);
 	iput(inode);
-	if (status < 0) {
-		mlog_errno(status);
-		goto bail;
-	}
 
-	di = (struct ocfs2_dinode *) bitmap_bh->b_data;
-	osb->bitmap_cpg = le16_to_cpu(di->id2.i_chain.cl_cpg);
-	brelse(bitmap_bh);
-	mlog(0, "cluster bitmap inode: %llu, clusters per group: %u\n",
-	     (unsigned long long)osb->bitmap_blkno, osb->bitmap_cpg);
+	osb->bitmap_cpg = ocfs2_group_bitmap_size(sb) * 8;
 
 	status = ocfs2_init_slot_info(osb);
 	if (status < 0) {

Signature · 1 line

gitgui.0.9.0.gd794

Tao Ma

author

2007-12-13 23:55 UTC #2 lore raw

PATCH 2/4 Add new ioctl number for OCFS2.take 2

In reply to Tao Ma

OCFS2 need a new ioctl number to call some ioctl.
So add it in ioctl-number.txt.

Signed-off-by Tao Ma <[email protected]>

---
 Documentation/ioctl-number.txt |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Documentation/ioctl-number.txt +1 -0

index 5c7fbf9..9c47f6d 100644
@@ -138,6 +138,7 @@ Code	Seq#	Include File		Comments
 'm'	00-1F	net/irda/irmod.h	conflict!
 'n'	00-7F	linux/ncp_fs.h
 'n'	E0-FF	video/matrox.h          matroxfb
+'o'	00-0F	fs/ocfs2/ocfs2_fs.h	OCFS2
 'p'	00-0F	linux/phantom.h		conflict! (OpenHaptics needs this)
 'p'	00-3F	linux/mc146818rtc.h	conflict!
 'p'	40-7F	linux/nvram.h

Signature · 1 line

gitgui.0.9.0.gd794

Mark Fasheh

2007-12-17 14:00 UTC #3 lore raw

PATCH 2/4 Add new ioctl number for OCFS2.take 2

Quoted 13 lines · tao.ma

OCFS2 need a new ioctl number to call some ioctl.
So add it in ioctl-number.txt.
Signed-off-by Tao Ma <[email protected]>
---
 Documentation/ioctl-number.txt |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Documentation/ioctl-number.txt +0 -0
index 5c7fbf9..9c47f6d 100644

Documentation/ioctl-number.txt Code Seq# Include File Comments · line 141

Quoted 4 lines

Documentation/ioctl-number.txt +1 -0

@@ -138,6 +138,7 @@ Code	Seq#	Include File		Comments
 'm'	00-1F	net/irda/irmod.h	conflict!
 'n'	00-7F	linux/ncp_fs.h
 'n'	E0-FF	video/matrox.h          matroxfb
+'o'	00-0F	fs/ocfs2/ocfs2_fs.h	OCFS2

00-1F - we might as well grab a nice sized chunk while we're doing this.

	--Mark

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

Tao Ma

author

2007-12-13 23:57 UTC #4 lore raw

PATCH 3/4 Add group extend for online resize.take 3

In reply to Tao Ma

Modification from V2 to V3:
1. Modify the call sequence for ocfs2_commit_trans so that we will always call it after a successful start_trans.
2 Add some rollback actions to "roll back" the modifications done to the
last group when we fail to change the inode.
3. Add the update of i_size of the inode.
4. Use the new defined ioctl numbers and "int" as the paramter.
5. Add the check of ocfs2_check_group_descriptor() for the last group before we use it.
6. Some minor fix.

User can do offline resize using tunefs.ocfs2 when a volume isn't mounted. Now the support for online resize is added into ocfs2.

global_bitmap, super block and all the backups will be updated in the kernel. And if super block or backup's update fails, we just output some error message in dmesg and continue the work.

The whole process is derived from ext3 and divided into 2 steps:
1. If the last group isn't full, tunefs.ocfs2 will call OCFS2_IOC_GROUP_EXTEND first to extend it. All the main work is done in kernel.
2. For every new groups, tunefs.ocfs2 will call OCFS2_IOC_GROUP_ADD to add them one by one. The new group descriptor is initialized in userspace, we only check it in the kernel and update the global_bitap, super blocks etc.

This patch includes the implementation for the 1st step.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/Makefile         |    1 +
 fs/ocfs2/buffer_head_io.c |   61 +++++++
 fs/ocfs2/buffer_head_io.h |    2 +
 fs/ocfs2/ioctl.c          |    8 +
 fs/ocfs2/journal.h        |    3 +
 fs/ocfs2/ocfs2_fs.h       |    2 +
 fs/ocfs2/resize.c         |  398 +++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/resize.h         |   31 ++++
 fs/ocfs2/suballoc.c       |   11 +-
 fs/ocfs2/suballoc.h       |    8 +
 10 files changed, 518 insertions(+), 7 deletions(-)
 create mode 100644 fs/ocfs2/resize.c
 create mode 100644 fs/ocfs2/resize.h

fs/ocfs2/Makefile +1 -0

index 9fb8132..4691544 100644
@@ -21,6 +21,7 @@ ocfs2-objs := \
 	localalloc.o 		\
 	mmap.o 			\
 	namei.o 		\
+	resize.o		\
 	slot_map.o 		\
 	suballoc.o 		\
 	super.o 		\

fs/ocfs2/buffer_head_io.c +61 -0

64 lines not on this page · raw message

fs/ocfs2/buffer_head_io.h +2 -0

index 6cc2093..c2e7861 100644
@@ -47,6 +47,8 @@ int ocfs2_read_blocks(struct ocfs2_super          *osb,
 		      int                  flags,
 		      struct inode        *inode);
 
+int ocfs2_write_super_or_backup(struct ocfs2_super *osb,
+				struct buffer_head *bh);
 
 #define OCFS2_BH_CACHED            1
 #define OCFS2_BH_READAHEAD         8

fs/ocfs2/ioctl.c +8 -0

index 87dcece..03ee69b 100644
@@ -20,6 +20,7 @@
 
 #include "ocfs2_fs.h"
 #include "ioctl.h"
+#include "resize.h"
 
 #include <linux/ext2_fs.h>

@@ -115,6 +116,7 @@ int ocfs2_ioctl(struct inode * inode, struct file * filp,
 	unsigned int cmd, unsigned long arg)
 {
 	unsigned int flags;
+	int new_clusters;
 	int status;
 	struct ocfs2_space_resv sr;

@@ -140,6 +142,11 @@ int ocfs2_ioctl(struct inode * inode, struct file * filp,
 			return -EFAULT;
 
 		return ocfs2_change_file_space(filp, cmd, &sr);
+	case OCFS2_IOC_GROUP_EXTEND:
+		if (get_user(new_clusters, (int __user *)arg))
+			return -EFAULT;
+
+		return ocfs2_group_extend(inode, new_clusters);
 	default:
 		return -ENOTTY;
 	}

@@ -162,6 +169,7 @@ long ocfs2_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 	case OCFS2_IOC_RESVSP64:
 	case OCFS2_IOC_UNRESVSP:
 	case OCFS2_IOC_UNRESVSP64:
+	case OCFS2_IOC_GROUP_EXTEND:
 		break;
 	default:
 		return -ENOIOCTLCMD;

fs/ocfs2/journal.h +3 -0

index 4b32e09..0ba3a42 100644
@@ -278,6 +278,9 @@ int                  ocfs2_journal_dirty_data(handle_t *handle,
 /* simple file updates like chmod, etc. */
 #define OCFS2_INODE_UPDATE_CREDITS 1
 
+/* group extend. inode update and last group update. */
+#define OCFS2_GROUP_EXTEND_CREDITS	(OCFS2_INODE_UPDATE_CREDITS + 1)
+
 /* get one bit out of a suballocator: dinode + group descriptor +
  * prev. group desc. if we relink. */
 #define OCFS2_SUBALLOC_ALLOC (3)

fs/ocfs2/ocfs2_fs.h +2 -0

index 6ef8767..19ac421 100644
@@ -231,6 +231,8 @@ struct ocfs2_space_resv {
 #define OCFS2_IOC_RESVSP64	_IOW ('X', 42, struct ocfs2_space_resv)
 #define OCFS2_IOC_UNRESVSP64	_IOW ('X', 43, struct ocfs2_space_resv)
 
+#define OCFS2_IOC_GROUP_EXTEND	_IOW('o', 1, int)
+
 /*
  * Journal Flags (ocfs2_dinode.id1.journal1.i_flags)
  */

fs/ocfs2/resize.c new file +398 -0

398 lines not on this page · raw message

fs/ocfs2/resize.h new file +31 -0

new file mode 100644
index 0000000..3acb79a
@@ -0,0 +1,31 @@
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * resize.h
+ *
+ * Function prototypes
+ *
+ * Copyright (C) 2007 Oracle.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#ifndef OCFS2_RESIZE_H
+#define OCFS2_RESIZE_H
+
+int ocfs2_group_extend(struct inode * inode, int new_clusters);
+
+#endif /* OCFS2_RESIZE_H */

fs/ocfs2/suballoc.c +4 -7

index 8f09f52..45060bf 100644
@@ -101,8 +101,6 @@ static inline int ocfs2_block_group_reasonably_empty(struct ocfs2_group_desc *bg
 static inline u32 ocfs2_desc_bitmap_to_cluster_off(struct inode *inode,
 						   u64 bg_blkno,
 						   u16 bg_bit_off);
-static inline u64 ocfs2_which_cluster_group(struct inode *inode,
-					    u32 cluster);
 static inline void ocfs2_block_to_cluster_group(struct inode *inode,
 						u64 data_blkno,
 						u64 *bg_blkno,

@@ -131,9 +129,9 @@ static u32 ocfs2_bits_per_group(struct ocfs2_chain_list *cl)
 }
 
 /* somewhat more expensive than our other checks, so use sparingly. */
-static int ocfs2_check_group_descriptor(struct super_block *sb,
-					struct ocfs2_dinode *di,
-					struct ocfs2_group_desc *gd)
+int ocfs2_check_group_descriptor(struct super_block *sb,
+				 struct ocfs2_dinode *di,
+				 struct ocfs2_group_desc *gd)
 {
 	unsigned int max_bits;

@@ -1443,8 +1441,7 @@ static inline u32 ocfs2_desc_bitmap_to_cluster_off(struct inode *inode,
 
 /* given a cluster offset, calculate which block group it belongs to
  * and return that block offset. */
-static inline u64 ocfs2_which_cluster_group(struct inode *inode,
-					    u32 cluster)
+u64 ocfs2_which_cluster_group(struct inode *inode, u32 cluster)
 {
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	u32 group_no;

fs/ocfs2/suballoc.h +8 -0

index cafe937..8799033 100644
@@ -147,4 +147,12 @@ static inline int ocfs2_is_cluster_bitmap(struct inode *inode)
 int ocfs2_reserve_cluster_bitmap_bits(struct ocfs2_super *osb,
 				      struct ocfs2_alloc_context *ac);
 
+/* given a cluster offset, calculate which block group it belongs to
+ * and return that block offset. */
+u64 ocfs2_which_cluster_group(struct inode *inode, u32 cluster);
+
+/* somewhat more expensive than our other checks, so use sparingly. */
+int ocfs2_check_group_descriptor(struct super_block *sb,
+				 struct ocfs2_dinode *di,
+				 struct ocfs2_group_desc *gd);
 #endif /* _CHAINALLOC_H_ */

Signature · 1 line

gitgui.0.9.0.gd794

Mark Fasheh

2007-12-17 14:10 UTC #5 lore raw

PATCH 3/4 Add group extend for online resize.take 3

On Fri, Dec 14, 2007 at 03:55:51PM +0800, tao.ma wrote:
...

Quoted 71 lines

fs/ocfs2/resize.c new file +71 -0

+static int ocfs2_update_last_group_and_inode(handle_t *handle,
+					     struct inode *bm_inode,
+					     struct buffer_head *bm_bh,
+					     struct buffer_head *group_bh,
+					     u32 first_new_cluster,
+					     int new_clusters)
+{
+	int ret = 0;
+	struct ocfs2_super *osb = OCFS2_SB(bm_inode->i_sb);
+	struct ocfs2_dinode *fe = (struct ocfs2_dinode *) bm_bh->b_data;
+	struct ocfs2_chain_list *cl = &fe->id2.i_chain;
+	struct ocfs2_chain_rec *cr;
+	struct ocfs2_group_desc *group;
+	u16 chain, num_bits, backups = 0;
+	u16 cl_bpc = le16_to_cpu(cl->cl_bpc);
+	u16 cl_cpg = le16_to_cpu(cl->cl_cpg);
+
+	mlog_entry("(new_clusters=%d, first_new_cluster = %u)\n",
+		   new_clusters, first_new_cluster);
+
+	ret = ocfs2_journal_access(handle, bm_inode, group_bh,
+				   OCFS2_JOURNAL_ACCESS_WRITE);
+	if (ret < 0) {
+		mlog_errno(ret);
+		goto out;
+	}
+
+	group = (struct ocfs2_group_desc *) group_bh->b_data;
+
+	/* update the group first. */
+	num_bits = new_clusters * cl_bpc;
+	le16_add_cpu(&group->bg_bits, num_bits);
+	le16_add_cpu(&group->bg_free_bits_count, num_bits);
+
+	/*
+	 * check whether there are some new backup superblocks exist in
+	 * this group and update the group bitmap accordingly.
+	 */
+	if (OCFS2_HAS_COMPAT_FEATURE(osb->sb,
+				     OCFS2_FEATURE_COMPAT_BACKUP_SB)) {
+		backups = ocfs2_calc_new_backup_super(bm_inode,
+						     group,
+						     new_clusters,
+						     first_new_cluster,
+						     cl_cpg, 1);
+		le16_add_cpu(&group->bg_free_bits_count, -1 * backups);
+	}
+
+	ret = ocfs2_journal_dirty(handle, group_bh);
+	if (ret < 0) {
+		mlog_errno(ret);
+		goto out_rollback;
+	}
+
+	/* update the inode accordingly. */
+	ret = ocfs2_journal_access(handle, bm_inode, bm_bh,
+				   OCFS2_JOURNAL_ACCESS_WRITE);
+	if (ret < 0) {
+		mlog_errno(ret);
+		goto out_rollback;
+	}
+
+	chain = le16_to_cpu(group->bg_chain);
+	cr = (&cl->cl_recs[chain]);
+	le32_add_cpu(&cr->c_total, num_bits);
+	le32_add_cpu(&cr->c_free, num_bits);
+	le32_add_cpu(&fe->id1.bitmap1.i_total, num_bits);
+	le32_add_cpu(&fe->i_clusters, new_clusters);
+
+	if (backups) {
+		le32_add_cpu(&cr->c_free, -1 * backups);

fs/ocfs2/resize.c line 169

Quoted 10 lines

fs/ocfs2/resize.c new file +10 -0

+		le32_add_cpu(&fe->id1.bitmap1.i_used, backups);
+	}
+
+	ocfs2_journal_dirty(handle, bm_bh);
+
+	spin_lock(&OCFS2_I(bm_inode)->ip_lock);
+	OCFS2_I(bm_inode)->ip_clusters = le32_to_cpu(fe->i_clusters);
+	le64_add_cpu(&fe->i_size, new_clusters << osb->s_clustersize_bits);
+	spin_unlock(&OCFS2_I(bm_inode)->ip_lock);
+	i_size_write(bm_inode, le64_to_cpu(fe->i_size));

Move the ocfs2_journal_dirty() past this block to make the disk changes easier to track.

Ok, the rest of this looks reasonable. Where are the userspace patches by the way? Since this is fairly tightly coupled to ocfs2-tools, I was hoping to get a look at those before merging this into ocfs2.git.

	--Mark

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

Tao Ma

author

2007-12-13 23:58 UTC #6 lore raw

PATCH 4/4 Implement "GROUP_ADD" for online resize.take 3

In reply to Tao Ma

Modification from V2 to V3:
1. Modify the call sequence for ocfs2_commit_trans so that we will always call it after a successful start_trans.
2. As we need to update the group's bg_next_group in the kernel, move the group read in the main function and add the writeout of this group also.
3. Add the update of i_size of the inode.
4. Use the new defined ioctl numbers.

User can do offline resize using tunefs.ocfs2 when a volume isn't mounted. Now the support for online resize is added into ocfs2.

global_bitmap, super block and all the backups will be updated in the kernel. And if super block or backup's update fails, we just output some error message in dmesg and continue the work.

This patch includes the implentation for the 2nd step.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/ioctl.c    |    9 ++
 fs/ocfs2/journal.h  |    3 +
 fs/ocfs2/ocfs2_fs.h |   12 +++
 fs/ocfs2/resize.c   |  238 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/resize.h   |    1 +
 5 files changed, 263 insertions(+), 0 deletions(-)

fs/ocfs2/ioctl.c +9 -0

index 03ee69b..a4dda60 100644
@@ -119,6 +119,7 @@ int ocfs2_ioctl(struct inode * inode, struct file * filp,
 	int new_clusters;
 	int status;
 	struct ocfs2_space_resv sr;
+	struct ocfs2_new_group_input input;
 
 	switch (cmd) {
 	case OCFS2_IOC_GETFLAGS:

@@ -147,6 +148,12 @@ int ocfs2_ioctl(struct inode * inode, struct file * filp,
 			return -EFAULT;
 
 		return ocfs2_group_extend(inode, new_clusters);
+	case OCFS2_IOC_GROUP_ADD:
+	case OCFS2_IOC_GROUP_ADD64:
+		if (copy_from_user(&input, (int __user *) arg, sizeof(input)))
+			return -EFAULT;
+
+		return ocfs2_group_add(inode, &input);
 	default:
 		return -ENOTTY;
 	}

@@ -170,6 +177,8 @@ long ocfs2_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 	case OCFS2_IOC_UNRESVSP:
 	case OCFS2_IOC_UNRESVSP64:
 	case OCFS2_IOC_GROUP_EXTEND:
+	case OCFS2_IOC_GROUP_ADD:
+	case OCFS2_IOC_GROUP_ADD64:
 		break;
 	default:
 		return -ENOIOCTLCMD;

fs/ocfs2/journal.h +3 -0

index 0ba3a42..220f3e8 100644
@@ -281,6 +281,9 @@ int                  ocfs2_journal_dirty_data(handle_t *handle,
 /* group extend. inode update and last group update. */
 #define OCFS2_GROUP_EXTEND_CREDITS	(OCFS2_INODE_UPDATE_CREDITS + 1)
 
+/* group add. inode update and the new group update. */
+#define OCFS2_GROUP_ADD_CREDITS	(OCFS2_INODE_UPDATE_CREDITS + 1)
+
 /* get one bit out of a suballocator: dinode + group descriptor +
  * prev. group desc. if we relink. */
 #define OCFS2_SUBALLOC_ALLOC (3)

fs/ocfs2/ocfs2_fs.h +12 -0

index 19ac421..4255517 100644
@@ -231,7 +231,19 @@ struct ocfs2_space_resv {
 #define OCFS2_IOC_RESVSP64	_IOW ('X', 42, struct ocfs2_space_resv)
 #define OCFS2_IOC_UNRESVSP64	_IOW ('X', 43, struct ocfs2_space_resv)
 
+/* Used to pass group descriptor data when online resize is done */
+struct ocfs2_new_group_input {
+	__u64 group;		/* Group descriptor's blkno. */
+	__u32 clusters;		/* Total number of clusters in this group */
+	__u32 frees;		/* Total free clusters in this group */
+	__u16 chain;		/* Chain for this group */
+	__u16 reserved1;
+	__u32 reserved2;
+};
+
 #define OCFS2_IOC_GROUP_EXTEND	_IOW('o', 1, int)
+#define OCFS2_IOC_GROUP_ADD	_IOW('o', 2,struct ocfs2_new_group_input)
+#define OCFS2_IOC_GROUP_ADD64	_IOW('o', 3,struct ocfs2_new_group_input)
 
 /*
  * Journal Flags (ocfs2_dinode.id1.journal1.i_flags)

fs/ocfs2/resize.c +238 -0

241 lines not on this page · raw message

fs/ocfs2/resize.h +1 -0

index 3acb79a..f38841a 100644
@@ -27,5 +27,6 @@
 #define OCFS2_RESIZE_H
 
 int ocfs2_group_extend(struct inode * inode, int new_clusters);
+int ocfs2_group_add(struct inode *inode, struct ocfs2_new_group_input *input);
 
 #endif /* OCFS2_RESIZE_H */

Signature · 1 line

gitgui.0.9.0.gd794

Joel Becker

2007-12-14 16:08 UTC #7 lore raw

PATCH 4/4 Implement "GROUP_ADD" for online resize.take 3

Quoted 4 lines · Tao Ma

2. For every new groups, tunefs.ocfs2 will call OCFS2_IOC_GROUP_ADD to add them one by one. The new group descriptor is initialized in userspace, we only check it in the kernel and update the global_bitap, super blocks etc.

I like the patches overall. They are clean and straightforward.

Quoted 10 lines

fs/ocfs2/resize.c +10 -0

+	if (cluster < total_clusters)
+		mlog(0, "add a group which is in the current volume.\n");
+	else if (input->chain >= cl_count)
+		mlog(0, "input chain exceeds the limit.\n");
+	else if (next_free != cl_count && next_free != input->chain)
+		mlog(0, "the add group should be in chain %u\n", next_free);
+	else if (total_clusters + input->clusters < total_clusters)
+		mlog(0, "add group's clusters overflow.\n");
+	else if (input->clusters > cl_cpg)
+		mlog(0, "the cluster exceeds the maximum of a group\n");

fs/ocfs2/resize.c out: · line 493

Quoted 10 lines

fs/ocfs2/resize.c +10 -0

+	else if (input->frees > input->clusters)
+		mlog(0, "the free cluster exceeds the total clusters\n");
+	else if (total_clusters % cl_cpg != 0)
+		mlog(0, "the last group isn't full. Use group extend first.\n");
+	else if (input->group != ocfs2_which_cluster_group(inode, cluster))
+		mlog(0, "group blkno is invalid\n");
+	else if ((ret = ocfs2_check_new_group(inode, di, input, group_bh)))
+		mlog(0, "group descriptor check failed.\n");
+	else
+		ret = 0;

These are some great errors, but the user will never see them.
tunefs.ocfs2 will just report "Invalid argument", which isn't very helpful. The same goes for the errors in the group check function, which returns -EIO. I'd love to see a way for the user to see them. Either targeted errno values that tunefs can translate into strings, or mlogs in dmesg so that tunefs can say "see dmesg for more info". Something like that. Mark points out that this should only be for real errors, not things that are "normal operation" like ENOSPC.

Joel

Signature · 9 lines

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

Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127

Tao Ma

author

2007-12-15 00:44 UTC #8 lore raw

PATCH 4/4 Implement "GROUP_ADD" for online resize.take 3

Joel Becker Wrote:

Quoted 33 lines · Tao Ma

Quoted 6 lines

2. For every new groups, tunefs.ocfs2 will call OCFS2_IOC_GROUP_ADD to add them one by one. The new group descriptor is initialized in userspace, we only check it in the kernel and update the global_bitap, super blocks etc.

I like the patches overall. They are clean and straightforward.

Quoted 11 lines

fs/ocfs2/resize.c +10 -0

+	if (cluster < total_clusters)
+		mlog(0, "add a group which is in the current volume.\n");
+	else if (input->chain >= cl_count)
+		mlog(0, "input chain exceeds the limit.\n");
+	else if (next_free != cl_count && next_free != input->chain)
+		mlog(0, "the add group should be in chain %u\n", next_free);
+	else if (total_clusters + input->clusters < total_clusters)
+		mlog(0, "add group's clusters overflow.\n");
+	else if (input->clusters > cl_cpg)
+		mlog(0, "the cluster exceeds the maximum of a group\n");

fs/ocfs2/resize.c out: · line 493

Quoted 10 lines

fs/ocfs2/resize.c +10 -0

+	else if (input->frees > input->clusters)
+		mlog(0, "the free cluster exceeds the total clusters\n");
+	else if (total_clusters % cl_cpg != 0)
+		mlog(0, "the last group isn't full. Use group extend first.\n");
+	else if (input->group != ocfs2_which_cluster_group(inode, cluster))
+		mlog(0, "group blkno is invalid\n");
+	else if ((ret = ocfs2_check_new_group(inode, di, input, group_bh)))
+		mlog(0, "group descriptor check failed.\n");
+	else
+		ret = 0;

Quoted 9 lines

These are some great errors, but the user will never see them.
tunefs.ocfs2 will just report "Invalid argument", which isn't very helpful. The same goes for the errors in the group check function, which returns -EIO. I'd love to see a way for the user to see them. Either targeted errno values that tunefs can translate into strings, or mlogs in dmesg so that tunefs can say "see dmesg for more info". Something like that. Mark points out that this should only be for real errors, not things that are "normal operation" like ENOSPC.

Yeah, so maybe I need to use mlog(ML_ERROR,...) so that the user can see it in dmesg and let tunefs.ocfs2 print out "see dmesg for more details". Thanks for your review.