| Message ID | 20131001012649.28415.17225.stgit@birch.djwong.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Mon, 30 Sep 2013, Darrick J. Wong wrote: > Date: Mon, 30 Sep 2013 18:26:49 -0700 > From: Darrick J. Wong <darrick.wong@oracle.com> > To: tytso@mit.edu, darrick.wong@oracle.com > Cc: linux-ext4@vger.kernel.org > Subject: [PATCH 01/31] tune2fs: Don't convert block # to cluster # when > clearing uninit_bg > > When we're constructing the initial block bitmap as part of removing the > gdt_csum (i.e. uninit_bg) feature, we mustn't convert the block numbers to > cluster numbers because ext2fs_mark_block_bitmap2() does this for us. I wonder if it's possible to use the old-style bitmap interface (the one which does not understand 64-bit bitmaps). If so, then you also need to fix ext2fs_mark_generic_bmap() (and others) so that we convert blocks to clusters if needed. Thanks! -Lukas > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > misc/tune2fs.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > index ddf3259..52247e0 100644 > --- a/misc/tune2fs.c > +++ b/misc/tune2fs.c > @@ -820,20 +820,17 @@ static void disable_uninit_bg(ext2_filsys fs, __u32 csum_feature_flag) > /* The bbitmap is zeroed; we must mark group metadata blocks in use */ > for (i = 0; i < fs->group_desc_count; i++) { > b = ext2fs_block_bitmap_loc(fs, i); > - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); > + ext2fs_mark_block_bitmap2(fs->block_map, b); > b = ext2fs_inode_bitmap_loc(fs, i); > - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); > + ext2fs_mark_block_bitmap2(fs->block_map, b); > > retval = ext2fs_super_and_bgd_loc2(fs, i, &b, &c, &d, NULL); > if (retval == 0 && b) > - ext2fs_mark_block_bitmap2(fs->block_map, > - EXT2FS_B2C(fs, b)); > + ext2fs_mark_block_bitmap2(fs->block_map, b); > if (retval == 0 && c) > - ext2fs_mark_block_bitmap2(fs->block_map, > - EXT2FS_B2C(fs, c)); > + ext2fs_mark_block_bitmap2(fs->block_map, c); > if (retval == 0 && d) > - ext2fs_mark_block_bitmap2(fs->block_map, > - EXT2FS_B2C(fs, d)); > + ext2fs_mark_block_bitmap2(fs->block_map, d); > if (retval) { > com_err("disable_uninit_bg", retval, > "while initializing block bitmaps"); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 03, 2013 at 06:53:38PM +0200, Lukáš Czerner wrote: > On Mon, 30 Sep 2013, Darrick J. Wong wrote: > > > Date: Mon, 30 Sep 2013 18:26:49 -0700 > > From: Darrick J. Wong <darrick.wong@oracle.com> > > To: tytso@mit.edu, darrick.wong@oracle.com > > Cc: linux-ext4@vger.kernel.org > > Subject: [PATCH 01/31] tune2fs: Don't convert block # to cluster # when > > clearing uninit_bg > > > > When we're constructing the initial block bitmap as part of removing the > > gdt_csum (i.e. uninit_bg) feature, we mustn't convert the block numbers to > > cluster numbers because ext2fs_mark_block_bitmap2() does this for us. > > I wonder if it's possible to use the old-style bitmap interface (the > one which does not understand 64-bit bitmaps). If so, then you also > need to fix ext2fs_mark_generic_bmap() (and others) so that we > convert blocks to clusters if needed. It's possible to do this (bad thing) by not specifying EXT2_FLAG_64BITS when calling ext2fs_openfs(); see patch 26 for a quick fix. (I'm imagining that most bigalloc users probably want 64bit anyway?) (No idea, bigalloc isn't (currently) my use case.) --D > > Thanks! > -Lukas > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > misc/tune2fs.c | 13 +++++-------- > > 1 file changed, 5 insertions(+), 8 deletions(-) > > > > > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > > index ddf3259..52247e0 100644 > > --- a/misc/tune2fs.c > > +++ b/misc/tune2fs.c > > @@ -820,20 +820,17 @@ static void disable_uninit_bg(ext2_filsys fs, __u32 csum_feature_flag) > > /* The bbitmap is zeroed; we must mark group metadata blocks in use */ > > for (i = 0; i < fs->group_desc_count; i++) { > > b = ext2fs_block_bitmap_loc(fs, i); > > - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); > > + ext2fs_mark_block_bitmap2(fs->block_map, b); > > b = ext2fs_inode_bitmap_loc(fs, i); > > - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); > > + ext2fs_mark_block_bitmap2(fs->block_map, b); > > > > retval = ext2fs_super_and_bgd_loc2(fs, i, &b, &c, &d, NULL); > > if (retval == 0 && b) > > - ext2fs_mark_block_bitmap2(fs->block_map, > > - EXT2FS_B2C(fs, b)); > > + ext2fs_mark_block_bitmap2(fs->block_map, b); > > if (retval == 0 && c) > > - ext2fs_mark_block_bitmap2(fs->block_map, > > - EXT2FS_B2C(fs, c)); > > + ext2fs_mark_block_bitmap2(fs->block_map, c); > > if (retval == 0 && d) > > - ext2fs_mark_block_bitmap2(fs->block_map, > > - EXT2FS_B2C(fs, d)); > > + ext2fs_mark_block_bitmap2(fs->block_map, d); > > if (retval) { > > com_err("disable_uninit_bg", retval, > > "while initializing block bitmaps"); > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 3 Oct 2013, Darrick J. Wong wrote: > Date: Thu, 3 Oct 2013 12:04:44 -0700 > From: Darrick J. Wong <darrick.wong@oracle.com> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: tytso@mit.edu, linux-ext4@vger.kernel.org > Subject: Re: [PATCH 01/31] tune2fs: Don't convert block # to cluster # when > clearing uninit_bg > > On Thu, Oct 03, 2013 at 06:53:38PM +0200, Lukáš Czerner wrote: > > On Mon, 30 Sep 2013, Darrick J. Wong wrote: > > > > > Date: Mon, 30 Sep 2013 18:26:49 -0700 > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > To: tytso@mit.edu, darrick.wong@oracle.com > > > Cc: linux-ext4@vger.kernel.org > > > Subject: [PATCH 01/31] tune2fs: Don't convert block # to cluster # when > > > clearing uninit_bg > > > > > > When we're constructing the initial block bitmap as part of removing the > > > gdt_csum (i.e. uninit_bg) feature, we mustn't convert the block numbers to > > > cluster numbers because ext2fs_mark_block_bitmap2() does this for us. > > > > I wonder if it's possible to use the old-style bitmap interface (the > > one which does not understand 64-bit bitmaps). If so, then you also > > need to fix ext2fs_mark_generic_bmap() (and others) so that we > > convert blocks to clusters if needed. > > It's possible to do this (bad thing) by not specifying EXT2_FLAG_64BITS when > calling ext2fs_openfs(); see patch 26 for a quick fix. > > (I'm imagining that most bigalloc users probably want 64bit anyway?) > > (No idea, bigalloc isn't (currently) my use case.) Fair enough. The patch no. 26 seems to do the trick. Thanks! -Lukas > > --D > > > > Thanks! > > -Lukas > > > > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > --- > > > misc/tune2fs.c | 13 +++++-------- > > > 1 file changed, 5 insertions(+), 8 deletions(-) > > > > > > > > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > > > index ddf3259..52247e0 100644 > > > --- a/misc/tune2fs.c > > > +++ b/misc/tune2fs.c > > > @@ -820,20 +820,17 @@ static void disable_uninit_bg(ext2_filsys fs, __u32 csum_feature_flag) > > > /* The bbitmap is zeroed; we must mark group metadata blocks in use */ > > > for (i = 0; i < fs->group_desc_count; i++) { > > > b = ext2fs_block_bitmap_loc(fs, i); > > > - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); > > > + ext2fs_mark_block_bitmap2(fs->block_map, b); > > > b = ext2fs_inode_bitmap_loc(fs, i); > > > - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); > > > + ext2fs_mark_block_bitmap2(fs->block_map, b); > > > > > > retval = ext2fs_super_and_bgd_loc2(fs, i, &b, &c, &d, NULL); > > > if (retval == 0 && b) > > > - ext2fs_mark_block_bitmap2(fs->block_map, > > > - EXT2FS_B2C(fs, b)); > > > + ext2fs_mark_block_bitmap2(fs->block_map, b); > > > if (retval == 0 && c) > > > - ext2fs_mark_block_bitmap2(fs->block_map, > > > - EXT2FS_B2C(fs, c)); > > > + ext2fs_mark_block_bitmap2(fs->block_map, c); > > > if (retval == 0 && d) > > > - ext2fs_mark_block_bitmap2(fs->block_map, > > > - EXT2FS_B2C(fs, d)); > > > + ext2fs_mark_block_bitmap2(fs->block_map, d); > > > if (retval) { > > > com_err("disable_uninit_bg", retval, > > > "while initializing block bitmaps"); > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Thu, Oct 03, 2013 at 12:04:44PM -0700, Darrick J. Wong wrote: > On Thu, Oct 03, 2013 at 06:53:38PM +0200, Lukáš Czerner wrote: > > On Mon, 30 Sep 2013, Darrick J. Wong wrote: > > > > > Date: Mon, 30 Sep 2013 18:26:49 -0700 > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > To: tytso@mit.edu, darrick.wong@oracle.com > > > Cc: linux-ext4@vger.kernel.org > > > Subject: [PATCH 01/31] tune2fs: Don't convert block # to cluster # when > > > clearing uninit_bg > > > > > > When we're constructing the initial block bitmap as part of removing the > > > gdt_csum (i.e. uninit_bg) feature, we mustn't convert the block numbers to > > > cluster numbers because ext2fs_mark_block_bitmap2() does this for us. > > > > I wonder if it's possible to use the old-style bitmap interface (the > > one which does not understand 64-bit bitmaps). If so, then you also > > need to fix ext2fs_mark_generic_bmap() (and others) so that we > > convert blocks to clusters if needed. > > It's possible to do this (bad thing) by not specifying EXT2_FLAG_64BITS when > calling ext2fs_openfs(); see patch 26 for a quick fix. Bigalloc requires the new-style bitmaps, so patch 26 is the right fix for this. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 07, 2013 at 09:03:28AM -0400, Theodore Ts'o wrote: > On Thu, Oct 03, 2013 at 12:04:44PM -0700, Darrick J. Wong wrote: > > On Thu, Oct 03, 2013 at 06:53:38PM +0200, Lukáš Czerner wrote: > > > On Mon, 30 Sep 2013, Darrick J. Wong wrote: > > > > > > > Date: Mon, 30 Sep 2013 18:26:49 -0700 > > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > To: tytso@mit.edu, darrick.wong@oracle.com > > > > Cc: linux-ext4@vger.kernel.org > > > > Subject: [PATCH 01/31] tune2fs: Don't convert block # to cluster # when > > > > clearing uninit_bg > > > > > > > > When we're constructing the initial block bitmap as part of removing the > > > > gdt_csum (i.e. uninit_bg) feature, we mustn't convert the block numbers to > > > > cluster numbers because ext2fs_mark_block_bitmap2() does this for us. > > > > > > I wonder if it's possible to use the old-style bitmap interface (the > > > one which does not understand 64-bit bitmaps). If so, then you also > > > need to fix ext2fs_mark_generic_bmap() (and others) so that we > > > convert blocks to clusters if needed. > > > > It's possible to do this (bad thing) by not specifying EXT2_FLAG_64BITS when > > calling ext2fs_openfs(); see patch 26 for a quick fix. > > Bigalloc requires the new-style bitmaps, so patch 26 is the right fix > for this. Any thoughts on patch #1? It fixes bugs in the recently-landed patch "tune2fs: zero inode table when removing checksums". --D > > - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 09, 2013 at 03:10:28PM -0700, Darrick J. Wong wrote: > > Any thoughts on patch #1? It fixes bugs in the recently-landed patch "tune2fs: > zero inode table when removing checksums". I haven't gotten to it yet since I've been focusing on patches which apply to the maint branch (i.e., for a 1.42.x release). The metadata checksum patches are on the master/next branch, which will be for an eventual 1.43 release. What I plan to do is to get all or most of the changes which are appropriate for the maint branch applied. I'll then merge the maint branch into the next branch, and then start applying the patches which are specific for a future 1.43 release. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 09, 2013 at 08:26:02PM -0400, Theodore Ts'o wrote: > On Wed, Oct 09, 2013 at 03:10:28PM -0700, Darrick J. Wong wrote: > > > > Any thoughts on patch #1? It fixes bugs in the recently-landed patch "tune2fs: > > zero inode table when removing checksums". > > I haven't gotten to it yet since I've been focusing on patches which > apply to the maint branch (i.e., for a 1.42.x release). > > The metadata checksum patches are on the master/next branch, which > will be for an eventual 1.43 release. > > What I plan to do is to get all or most of the changes which are > appropriate for the maint branch applied. I'll then merge the maint > branch into the next branch, and then start applying the patches which > are specific for a future 1.43 release. Ok. Do you want me to hold off on reissuing changed patches and sending out new patches until then? Or should I just send 'em and we can deal with the blizzard later? Patches 2, 14-16, and 22 have changed, and there are now 9 more patches, mostly to fix resize2fs bugs. Eh, I'll move the ones you took to the start of my quilt, and hope they don't change too much when they get merged in. That'll help me keep the in-review patches in a consecutive pile. --D > > Cheers, > > - Ted > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/misc/tune2fs.c b/misc/tune2fs.c index ddf3259..52247e0 100644 --- a/misc/tune2fs.c +++ b/misc/tune2fs.c @@ -820,20 +820,17 @@ static void disable_uninit_bg(ext2_filsys fs, __u32 csum_feature_flag) /* The bbitmap is zeroed; we must mark group metadata blocks in use */ for (i = 0; i < fs->group_desc_count; i++) { b = ext2fs_block_bitmap_loc(fs, i); - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); + ext2fs_mark_block_bitmap2(fs->block_map, b); b = ext2fs_inode_bitmap_loc(fs, i); - ext2fs_mark_block_bitmap2(fs->block_map, EXT2FS_B2C(fs, b)); + ext2fs_mark_block_bitmap2(fs->block_map, b); retval = ext2fs_super_and_bgd_loc2(fs, i, &b, &c, &d, NULL); if (retval == 0 && b) - ext2fs_mark_block_bitmap2(fs->block_map, - EXT2FS_B2C(fs, b)); + ext2fs_mark_block_bitmap2(fs->block_map, b); if (retval == 0 && c) - ext2fs_mark_block_bitmap2(fs->block_map, - EXT2FS_B2C(fs, c)); + ext2fs_mark_block_bitmap2(fs->block_map, c); if (retval == 0 && d) - ext2fs_mark_block_bitmap2(fs->block_map, - EXT2FS_B2C(fs, d)); + ext2fs_mark_block_bitmap2(fs->block_map, d); if (retval) { com_err("disable_uninit_bg", retval, "while initializing block bitmaps");
When we're constructing the initial block bitmap as part of removing the gdt_csum (i.e. uninit_bg) feature, we mustn't convert the block numbers to cluster numbers because ext2fs_mark_block_bitmap2() does this for us. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- misc/tune2fs.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html