All of lore.kernel.org
 help / color / mirror / Atom feed
* dumpe2fs: only print 'unused inodes' once
@ 2014-09-09 19:32 TR Reardon
  2014-09-09 19:47 ` Darrick J. Wong
  0 siblings, 1 reply; 5+ messages in thread
From: TR Reardon @ 2014-09-09 19:32 UTC (permalink / raw)
  To: linux-ext4

just a minor nit...and I don't think fixing this harms any existing tests.

+Reardon


diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
index a1c5ba2..d30cf87 100644
--- a/misc/dumpe2fs.c
+++ b/misc/dumpe2fs.c
@@ -205,8 +205,6 @@ static void list_desc (ext2_filsys fs)
                        printf(_("  Checksum 0x%04x"), csum);
                        if (csum != exp_csum)
                                printf(_(" (EXPECTED 0x%04x)"), exp_csum);
-                       printf(_(", unused inodes %u\n"),
-                              ext2fs_bg_itable_unused(fs, i));
                }
                has_super = ((i==0) || super_blk);
                if (has_super) {


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: dumpe2fs: only print 'unused inodes' once
  2014-09-09 19:32 dumpe2fs: only print 'unused inodes' once TR Reardon
@ 2014-09-09 19:47 ` Darrick J. Wong
  2014-09-10 21:38   ` Theodore Ts'o
  0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2014-09-09 19:47 UTC (permalink / raw)
  To: TR Reardon; +Cc: linux-ext4

On Tue, Sep 09, 2014 at 03:32:56PM -0400, TR Reardon wrote:
> just a minor nit...and I don't think fixing this harms any existing tests.

Running 'make check' will tell you.

This also needs a Signed-off-by.

--D
> 
> +Reardon
> 
> 
> diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
> index a1c5ba2..d30cf87 100644
> --- a/misc/dumpe2fs.c
> +++ b/misc/dumpe2fs.c
> @@ -205,8 +205,6 @@ static void list_desc (ext2_filsys fs)
>                         printf(_("  Checksum 0x%04x"), csum);
>                         if (csum != exp_csum)
>                                 printf(_(" (EXPECTED 0x%04x)"), exp_csum);
> -                       printf(_(", unused inodes %u\n"),
> -                              ext2fs_bg_itable_unused(fs, i));
>                 }
>                 has_super = ((i==0) || super_blk);
>                 if (has_super) {
> 
> 
>  		 	   		  --
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: dumpe2fs: only print 'unused inodes' once
  2014-09-09 19:47 ` Darrick J. Wong
@ 2014-09-10 21:38   ` Theodore Ts'o
  2014-09-10 21:47     ` Darrick J. Wong
  0 siblings, 1 reply; 5+ messages in thread
From: Theodore Ts'o @ 2014-09-10 21:38 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: TR Reardon, linux-ext4

On Tue, Sep 09, 2014 at 12:47:46PM -0700, Darrick J. Wong wrote:
> On Tue, Sep 09, 2014 at 03:32:56PM -0400, TR Reardon wrote:
> > just a minor nit...and I don't think fixing this harms any existing tests.
> 
> Running 'make check' will tell you.
> 
> This also needs a Signed-off-by.

It also might make more sense to move the checksum to after the inode
table, i.e., instead of:

Group 0: (Blocks 0-32767) [ITABLE_ZEROED]
  Checksum 0xe0c6
  Primary superblock at 0, Group descriptors at 1-1
  Reserved GDT blocks at 2-512
  Block bitmap at 513 (+513), Inode bitmap at 529 (+529)
  Inode table at 545-1056 (+545)
  24025 free blocks, 8181 free inodes, 2 directories, 8181 unused inodes
  Free blocks: 8743-32767
  Free inodes: 12-8192

how about:

Group 0: (Blocks 0-32767) [ITABLE_ZEROED]
  Primary superblock at 0, Group descriptors at 1-1
  Reserved GDT blocks at 2-512
  Block bitmap at 513 (+513), Inode bitmap at 529 (+529)
  Inode table at 545-1056 (+545), Checksum 0xe0c6
  24025 free blocks, 8181 free inodes, 2 directories, 8181 unused inodes
  Free blocks: 8743-32767
  Free inodes: 12-8192

								- Ted






> 
> --D
> > 
> > +Reardon
> > 
> > 
> > diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
> > index a1c5ba2..d30cf87 100644
> > --- a/misc/dumpe2fs.c
> > +++ b/misc/dumpe2fs.c
> > @@ -205,8 +205,6 @@ static void list_desc (ext2_filsys fs)
> >                         printf(_("  Checksum 0x%04x"), csum);
> >                         if (csum != exp_csum)
> >                                 printf(_(" (EXPECTED 0x%04x)"), exp_csum);
> > -                       printf(_(", unused inodes %u\n"),
> > -                              ext2fs_bg_itable_unused(fs, i));
> >                 }
> >                 has_super = ((i==0) || super_blk);
> >                 if (has_super) {
> > 
> > 
> >  		 	   		  --
> > 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
--
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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: dumpe2fs: only print 'unused inodes' once
  2014-09-10 21:38   ` Theodore Ts'o
@ 2014-09-10 21:47     ` Darrick J. Wong
  2014-09-11 12:53       ` Theodore Ts'o
  0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2014-09-10 21:47 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: TR Reardon, linux-ext4

On Wed, Sep 10, 2014 at 05:38:43PM -0400, Theodore Ts'o wrote:
> On Tue, Sep 09, 2014 at 12:47:46PM -0700, Darrick J. Wong wrote:
> > On Tue, Sep 09, 2014 at 03:32:56PM -0400, TR Reardon wrote:
> > > just a minor nit...and I don't think fixing this harms any existing tests.
> > 
> > Running 'make check' will tell you.
> > 
> > This also needs a Signed-off-by.
> 
> It also might make more sense to move the checksum to after the inode
> table, i.e., instead of:
> 
> Group 0: (Blocks 0-32767) [ITABLE_ZEROED]
>   Checksum 0xe0c6
>   Primary superblock at 0, Group descriptors at 1-1
>   Reserved GDT blocks at 2-512
>   Block bitmap at 513 (+513), Inode bitmap at 529 (+529)
>   Inode table at 545-1056 (+545)
>   24025 free blocks, 8181 free inodes, 2 directories, 8181 unused inodes
>   Free blocks: 8743-32767
>   Free inodes: 12-8192
> 
> how about:
> 
> Group 0: (Blocks 0-32767) [ITABLE_ZEROED]
>   Primary superblock at 0, Group descriptors at 1-1
>   Reserved GDT blocks at 2-512
>   Block bitmap at 513 (+513), Inode bitmap at 529 (+529)
>   Inode table at 545-1056 (+545), Checksum 0xe0c6

But... 0xE0C6 is the group descriptor checksum.  Since we report the bitmap
csum after mentioning them:

>  Block bitmap at 2 (+2), csum 0xc759340d, Inode bitmap at 18 (+18), csum 0x941e2bba

It would make more sense to report the group checksum on the first line:

Group 0: (Blocks 0-32767) csum 0xe0c6 [ITABLE_ZEROED]                                     

Unless it'll break peoples' scripts?

--D

>   24025 free blocks, 8181 free inodes, 2 directories, 8181 unused inodes
>   Free blocks: 8743-32767
>   Free inodes: 12-8192
> 
> 								- Ted
> 
> 
> 
> 
> 
> 
> > 
> > --D
> > > 
> > > +Reardon
> > > 
> > > 
> > > diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
> > > index a1c5ba2..d30cf87 100644
> > > --- a/misc/dumpe2fs.c
> > > +++ b/misc/dumpe2fs.c
> > > @@ -205,8 +205,6 @@ static void list_desc (ext2_filsys fs)
> > >                         printf(_("  Checksum 0x%04x"), csum);
> > >                         if (csum != exp_csum)
> > >                                 printf(_(" (EXPECTED 0x%04x)"), exp_csum);
> > > -                       printf(_(", unused inodes %u\n"),
> > > -                              ext2fs_bg_itable_unused(fs, i));
> > >                 }
> > >                 has_super = ((i==0) || super_blk);
> > >                 if (has_super) {
> > > 
> > > 
> > >  		 	   		  --
> > > 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
> --
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: dumpe2fs: only print 'unused inodes' once
  2014-09-10 21:47     ` Darrick J. Wong
@ 2014-09-11 12:53       ` Theodore Ts'o
  0 siblings, 0 replies; 5+ messages in thread
From: Theodore Ts'o @ 2014-09-11 12:53 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: TR Reardon, linux-ext4

On Wed, Sep 10, 2014 at 02:47:46PM -0700, Darrick J. Wong wrote:
> > 
> > Group 0: (Blocks 0-32767) [ITABLE_ZEROED]
> >   Primary superblock at 0, Group descriptors at 1-1
> >   Reserved GDT blocks at 2-512
> >   Block bitmap at 513 (+513), Inode bitmap at 529 (+529)
> >   Inode table at 545-1056 (+545), Checksum 0xe0c6
> 
> But... 0xE0C6 is the group descriptor checksum.  Since we report the bitmap
> csum after mentioning them:
> 
> >  Block bitmap at 2 (+2), csum 0xc759340d, Inode bitmap at 18 (+18), csum 0x941e2bba
> 
> It would make more sense to report the group checksum on the first line:
> 
> Group 0: (Blocks 0-32767) csum 0xe0c6 [ITABLE_ZEROED]                                     
> 

Good point, I was thinking more about about the case where we only had
checksums on the block group.  But in the case of metadata_csum, where
everything is checksummed, your suggestion is much better.

I'm not too worried about breaking scripts; dumpe2fs hasn't had a
stable format in a while.  And breaking it for 1.43 is probably
acceptable.

We should probably add stable way of extracting information about the
location of the bitmaps and inode table to debugfs, though.  Something
where we do something like this:

0:0:1:9:1025:1041:1057
1:32768:32769:32777:1026:1042:1569
2:-1:-1:-1:1027:1043:2081

where we use colon separated fields for the group number, and the
location of the superblock, group descriptors, block bitmap, inode
bitmap, and inode table.  Becasue that sort of thing is really useful
when trying to annotate a blktrace output (and there's a script that I
should find and drop into contrib that does exactly that).

					- Ted

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-09-11 12:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09 19:32 dumpe2fs: only print 'unused inodes' once TR Reardon
2014-09-09 19:47 ` Darrick J. Wong
2014-09-10 21:38   ` Theodore Ts'o
2014-09-10 21:47     ` Darrick J. Wong
2014-09-11 12:53       ` Theodore Ts'o

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.