subject:"\[PATCH\] Ext4\: Uninitialized Block Groups"

Re: [PATCH] Ext4: Uninitialized Block Groups

2007-09-20 Thread Andrew Morton

On Tue, 18 Sep 2007 17:25:31 -0700
Avantika Mathur [EMAIL PROTECTED] wrote:

 In pass1 of e2fsck, every inode table in the fileystem is scanned and 
 checked, 
 regardless of whether it is in use.  This is this the most time consuming 
 part 
 of the filesystem check.  The unintialized block group feature can greatly 
 reduce e2fsck time by eliminating checking of uninitialized inodes.  
 
 With this feature, there is a a high water mark of used inodes for each block 
 group.  Block and inode bitmaps can be uninitialized on disk via a flag in the
 group descriptor to avoid reading or scanning them at e2fsck time.  A checksum
 of each group descriptor is used to ensure that corruption in the group
 descriptor's bit flags does not cause incorrect operation.

This needed a few fixups due to conflicts with
ext2-ext3-ext4-add-block-bitmap-validation.patch but they were pretty
straightforward.  Please check that the result is OK.


-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Ext4: Uninitialized Block Groups

2007-09-19 Thread Valerie Clement

Hi Avantika,
I ran some tests with the uninit_groups feature enabled and got error messages
when running e2fsck on my ext4 partition. e2fsck complains of an invalid 
unused inodes count in some group descriptors.
These errors occur when checking groups which have only one inode in use. The 
free inodes count has been decremented by one in these groups but not the 
unused inodes count.

The following patch fixes the problem.

Andreas, could you check if my patch is correct?
Thanks a lot,
  Valérie

Index: linux-2.6.23-rc6/fs/ext4/ialloc.c
===
--- linux-2.6.23-rc6.orig/fs/ext4/ialloc.c  2007-09-19 11:31:01.0 
+0200
+++ linux-2.6.23-rc6/fs/ext4/ialloc.c   2007-09-19 11:31:41.0 +0200
@@ -633,13 +633,10 @@ got:
/* If we didn't allocate from within the initialized part of the inode
 * table then we need to initialize up to this inode. */
if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
-   if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
+   if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT))
gdp-bg_flags = cpu_to_le16(~EXT4_BG_INODE_UNINIT);
-   free = EXT4_INODES_PER_GROUP(sb);
-   } else {
-   free = EXT4_INODES_PER_GROUP(sb) -
+   free = EXT4_INODES_PER_GROUP(sb) -
le16_to_cpu(gdp-bg_itable_unused);
-   }
 
if (ino  free)
gdp-bg_itable_unused =


-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Ext4: Uninitialized Block Groups

2007-09-19 Thread Aneesh Kumar K.V




Valerie Clement wrote:

Hi Avantika,
I ran some tests with the uninit_groups feature enabled and got error messages
when running e2fsck on my ext4 partition. e2fsck complains of an invalid 
unused inodes count in some group descriptors.
These errors occur when checking groups which have only one inode in use. The 
free inodes count has been decremented by one in these groups but not the 
unused inodes count.


The following patch fixes the problem.

Andreas, could you check if my patch is correct?
Thanks a lot,
  Valérie

Index: linux-2.6.23-rc6/fs/ext4/ialloc.c
===
--- linux-2.6.23-rc6.orig/fs/ext4/ialloc.c  2007-09-19 11:31:01.0 
+0200
+++ linux-2.6.23-rc6/fs/ext4/ialloc.c   2007-09-19 11:31:41.0 +0200
@@ -633,13 +633,10 @@ got:
/* If we didn't allocate from within the initialized part of the inode
 * table then we need to initialize up to this inode. */
if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
-   if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
+   if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT))
gdp-bg_flags = cpu_to_le16(~EXT4_BG_INODE_UNINIT);
-   free = EXT4_INODES_PER_GROUP(sb);
-   } else {
-   free = EXT4_INODES_PER_GROUP(sb) -
+   free = EXT4_INODES_PER_GROUP(sb) -
le16_to_cpu(gdp-bg_itable_unused);
-   }




the variable free is confusingly named here. It is not the free inode count.
rather it indicate the last used relative inode number in the group. How about
the below ?

-aneesh

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 4250c02..cfe2e09 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -633,17 +633,21 @@ got:
/* If we didn't allocate from within the initialized part of the inode
 * table then we need to initialize up to this inode. */
if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
-   if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
+   if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT))
gdp-bg_flags = cpu_to_le16(~EXT4_BG_INODE_UNINIT);
-   free = EXT4_INODES_PER_GROUP(sb);
-   } else {
-   free = EXT4_INODES_PER_GROUP(sb) -
-   le16_to_cpu(gdp-bg_itable_unused);
-   }

-   if (ino  free)
+   /*
+* Check the relative inode number against the last used
+* relative inode number in this group. if it is greater
+* we need to  update the bg_itable_unused count
+*
+*/
+   if (ino  (EXT4_INODES_PER_GROUP(sb) -
+   le16_to_cpu(gdp-bg_itable_unused))) {
+
gdp-bg_itable_unused =
cpu_to_le16(EXT4_INODES_PER_GROUP(sb) - ino);
+   }
}

gdp-bg_free_inodes_count =
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Ext4: Uninitialized Block Groups

2007-09-19 Thread Valerie Clement


Aneesh Kumar K.V wrote:
the variable free is confusingly named here. It is not the free inode 
count.
rather it indicate the last used relative inode number in the group. How 
about

the below ?

-aneesh


I agree, it's better.
  Valérie



diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 4250c02..cfe2e09 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -633,17 +633,21 @@ got:
/* If we didn't allocate from within the initialized part of the inode
 * table then we need to initialize up to this inode. */
if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
-if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
+if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT))
gdp-bg_flags = cpu_to_le16(~EXT4_BG_INODE_UNINIT);
-free = EXT4_INODES_PER_GROUP(sb);
-} else {
-free = EXT4_INODES_PER_GROUP(sb) -
-le16_to_cpu(gdp-bg_itable_unused);
-}

-if (ino  free)
+/*
+ * Check the relative inode number against the last used
+ * relative inode number in this group. if it is greater
+ * we need to  update the bg_itable_unused count
+ *
+ */
+if (ino  (EXT4_INODES_PER_GROUP(sb) -
+le16_to_cpu(gdp-bg_itable_unused))) {
+
gdp-bg_itable_unused =
cpu_to_le16(EXT4_INODES_PER_GROUP(sb) - ino);
+}
}

gdp-bg_free_inodes_count =



-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Ext4: Uninitialized Block Groups

2007-09-19 Thread Andreas Dilger

On Sep 19, 2007  14:06 +0200, Valerie Clement wrote:
 I ran some tests with the uninit_groups feature enabled and got error messages
 when running e2fsck on my ext4 partition. e2fsck complains of an invalid 
 unused inodes count in some group descriptors.
 These errors occur when checking groups which have only one inode in use. The 
 free inodes count has been decremented by one in these groups but not the 
 unused inodes count.
 
 Index: linux-2.6.23-rc6/fs/ext4/ialloc.c
 ===
 --- linux-2.6.23-rc6.orig/fs/ext4/ialloc.c2007-09-19 11:31:01.0 
 +0200
 +++ linux-2.6.23-rc6/fs/ext4/ialloc.c 2007-09-19 11:31:41.0 +0200
 @@ -633,13 +633,10 @@ got:
   /* If we didn't allocate from within the initialized part of the inode
* table then we need to initialize up to this inode. */
   if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
 - if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
 + if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT))
   gdp-bg_flags = cpu_to_le16(~EXT4_BG_INODE_UNINIT);
 - free = EXT4_INODES_PER_GROUP(sb);
 - } else {
 - free = EXT4_INODES_PER_GROUP(sb) -
 + free = EXT4_INODES_PER_GROUP(sb) -
   le16_to_cpu(gdp-bg_itable_unused);
 - }

Hmm, this is indeed incorrect, but I'm not sure solution is the right one.
I guess in our testing we ran it for a long time and must have created
more than a single inode per group...

What about the following instead?  I think the assumption in the original
code is that it's a new group, all the inodes are free, but that is not
correct - we want to make NONE of the inodes free initially so that
bt_itable_unused is recalculated below:

if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
gdp-bg_flags = cpu_to_le16(~EXT4_BG_INODE_UNINIT);
-   free = EXT4_INODES_PER_GROUP(sb);
+   free = 0;
} else {
free = EXT4_INODES_PER_GROUP(sb) -
le16_to_cpu(gdp-bg_itable_unused);
}

 if (ino  free)
   gdp-bg_itable_unused =
cpu_to_le16(EXT3_INODES_PER_GROUP(sb) - ino);

Still a bit uneasy about off-by-one errors here though.  Is ino 0 or
1 for the first inode in the group.  We might need to have a -1 in the
bg_itable_unused calculation still.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Ext4: Uninitialized Block Groups

2007-09-19 Thread Avantika Mathur


Andreas Dilger wrote:

On Sep 18, 2007  20:03 -0700, Andrew Morton wrote:
  

On Tue, 18 Sep 2007 17:25:31 -0700 Avantika Mathur [EMAIL PROTECTED] wrote:



+#if !defined(CONFIG_CRC16)
+/** CRC table for the CRC-16. The poly is 0x8005 (x16 + x15 + x2 + 1) */
+__u16 const crc16_table[256] = { me
+   0x, 0xC0C1, 0xC181, 0x0140, 0xC301, 0x03C0, 0x0280, 0xC241,
  

That's rather sad.  A plain old depends on would be better.



My bad.  We wrote this patch and it had to run on older kernels that might
not even have lib/crc16.c (it was added around 2.6.15 or so, so e.g. RHEL4
doesn't have it).  I forgot to remove it from the upstream submission,
and since it didn't cause problems nobody else complained...

The incremental patch below removes the local crc16 code, and also resolves an
issue with properly updating bg_itable_unused when an inode is allocated in an 
unused block groups.


Thanks
Avantika

---
fs/Kconfig   |1 +
fs/ext4/ialloc.c |8 +++-
fs/ext4/super.c  |   51 +--
3 files changed, 9 insertions(+), 51 deletions(-)

Index: linux-2.6.23-rc6/fs/ext4/ialloc.c
===
--- linux-2.6.23-rc6.orig/fs/ext4/ialloc.c  2007-09-19 15:38:21.0 
-0700
+++ linux-2.6.23-rc6/fs/ext4/ialloc.c   2007-09-19 15:41:11.0 -0700
@@ -635,12 +635,18 @@
if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
if (gdp-bg_flags  cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
gdp-bg_flags = cpu_to_le16(~EXT4_BG_INODE_UNINIT);
-   free = EXT4_INODES_PER_GROUP(sb);
+   free = 0;
} else {
free = EXT4_INODES_PER_GROUP(sb) -
le16_to_cpu(gdp-bg_itable_unused);
}

+   /*
+* Check the relative inode number against the last used
+* relative inode number in this group. if it is greater
+* we need to  update the bg_itable_unused count
+*
+*/
if (ino  free)
gdp-bg_itable_unused =
cpu_to_le16(EXT4_INODES_PER_GROUP(sb) - ino);
Index: linux-2.6.23-rc6/fs/ext4/super.c
===
--- linux-2.6.23-rc6.orig/fs/ext4/super.c   2007-09-19 15:38:21.0 
-0700
+++ linux-2.6.23-rc6/fs/ext4/super.c2007-09-19 15:38:51.0 -0700
@@ -37,6 +37,7 @@
#include linux/quotaops.h
#include linux/seq_file.h
#include linux/log2.h
+#include linux/crc16.h

#include asm/uaccess.h

@@ -1248,56 +1249,6 @@
return res;
}

-#if !defined(CONFIG_CRC16)
-/** CRC table for the CRC-16. The poly is 0x8005 (x16 + x15 + x2 + 1) */
-__u16 const crc16_table[256] = {
-   0x, 0xC0C1, 0xC181, 0x0140, 0xC301, 0x03C0, 0x0280, 0xC241,
-   0xC601, 0x06C0, 0x0780, 0xC741, 0x0500, 0xC5C1, 0xC481, 0x0440,
-   0xCC01, 0x0CC0, 0x0D80, 0xCD41, 0x0F00, 0xCFC1, 0xCE81, 0x0E40,
-   0x0A00, 0xCAC1, 0xCB81, 0x0B40, 0xC901, 0x09C0, 0x0880, 0xC841,
-   0xD801, 0x18C0, 0x1980, 0xD941, 0x1B00, 0xDBC1, 0xDA81, 0x1A40,
-   0x1E00, 0xDEC1, 0xDF81, 0x1F40, 0xDD01, 0x1DC0, 0x1C80, 0xDC41,
-   0x1400, 0xD4C1, 0xD581, 0x1540, 0xD701, 0x17C0, 0x1680, 0xD641,
-   0xD201, 0x12C0, 0x1380, 0xD341, 0x1100, 0xD1C1, 0xD081, 0x1040,
-   0xF001, 0x30C0, 0x3180, 0xF141, 0x3300, 0xF3C1, 0xF281, 0x3240,
-   0x3600, 0xF6C1, 0xF781, 0x3740, 0xF501, 0x35C0, 0x3480, 0xF441,
-   0x3C00, 0xFCC1, 0xFD81, 0x3D40, 0xFF01, 0x3FC0, 0x3E80, 0xFE41,
-   0xFA01, 0x3AC0, 0x3B80, 0xFB41, 0x3900, 0xF9C1, 0xF881, 0x3840,
-   0x2800, 0xE8C1, 0xE981, 0x2940, 0xEB01, 0x2BC0, 0x2A80, 0xEA41,
-   0xEE01, 0x2EC0, 0x2F80, 0xEF41, 0x2D00, 0xEDC1, 0xEC81, 0x2C40,
-   0xE401, 0x24C0, 0x2580, 0xE541, 0x2700, 0xE7C1, 0xE681, 0x2640,
-   0x2200, 0xE2C1, 0xE381, 0x2340, 0xE101, 0x21C0, 0x2080, 0xE041,
-   0xA001, 0x60C0, 0x6180, 0xA141, 0x6300, 0xA3C1, 0xA281, 0x6240,
-   0x6600, 0xA6C1, 0xA781, 0x6740, 0xA501, 0x65C0, 0x6480, 0xA441,
-   0x6C00, 0xACC1, 0xAD81, 0x6D40, 0xAF01, 0x6FC0, 0x6E80, 0xAE41,
-   0xAA01, 0x6AC0, 0x6B80, 0xAB41, 0x6900, 0xA9C1, 0xA881, 0x6840,
-   0x7800, 0xB8C1, 0xB981, 0x7940, 0xBB01, 0x7BC0, 0x7A80, 0xBA41,
-   0xBE01, 0x7EC0, 0x7F80, 0xBF41, 0x7D00, 0xBDC1, 0xBC81, 0x7C40,
-   0xB401, 0x74C0, 0x7580, 0xB541, 0x7700, 0xB7C1, 0xB681, 0x7640,
-   0x7200, 0xB2C1, 0xB381, 0x7340, 0xB101, 0x71C0, 0x7080, 0xB041,
-   0x5000, 0x90C1, 0x9181, 0x5140, 0x9301, 0x53C0, 0x5280, 0x9241,
-   0x9601, 0x56C0, 0x5780, 0x9741, 0x5500, 0x95C1, 0x9481, 0x5440,
-   0x9C01, 0x5CC0, 0x5D80, 0x9D41, 0x5F00, 0x9FC1, 0x9E81, 0x5E40,
-   0x5A00, 0x9AC1, 0x9B81, 0x5B40, 0x9901, 0x59C0, 0x5880, 0x9841,
-   0x8801, 0x48C0, 0x4980, 0x8941, 0x4B00, 0x8BC1, 0x8A81,

Re: [PATCH] Ext4: Uninitialized Block Groups

2007-09-18 Thread Andrew Morton

On Tue, 18 Sep 2007 17:25:31 -0700 Avantika Mathur [EMAIL PROTECTED] wrote:

 +
 +__u16 crc16(__u16 crc, __u8 const *buffer, size_t len)

And is we really really have to do this, then the ext4-private crc16() 
should have static scope.
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Ext4: Uninitialized Block Groups

Re: [PATCH] Ext4: Uninitialized Block Groups

Re: [PATCH] Ext4: Uninitialized Block Groups

Re: [PATCH] Ext4: Uninitialized Block Groups

Re: [PATCH] Ext4: Uninitialized Block Groups

Re: [PATCH] Ext4: Uninitialized Block Groups

Re: [PATCH] Ext4: Uninitialized Block Groups

7 matches

Site Navigation

Mail list logo

Footer information