Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1325407 > unrolled thread

[PATCH] ext4: add a line break for proc mb_groups display

Started byHuaitong Han <huaitong.han@intel.com>
First post2016-02-03 14:20 +0100
Last post2016-02-03 22:20 +0100
Articles 9 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] ext4: add a line break for proc mb_groups display Huaitong Han <huaitong.han@intel.com> - 2016-02-03 14:20 +0100
    Re: [PATCH] ext4: add a line break for proc mb_groups display "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-02-03 19:20 +0100
      Re: [PATCH] ext4: add a line break for proc mb_groups display Joe Perches <joe@perches.com> - 2016-02-03 19:40 +0100
        Re: [PATCH] ext4: add a line break for proc mb_groups display "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-02-03 20:40 +0100
          Re: [PATCH] ext4: add a line break for proc mb_groups display Joe Perches <joe@perches.com> - 2016-02-03 20:40 +0100
            Re: [PATCH] ext4: add a line break for proc mb_groups display "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-02-03 21:20 +0100
              Re: [PATCH] ext4: add a line break for proc mb_groups display Andreas Dilger <adilger@dilger.ca> - 2016-02-03 22:20 +0100
                Re: [PATCH] ext4: add a line break for proc mb_groups display Joe Perches <joe@perches.com> - 2016-02-03 22:20 +0100
    Re: [PATCH] ext4: add a line break for proc mb_groups display Andreas Dilger <adilger@dilger.ca> - 2016-02-03 22:20 +0100

#1325407 — [PATCH] ext4: add a line break for proc mb_groups display

FromHuaitong Han <huaitong.han@intel.com>
Date2016-02-03 14:20 +0100
Subject[PATCH] ext4: add a line break for proc mb_groups display
Message-ID<qY5vJ-2qT-37@gated-at.bofh.it>
This patch adds a line break for proc mb_groups display.

Signed-off-by: Huaitong Han <huaitong.han@intel.com>
---
 fs/ext4/mballoc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 61eaf74..4424b7b 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2285,7 +2285,7 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v)
 	if (group == 0)
 		seq_puts(seq, "#group: free  frags first ["
 			      " 2^0   2^1   2^2   2^3   2^4   2^5   2^6  "
-			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]");
+			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]\n");
 
 	i = (sb->s_blocksize_bits + 2) * sizeof(sg.info.bb_counters[0]) +
 		sizeof(struct ext4_group_info);
-- 
2.4.3

[toc] | [next] | [standalone]

#1325791

From"Darrick J. Wong" <darrick.wong@oracle.com>
Date2016-02-03 19:20 +0100
Message-ID<qYac2-5uj-35@gated-at.bofh.it>
In reply to#1325407
On Wed, Feb 03, 2016 at 09:19:06PM +0800, Huaitong Han wrote:
> This patch adds a line break for proc mb_groups display.
> 
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>
> ---
>  fs/ext4/mballoc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 61eaf74..4424b7b 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2285,7 +2285,7 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v)
>  	if (group == 0)
>  		seq_puts(seq, "#group: free  frags first ["
>  			      " 2^0   2^1   2^2   2^3   2^4   2^5   2^6  "
> -			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]");
> +			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]\n");

Oh, heh.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  
>  	i = (sb->s_blocksize_bits + 2) * sizeof(sg.info.bb_counters[0]) +
>  		sizeof(struct ext4_group_info);
> -- 
> 2.4.3
> 
> --
> 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

[toc] | [prev] | [next] | [standalone]

#1325813

FromJoe Perches <joe@perches.com>
Date2016-02-03 19:40 +0100
Message-ID<qYavo-5AG-13@gated-at.bofh.it>
In reply to#1325791
On Wed, 2016-02-03 at 10:13 -0800, Darrick J. Wong wrote:
> On Wed, Feb 03, 2016 at 09:19:06PM +0800, Huaitong Han wrote:
> > This patch adds a line break for proc mb_groups display.

Using 2 lines for output might break any existing users.

Are there any?

> > 
> > Signed-off-by: Huaitong Han <huaitong.han@intel.com>
> > ---
> >  fs/ext4/mballoc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> > index 61eaf74..4424b7b 100644
> > --- a/fs/ext4/mballoc.c
> > +++ b/fs/ext4/mballoc.c
> > @@ -2285,7 +2285,7 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v)
> >  	if (group == 0)
> >  		seq_puts(seq, "#group: free  frags first ["
> >  			      " 2^0   2^1   2^2   2^3   2^4   2^5   2^6  "
> > -			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]");
> > +			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]\n");
> 
> Oh, heh.
> 
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> --D
> 
> >  
> >  	i = (sb->s_blocksize_bits + 2) * sizeof(sg.info.bb_counters[0]) +
> >  		sizeof(struct ext4_group_info);

[toc] | [prev] | [next] | [standalone]

#1325891

From"Darrick J. Wong" <darrick.wong@oracle.com>
Date2016-02-03 20:40 +0100
Message-ID<qYbrs-6bI-11@gated-at.bofh.it>
In reply to#1325813
On Wed, Feb 03, 2016 at 10:30:32AM -0800, Joe Perches wrote:
> On Wed, 2016-02-03 at 10:13 -0800, Darrick J. Wong wrote:
> > On Wed, Feb 03, 2016 at 09:19:06PM +0800, Huaitong Han wrote:
> > > This patch adds a line break for proc mb_groups display.
> 
> Using 2 lines for output might break any existing users.
> 
> Are there any?

It's a multiline file if you have more than one blockgroup; this just makes it
so that you don't have to special-case BG 0.

IOW: mb_groups scripts already had to parse multiple lines, and most likely any
script parsing it would inject a newline after the header.

--D

> 
> > > 
> > > Signed-off-by: Huaitong Han <huaitong.han@intel.com>
> > > ---
> > >  fs/ext4/mballoc.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> > > index 61eaf74..4424b7b 100644
> > > --- a/fs/ext4/mballoc.c
> > > +++ b/fs/ext4/mballoc.c
> > > @@ -2285,7 +2285,7 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v)
> > >  	if (group == 0)
> > >  		seq_puts(seq, "#group: free  frags first ["
> > >  			      " 2^0   2^1   2^2   2^3   2^4   2^5   2^6  "
> > > -			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]");
> > > +			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]\n");
> > 
> > Oh, heh.
> > 
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > --D
> > 
> > >  
> > >  	i = (sb->s_blocksize_bits + 2) * sizeof(sg.info.bb_counters[0]) +
> > >  		sizeof(struct ext4_group_info);
> 
> --
> 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

[toc] | [prev] | [next] | [standalone]

#1325892

FromJoe Perches <joe@perches.com>
Date2016-02-03 20:40 +0100
Message-ID<qYbrs-6bI-17@gated-at.bofh.it>
In reply to#1325891
On Wed, 2016-02-03 at 11:32 -0800, Darrick J. Wong wrote:
> On Wed, Feb 03, 2016 at 10:30:32AM -0800, Joe Perches wrote:
> > On Wed, 2016-02-03 at 10:13 -0800, Darrick J. Wong wrote:
> > > On Wed, Feb 03, 2016 at 09:19:06PM +0800, Huaitong Han wrote:
> > > > This patch adds a line break for proc mb_groups display.
> > 
> > Using 2 lines for output might break any existing users.
> > 
> > Are there any?
> 
> It's a multiline file if you have more than one blockgroup; this just makes it
> so that you don't have to special-case BG 0.

And existing scripts might do that now and might fail
to do properly after this change.

> IOW: mb_groups scripts already had to parse multiple lines, and most likely any
> script parsing it would inject a newline after the header.

I've no dog in this fight really.  I just wanted to make
it clear that this could cause existing scripts to fail.

proc output is supposed to be unchanging except maybe
adding new fields to existing lines.

Your choice.

cheers, Joe

[toc] | [prev] | [next] | [standalone]

#1325914

From"Darrick J. Wong" <darrick.wong@oracle.com>
Date2016-02-03 21:20 +0100
Message-ID<qYc4b-6E7-33@gated-at.bofh.it>
In reply to#1325892
On Wed, Feb 03, 2016 at 11:38:33AM -0800, Joe Perches wrote:
> On Wed, 2016-02-03 at 11:32 -0800, Darrick J. Wong wrote:
> > On Wed, Feb 03, 2016 at 10:30:32AM -0800, Joe Perches wrote:
> > > On Wed, 2016-02-03 at 10:13 -0800, Darrick J. Wong wrote:
> > > > On Wed, Feb 03, 2016 at 09:19:06PM +0800, Huaitong Han wrote:
> > > > > This patch adds a line break for proc mb_groups display.
> > > 
> > > Using 2 lines for output might break any existing users.
> > > 
> > > Are there any?
> > 
> > It's a multiline file if you have more than one blockgroup; this just makes it
> > so that you don't have to special-case BG 0.
> 
> And existing scripts might do that now and might fail
> to do properly after this change.

Or they might have sed -e 's/]#0/]\n#0/g' in which case they won't be affected.

> > IOW: mb_groups scripts already had to parse multiple lines, and most likely any
> > script parsing it would inject a newline after the header.
> 
> I've no dog in this fight really.  I just wanted to make
> it clear that this could cause existing scripts to fail.
> 
> proc output is supposed to be unchanging except maybe
> adding new fields to existing lines.
> 
> Your choice.

Ted's, really.  I have no idea which scripts do with various per-fs /proc
files.  Usually poking in mb_groups is only done as part of failure report data
collection to see what's mucked up the fs this time.

Anyway, I'll defer to the maintainer. :)

--D

> 
> cheers, Joe

[toc] | [prev] | [next] | [standalone]

#1325956

FromAndreas Dilger <adilger@dilger.ca>
Date2016-02-03 22:20 +0100
Message-ID<qYd0e-7f0-15@gated-at.bofh.it>
In reply to#1325914

[Multipart message — attachments visible in raw view] — view raw

On Feb 3, 2016, at 1:07 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> On Wed, Feb 03, 2016 at 11:38:33AM -0800, Joe Perches wrote:
>> On Wed, 2016-02-03 at 11:32 -0800, Darrick J. Wong wrote:
>>> On Wed, Feb 03, 2016 at 10:30:32AM -0800, Joe Perches wrote:
>>>> On Wed, 2016-02-03 at 10:13 -0800, Darrick J. Wong wrote:
>>>>> On Wed, Feb 03, 2016 at 09:19:06PM +0800, Huaitong Han wrote:
>>>>>> This patch adds a line break for proc mb_groups display.
>>>> 
>>>> Using 2 lines for output might break any existing users.
>>>> 
>>>> Are there any?
>>> 
>>> It's a multiline file if you have more than one blockgroup; this just makes it
>>> so that you don't have to special-case BG 0.
>> 
>> And existing scripts might do that now and might fail
>> to do properly after this change.
> 
> Or they might have sed -e 's/]#0/]\n#0/g' in which case they won't be affected.

I suspect that any scripts which handled this in the past probably didn't
even notice and just missed the bg=0 line at the end of the header.  Users
looking at the output probably saw it line-wrapped by the terminal and
didn't notice either.

>>> IOW: mb_groups scripts already had to parse multiple lines, and most likely any
>>> script parsing it would inject a newline after the header.
>> 
>> I've no dog in this fight really.  I just wanted to make
>> it clear that this could cause existing scripts to fail.
>> 
>> proc output is supposed to be unchanging except maybe
>> adding new fields to existing lines.
>> 
>> Your choice.
> 
> Ted's, really.  I have no idea which scripts do with various per-fs /proc
> files.  Usually poking in mb_groups is only done as part of failure report data
> collection to see what's mucked up the fs this time.
> 
> Anyway, I'll defer to the maintainer. :)

I think it makes sense to accept the patch, since I doubt any scripts will be broken, and it is the "right thing to do" rather than perpetuate a bug.

Cheers, Andreas

[toc] | [prev] | [next] | [standalone]

#1325957

FromJoe Perches <joe@perches.com>
Date2016-02-03 22:20 +0100
Message-ID<qYd0f-7f0-21@gated-at.bofh.it>
In reply to#1325956
On Wed, 2016-02-03 at 14:12 -0700, Andreas Dilger wrote:

> I think it makes sense to accept the patch, since I doubt any scripts
> will be broken, and it is the "right thing to do" rather than
> perpetuate a bug.

Perhaps it also would make sense to add some text
to the commit log about this.

[toc] | [prev] | [next] | [standalone]

#1325961

FromAndreas Dilger <adilger@dilger.ca>
Date2016-02-03 22:20 +0100
Message-ID<qYd0f-7f0-29@gated-at.bofh.it>
In reply to#1325407

[Multipart message — attachments visible in raw view] — view raw

> On Feb 3, 2016, at 6:19 AM, Huaitong Han <huaitong.han@intel.com> wrote:
> 
> This patch adds a line break for proc mb_groups display.
> 
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/ext4/mballoc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 61eaf74..4424b7b 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2285,7 +2285,7 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v)
> 	if (group == 0)
> 		seq_puts(seq, "#group: free  frags first ["
> 			      " 2^0   2^1   2^2   2^3   2^4   2^5   2^6  "
> -			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]");
> +			      " 2^7   2^8   2^9   2^10  2^11  2^12  2^13  ]\n");
> 
> 	i = (sb->s_blocksize_bits + 2) * sizeof(sg.info.bb_counters[0]) +
> 		sizeof(struct ext4_group_info);
> --
> 2.4.3
> 


Cheers, Andreas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web