Thread: [ext2resize] [patch] ext2fs_allocate_group_table cleanup
Status: Inactive
Brought to you by:
adilger
Menu
▾
▴
ext2resize-devel
|
From: Coywolf Qi H. <qi...@fc...> - 2006-03-17 02:45:11
|
Hello, Cleanup the fake 'if' statement because of (a % b) <= (b - 1). Conceptually, this fix classifies that with stride, BM < IT impossible. It is for ext2resize bug fix. Signed-off-by: Coywolf Qi Hunt <qi...@fc...> --- diff -r 8991ece34224 lib/ext2fs/alloc_tables.c --- a/lib/ext2fs/alloc_tables.c Fri Mar 10 21:39:40 2006 -0500 +++ b/lib/ext2fs/alloc_tables.c Fri Mar 17 10:30:33 2006 +0800 @@ -51,8 +51,6 @@ errcode_t ext2fs_allocate_group_table(ex start_blk = group_blk + fs->inode_blocks_per_group; start_blk += ((fs->stride * group) % (last_blk - start_blk)); - if (start_blk > last_blk) - start_blk = group_blk; } else start_blk = group_blk; -- Coywolf Qi Hunt |
|
From: Andreas D. <ad...@us...> - 2006-03-17 06:39:35
|
On Mar 17, 2006 10:45 +0800, Coywolf Qi Hunt wrote: > Cleanup the fake 'if' statement because of (a % b) <= (b - 1). > Conceptually, this fix classifies that with stride, BM < IT impossible. > It is for ext2resize bug fix. > > Signed-off-by: Coywolf Qi Hunt <qi...@fc...> > --- > > diff -r 8991ece34224 lib/ext2fs/alloc_tables.c > --- a/lib/ext2fs/alloc_tables.c Fri Mar 10 21:39:40 2006 -0500 > +++ b/lib/ext2fs/alloc_tables.c Fri Mar 17 10:30:33 2006 +0800 > @@ -51,8 +51,6 @@ errcode_t ext2fs_allocate_group_table(ex > start_blk = group_blk + fs->inode_blocks_per_group; > start_blk += ((fs->stride * group) % > (last_blk - start_blk)); > - if (start_blk > last_blk) > - start_blk = group_blk; > } else > start_blk = group_blk; Hi, could you please explain a bit more about this? My thought would be that you are correct in saying that this is a "fake 'if' statement", because we know the start_blk will always be less than last_blk in any case. I'm a bit confused about how it relates to a bug in ext2resize however. Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. |
|
From: Coywolf Qi H. <co...@gm...> - 2006-03-17 06:54:32
|
2006/3/17, Andreas Dilger <ad...@us...>: > On Mar 17, 2006 10:45 +0800, Coywolf Qi Hunt wrote: > > Cleanup the fake 'if' statement because of (a % b) <=3D (b - 1). > > Conceptually, this fix classifies that with stride, BM < IT impossible. > > It is for ext2resize bug fix. > > > > Signed-off-by: Coywolf Qi Hunt <qi...@fc...> > > --- > > > > diff -r 8991ece34224 lib/ext2fs/alloc_tables.c > > --- a/lib/ext2fs/alloc_tables.c Fri Mar 10 21:39:40 2006 -0500 > > +++ b/lib/ext2fs/alloc_tables.c Fri Mar 17 10:30:33 2006 +0800 > > @@ -51,8 +51,6 @@ errcode_t ext2fs_allocate_group_table(ex > > start_blk =3D group_blk + fs->inode_blocks_per_group; > > start_blk +=3D ((fs->stride * group) % > > (last_blk - start_blk)); > > - if (start_blk > last_blk) > > - start_blk =3D group_blk; > > } else > > start_blk =3D group_blk; > > Hi, > could you please explain a bit more about this? My thought would be that > you are correct in saying that this is a "fake 'if' statement", because > we know the start_blk will always be less than last_blk in any case. > > I'm a bit confused about how it relates to a bug in ext2resize however. Currently in ext2resize the IT offset is no correct. In order to fix it, I'd first make sure that in any case, with stride, mke2fs will never put BM before IT in a group. -- Coywolf Qi Hunt |
|
From: Andreas D. <ad...@cl...> - 2006-03-17 08:12:47
|
On Mar 17, 2006 14:54 +0800, Coywolf Qi Hunt wrote: > 2006/3/17, Andreas Dilger <ad...@us...>: > > On Mar 17, 2006 10:45 +0800, Coywolf Qi Hunt wrote: > > > Cleanup the fake 'if' statement because of (a % b) <= (b - 1). > > > Conceptually, this fix classifies that with stride, BM < IT impossible. > > > It is for ext2resize bug fix. > > > > > > Signed-off-by: Coywolf Qi Hunt <qi...@fc...> > > > --- > > > > > > diff -r 8991ece34224 lib/ext2fs/alloc_tables.c > > > --- a/lib/ext2fs/alloc_tables.c Fri Mar 10 21:39:40 2006 -0500 > > > +++ b/lib/ext2fs/alloc_tables.c Fri Mar 17 10:30:33 2006 +0800 > > > @@ -51,8 +51,6 @@ errcode_t ext2fs_allocate_group_table(ex > > > start_blk = group_blk + fs->inode_blocks_per_group; > > > start_blk += ((fs->stride * group) % > > > (last_blk - start_blk)); > > > - if (start_blk > last_blk) > > > - start_blk = group_blk; > > > } else > > > start_blk = group_blk; > > > > Hi, > > could you please explain a bit more about this? My thought would be that > > you are correct in saying that this is a "fake 'if' statement", because > > we know the start_blk will always be less than last_blk in any case. > > > > I'm a bit confused about how it relates to a bug in ext2resize however. > > Currently in ext2resize the IT offset is no correct. In order to fix it, > I'd first make sure that in any case, with stride, mke2fs will never > put BM before IT in a group. While good in theory, I don't think any such guarantee can be made. There is no restriction on bitmap placement, and things like resize2fs can change the location of the bitmaps after mke2fs has been run. I think ext2resize needs to do its best with whatever it finds, and if the last group is not the same as the others then it just needs to be handled as well as possible. Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. |
|
From: Theodore Ts'o <ty...@mi...> - 2006-03-17 15:21:51
|
On Fri, Mar 17, 2006 at 01:12:36AM -0700, Andreas Dilger wrote: > While good in theory, I don't think any such guarantee can be made. There > is no restriction on bitmap placement, and things like resize2fs can change > the location of the bitmaps after mke2fs has been run. > > I think ext2resize needs to do its best with whatever it finds, and if > the last group is not the same as the others then it just needs to be > handled as well as possible. Hey Andreas, I've been meaning to ask you.... now that I have online resizing support integrated into resize2fs, is there any other features in ext2resize that isn't in resize2fs? (Aside from ext2prepare, of course.) Regards, - Ted |
|
From: Andreas D. <ad...@cl...> - 2006-03-17 17:19:49
|
On Mar 17, 2006 10:21 -0500, Theodore Ts'o wrote: > I've been meaning to ask you.... now that I have online resizing > support integrated into resize2fs, is there any other features in > ext2resize that isn't in resize2fs? (Aside from ext2prepare, of > course.) What??? I didn't know that was done. Since when did you do that? Excellent news. As for ext2prepare, that is really just resize2fs that doesn't extend the end of the filesystem, and creates/modifies the resize inode. I believe that e2fsck already handles the resize inode, so combining the parts is all that's needed. Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. |
|
From: Theodore Ts'o <ty...@mi...> - 2006-03-17 18:00:52
|
On Fri, Mar 17, 2006 at 10:19:44AM -0700, Andreas Dilger wrote: > On Mar 17, 2006 10:21 -0500, Theodore Ts'o wrote: > > I've been meaning to ask you.... now that I have online resizing > > support integrated into resize2fs, is there any other features in > > ext2resize that isn't in resize2fs? (Aside from ext2prepare, of > > course.) > > What??? I didn't know that was done. Since when did you do that? > Excellent news. Last night, around 1am. :-) It's not yet checked into mercurial yet, but it's working. I still need to quickly update the man page and decide whether or not provide a command-line compatible version of ext2online or just try to tell users/scripts to recode to use resize2fs instead. > As for ext2prepare, that is really just resize2fs that doesn't extend > the end of the filesystem, and creates/modifies the resize inode. I > believe that e2fsck already handles the resize inode, so combining the > parts is all that's needed. Not really. You *can* do it using some somewhat obscure debugfs, followed by running e2fsck -f and answering yes to a whole bunch of scary questions, and in some cases it still might not work. So we still need to enhance either resize2fs or write some new program if we want ext2prepare functionality. One thing I have thought about doing is simply enhacing /usr/src/linux/fs/ext3/resize.c to support metablock groups, and automatically switching to use meta blockgroups once there are no more reserved gdt blocks. This means you no longer need ext2prepare, and aside from some conservative stick-in-the-mud types running RHEL 3 or other distro's with 2.4 kernels (heck, even Debian has finally gotten off of 2.4), all modern Linux systems will support the metablock group feature anyway. This will allow us to do resizing without needing to use ext2prepare, or needing to reserve blocks at all. - Ted |
|
From: Andreas D. <ad...@cl...> - 2006-03-18 07:58:52
|
On Mar 17, 2006 13:00 -0500, Theodore Ts'o wrote: > > As for ext2prepare, that is really just resize2fs that doesn't extend > > the end of the filesystem, and creates/modifies the resize inode. I > > believe that e2fsck already handles the resize inode, so combining the > > parts is all that's needed. > > Not really. You *can* do it using some somewhat obscure debugfs, > followed by running e2fsck -f and answering yes to a whole bunch of > scary questions, and in some cases it still might not work. So we > still need to enhance either resize2fs or write some new program if we > want ext2prepare functionality. Hmm, I'd think the basic functionality is mostly the same. Use resize2fs to move the inode bitmaps as if you were really going to resize the filesystem and needed more group descriptor blocks. Then "allocate" the reserved group descriptor blocks to the resize inode. What more is needed? The hard part of the operation (moving the itable, and file data blocks) is already implemented in resize2fs. > One thing I have thought about doing is simply enhacing > /usr/src/linux/fs/ext3/resize.c to support metablock groups, and > automatically switching to use meta blockgroups once there are no more > reserved gdt blocks. This means you no longer need ext2prepare, and > aside from some conservative stick-in-the-mud types running RHEL 3 or > other distro's with 2.4 kernels (heck, even Debian has finally gotten > off of 2.4), all modern Linux systems will support the metablock group > feature anyway. This will allow us to do resizing without needing to > use ext2prepare, or needing to reserve blocks at all. I believe that this is already implemented by Glauber de Oliveira Costa <go...@br...> and posted to ext2-devel a month ago, see: "[PATCH] Online Resizing - New attempt". Sadly, I haven't yet looked at it. Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X