2024-04-17 01:55:17

by Youling Tang

[permalink] [raw]
Subject: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`

From: Youling Tang <[email protected]>

Before patch:
```
#cat btrees/inodes/keys
u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
flags= (16300000)
```

After patch:
```
#cat btrees/inodes/keys
u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
mode=40755
flags= (16300000)
```

Signed-off-by: Youling Tang <[email protected]>
---
fs/bcachefs/bkey_methods.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/bcachefs/bkey_methods.c b/fs/bcachefs/bkey_methods.c
index db336a43fc08..03c755f65430 100644
--- a/fs/bcachefs/bkey_methods.c
+++ b/fs/bcachefs/bkey_methods.c
@@ -306,7 +306,8 @@ void bch2_bkey_val_to_text(struct printbuf *out, struct bch_fs *c,
bch2_bkey_to_text(out, k.k);

if (bkey_val_bytes(k.k)) {
- prt_printf(out, ": ");
+ prt_printf(out, ":");
+ prt_newline(out);
bch2_val_to_text(out, c, k);
}
}
--
2.34.1



2024-04-17 02:20:45

by Kent Overstreet

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`

On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
> From: Youling Tang <[email protected]>
>
> Before patch:
> ```
> #cat btrees/inodes/keys
> u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
> flags= (16300000)
> ```
>
> After patch:
> ```
> #cat btrees/inodes/keys
> u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
> mode=40755
> flags= (16300000)

This would print a newline for keys that don't have a value...

2024-04-17 02:50:52

by Youling Tang

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`

Hi, Kent
On 17/04/2024 10:20, Kent Overstreet wrote:
> On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
>> From: Youling Tang <[email protected]>
>>
>> Before patch:
>> ```
>> #cat btrees/inodes/keys
>> u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
>> flags= (16300000)
>> ```
>>
>> After patch:
>> ```
>> #cat btrees/inodes/keys
>> u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
>> mode=40755
>> flags= (16300000)
> This would print a newline for keys that don't have a value...
The original intention was to make the display of the printed content in
'__bch2_inode_unpacked_to_text ()' consistent, without considering other
callbacks.

Or just modify it in the following way?
--- a/fs/bcachefs/inode.c
+++ b/fs/bcachefs/inode.c
@@ -534,6 +534,8 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
bkey_s_c k,
 static void __bch2_inode_unpacked_to_text(struct printbuf *out,
struct bch_inode_unpacked *inode)
 {
+       prt_newline(out);
+
        printbuf_indent_add(out, 2);
        prt_printf(out, "mode=%o", inode->bi_mode);
        prt_newline(out);


Thanks,
Youling.

2024-04-17 02:59:50

by Kent Overstreet

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`

On Wed, Apr 17, 2024 at 10:50:10AM +0800, Youling Tang wrote:
> Hi, Kent
> On 17/04/2024 10:20, Kent Overstreet wrote:
> > On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
> > > From: Youling Tang <[email protected]>
> > >
> > > Before patch:
> > > ```
> > > #cat btrees/inodes/keys
> > > u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
> > > flags= (16300000)
> > > ```
> > >
> > > After patch:
> > > ```
> > > #cat btrees/inodes/keys
> > > u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
> > > mode=40755
> > > flags= (16300000)
> > This would print a newline for keys that don't have a value...
> The original intention was to make the display of the printed content in
> '__bch2_inode_unpacked_to_text ()' consistent, without considering other
> callbacks.
>
> Or just modify it in the following way?

Yeah, that's better

Do it off my master branch though, there's some printbuf imprevements in
there.

https://evilpiepirate.org/git/bcachefs.git

> --- a/fs/bcachefs/inode.c
> +++ b/fs/bcachefs/inode.c
> @@ -534,6 +534,8 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
> bkey_s_c k,
>  static void __bch2_inode_unpacked_to_text(struct printbuf *out,
> struct bch_inode_unpacked *inode)
>  {
> +       prt_newline(out);
> +
>         printbuf_indent_add(out, 2);
>         prt_printf(out, "mode=%o", inode->bi_mode);
>         prt_newline(out);
>
>
> Thanks,
> Youling.

2024-04-17 03:16:31

by Hongbo Li

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`



On 2024/4/17 10:59, Kent Overstreet wrote:
> On Wed, Apr 17, 2024 at 10:50:10AM +0800, Youling Tang wrote:
>> Hi, Kent
>> On 17/04/2024 10:20, Kent Overstreet wrote:
>>> On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
>>>> From: Youling Tang <[email protected]>
>>>>
>>>> Before patch:
>>>> ```
>>>> #cat btrees/inodes/keys
>>>> u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
>>>> flags= (16300000)
>>>> ```
>>>>
>>>> After patch:
>>>> ```
>>>> #cat btrees/inodes/keys
>>>> u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
>>>> mode=40755
>>>> flags= (16300000)
The flags also with the space after "=". Is it reseonable?
>>> This would print a newline for keys that don't have a value...
>> The original intention was to make the display of the printed content in
>> '__bch2_inode_unpacked_to_text ()' consistent, without considering other
>> callbacks.
>>
>> Or just modify it in the following way?
>
> Yeah, that's better
>
> Do it off my master branch though, there's some printbuf imprevements in
> there.
>
> https://evilpiepirate.org/git/bcachefs.git
>
>> --- a/fs/bcachefs/inode.c
>> +++ b/fs/bcachefs/inode.c
>> @@ -534,6 +534,8 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
>> bkey_s_c k,
>>  static void __bch2_inode_unpacked_to_text(struct printbuf *out,
>> struct bch_inode_unpacked *inode)
>>  {
>> +       prt_newline(out);
>> +
>>         printbuf_indent_add(out, 2);
>>         prt_printf(out, "mode=%o", inode->bi_mode);
>>         prt_newline(out);
>>
>>
>> Thanks,
>> Youling.
>

2024-04-17 03:22:09

by Hongbo Li

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`



On 2024/4/17 11:16, Hongbo Li wrote:
>
>
> On 2024/4/17 10:59, Kent Overstreet wrote:
>> On Wed, Apr 17, 2024 at 10:50:10AM +0800, Youling Tang wrote:
>>> Hi, Kent
>>> On 17/04/2024 10:20, Kent Overstreet wrote:
>>>> On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
>>>>> From: Youling Tang <[email protected]>
>>>>>
>>>>> Before patch:
>>>>> ```
>>>>>    #cat btrees/inodes/keys
>>>>>    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:   mode=40755
>>>>>      flags= (16300000)
>>>>> ```
>>>>>
>>>>> After patch:
>>>>> ```
>>>>>    #cat btrees/inodes/keys
>>>>>    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
>>>>>      mode=40755
>>>>>      flags= (16300000)
> The flags also with the space after "=". Is it reseonable?
Sorry, I misspell. I mean whether it is reasonable.
>>>> This would print a newline for keys that don't have a value...
>>> The original intention was to make the display of the printed content in
>>> '__bch2_inode_unpacked_to_text ()' consistent, without considering other
>>> callbacks.
>>>
>>> Or just modify it in the following way?
>>
>> Yeah, that's better
>>
>> Do it off my master branch though, there's some printbuf imprevements in
>> there.
>>
>> https://evilpiepirate.org/git/bcachefs.git
>>
>>> --- a/fs/bcachefs/inode.c
>>> +++ b/fs/bcachefs/inode.c
>>> @@ -534,6 +534,8 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
>>> bkey_s_c k,
>>>   static void __bch2_inode_unpacked_to_text(struct printbuf *out,
>>> struct bch_inode_unpacked *inode)
>>>   {
>>> +       prt_newline(out);
>>> +
>>>          printbuf_indent_add(out, 2);
>>>          prt_printf(out, "mode=%o", inode->bi_mode);
>>>          prt_newline(out);
>>>
>>>
>>> Thanks,
>>> Youling.
>>
>

2024-04-17 05:52:42

by Youling Tang

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`

Hi, Kent & Hongbo
On 17/04/2024 11:21, Hongbo Li wrote:
>
>
> On 2024/4/17 11:16, Hongbo Li wrote:
>>
>>
>> On 2024/4/17 10:59, Kent Overstreet wrote:
>>> On Wed, Apr 17, 2024 at 10:50:10AM +0800, Youling Tang wrote:
>>>> Hi, Kent
>>>> On 17/04/2024 10:20, Kent Overstreet wrote:
>>>>> On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
>>>>>> From: Youling Tang <[email protected]>
>>>>>>
>>>>>> Before patch:
>>>>>> ```
>>>>>>    #cat btrees/inodes/keys
>>>>>>    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
>>>>>>      flags= (16300000)
>>>>>> ```
>>>>>>
>>>>>> After patch:
>>>>>> ```
>>>>>>    #cat btrees/inodes/keys
>>>>>>    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
>>>>>>      mode=40755
>>>>>>      flags= (16300000)
>> The flags also with the space after "=". Is it reseonable?
> Sorry, I misspell. I mean whether it is reasonable.
>>>>> This would print a newline for keys that don't have a value...
>>>> The original intention was to make the display of the printed
>>>> content in
>>>> '__bch2_inode_unpacked_to_text ()' consistent, without considering
>>>> other
>>>> callbacks.
>>>>
>>>> Or just modify it in the following way?
>>>
>>> Yeah, that's better
>>>
>>> Do it off my master branch though, there's some printbuf
>>> imprevements in
>>> there.
>>>
>>> https://evilpiepirate.org/git/bcachefs.git
I will make the following changes based on the master branch,

--- a/fs/bcachefs/inode.c

+++ b/fs/bcachefs/inode.c
@@ -534,12 +534,13 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
bkey_s_c k,
 static void __bch2_inode_unpacked_to_text(struct printbuf *out,
                                          struct bch_inode_unpacked *inode)
 {
+       prt_printf(out, "\n");
        printbuf_indent_add(out, 2);
        prt_printf(out, "mode=%o\n", inode->bi_mode);

        prt_str(out, "flags=");
        prt_bitflags(out, bch2_inode_flag_strs, inode->bi_flags & ((1U
<< 20) - 1));
-       prt_printf(out, " (%x)\n", inode->bi_flags);
+       prt_printf(out, "(%x)\n", inode->bi_flags);
>>>
>>>> --- a/fs/bcachefs/inode.c
>>>> +++ b/fs/bcachefs/inode.c
>>>> @@ -534,6 +534,8 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
>>>> bkey_s_c k,
>>>>   static void __bch2_inode_unpacked_to_text(struct printbuf *out,
>>>> struct bch_inode_unpacked *inode)
>>>>   {
>>>> +       prt_newline(out);
>>>> +
>>>>          printbuf_indent_add(out, 2);
>>>>          prt_printf(out, "mode=%o", inode->bi_mode);
>>>>          prt_newline(out);
>>>>
>>>>
>>>> Thanks,
>>>> Youling.
>>>
>>

2024-04-17 09:49:55

by Hongbo Li

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`



On 2024/4/17 13:51, Youling Tang wrote:
> Hi, Kent & Hongbo
> On 17/04/2024 11:21, Hongbo Li wrote:
>>
>>
>> On 2024/4/17 11:16, Hongbo Li wrote:
>>>
>>>
>>> On 2024/4/17 10:59, Kent Overstreet wrote:
>>>> On Wed, Apr 17, 2024 at 10:50:10AM +0800, Youling Tang wrote:
>>>>> Hi, Kent
>>>>> On 17/04/2024 10:20, Kent Overstreet wrote:
>>>>>> On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
>>>>>>> From: Youling Tang <[email protected]>
>>>>>>>
>>>>>>> Before patch:
>>>>>>> ```
>>>>>>>    #cat btrees/inodes/keys
>>>>>>>    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
>>>>>>>      flags= (16300000)
>>>>>>> ```
>>>>>>>
>>>>>>> After patch:
>>>>>>> ```
>>>>>>>    #cat btrees/inodes/keys
>>>>>>>    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
>>>>>>>      mode=40755
>>>>>>>      flags= (16300000)
>>> The flags also with the space after "=". Is it reseonable?
>> Sorry, I misspell. I mean whether it is reasonable.
>>>>>> This would print a newline for keys that don't have a value...
>>>>> The original intention was to make the display of the printed
>>>>> content in
>>>>> '__bch2_inode_unpacked_to_text ()' consistent, without considering
>>>>> other
>>>>> callbacks.
>>>>>
>>>>> Or just modify it in the following way?
>>>>
>>>> Yeah, that's better
>>>>
>>>> Do it off my master branch though, there's some printbuf
>>>> imprevements in
>>>> there.
>>>>
>>>> https://evilpiepirate.org/git/bcachefs.git
> I will make the following changes based on the master branch,
>
> --- a/fs/bcachefs/inode.c
>
> +++ b/fs/bcachefs/inode.c
> @@ -534,12 +534,13 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
> bkey_s_c k,
>  static void __bch2_inode_unpacked_to_text(struct printbuf *out,
>                                           struct bch_inode_unpacked
> *inode)
>  {
> +       prt_printf(out, "\n");
>         printbuf_indent_add(out, 2);
>         prt_printf(out, "mode=%o\n", inode->bi_mode);
>
>         prt_str(out, "flags=");
>         prt_bitflags(out, bch2_inode_flag_strs, inode->bi_flags & ((1U
> << 20) - 1));
> -       prt_printf(out, " (%x)\n", inode->bi_flags);
> +       prt_printf(out, "(%x)\n", inode->bi_flags);
I think it's ok. Maybe use prt_newline(out) is better than
prt_printf(out, "\n");
>>>>
>>>>> --- a/fs/bcachefs/inode.c
>>>>> +++ b/fs/bcachefs/inode.c
>>>>> @@ -534,6 +534,8 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
>>>>> bkey_s_c k,
>>>>>   static void __bch2_inode_unpacked_to_text(struct printbuf *out,
>>>>> struct bch_inode_unpacked *inode)
>>>>>   {
>>>>> +       prt_newline(out);
>>>>> +
>>>>>          printbuf_indent_add(out, 2);
>>>>>          prt_printf(out, "mode=%o", inode->bi_mode);
>>>>>          prt_newline(out);
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Youling.
>>>>
>>>

2024-04-17 10:08:11

by Kent Overstreet

[permalink] [raw]
Subject: Re: [PATCH] bcachefs: Align the display format of `btrees/inodes/keys`

On Wed, Apr 17, 2024 at 05:49:41PM +0800, Hongbo Li wrote:
>
>
> On 2024/4/17 13:51, Youling Tang wrote:
> > Hi, Kent & Hongbo
> > On 17/04/2024 11:21, Hongbo Li wrote:
> > >
> > >
> > > On 2024/4/17 11:16, Hongbo Li wrote:
> > > >
> > > >
> > > > On 2024/4/17 10:59, Kent Overstreet wrote:
> > > > > On Wed, Apr 17, 2024 at 10:50:10AM +0800, Youling Tang wrote:
> > > > > > Hi, Kent
> > > > > > On 17/04/2024 10:20, Kent Overstreet wrote:
> > > > > > > On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
> > > > > > > > From: Youling Tang <[email protected]>
> > > > > > > >
> > > > > > > > Before patch:
> > > > > > > > ```
> > > > > > > >    #cat btrees/inodes/keys
> > > > > > > >    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0: mode=40755
> > > > > > > >      flags= (16300000)
> > > > > > > > ```
> > > > > > > >
> > > > > > > > After patch:
> > > > > > > > ```
> > > > > > > >    #cat btrees/inodes/keys
> > > > > > > >    u64s 17 type inode_v3 0:4096:U32_MAX len 0 ver 0:
> > > > > > > >      mode=40755
> > > > > > > >      flags= (16300000)
> > > > The flags also with the space after "=". Is it reseonable?
> > > Sorry, I misspell. I mean whether it is reasonable.
> > > > > > > This would print a newline for keys that don't have a value...
> > > > > > The original intention was to make the display of the
> > > > > > printed content in
> > > > > > '__bch2_inode_unpacked_to_text ()' consistent, without
> > > > > > considering other
> > > > > > callbacks.
> > > > > >
> > > > > > Or just modify it in the following way?
> > > > >
> > > > > Yeah, that's better
> > > > >
> > > > > Do it off my master branch though, there's some printbuf
> > > > > imprevements in
> > > > > there.
> > > > >
> > > > > https://evilpiepirate.org/git/bcachefs.git
> > I will make the following changes based on the master branch,
> >
> > --- a/fs/bcachefs/inode.c
> >
> > +++ b/fs/bcachefs/inode.c
> > @@ -534,12 +534,13 @@ int bch2_inode_v3_invalid(struct bch_fs *c, struct
> > bkey_s_c k,
> >  static void __bch2_inode_unpacked_to_text(struct printbuf *out,
> >                                           struct bch_inode_unpacked
> > *inode)
> >  {
> > +       prt_printf(out, "\n");
> >         printbuf_indent_add(out, 2);
> >         prt_printf(out, "mode=%o\n", inode->bi_mode);
> >
> >         prt_str(out, "flags=");
> >         prt_bitflags(out, bch2_inode_flag_strs, inode->bi_flags & ((1U
> > << 20) - 1));
> > -       prt_printf(out, " (%x)\n", inode->bi_flags);
> > +       prt_printf(out, "(%x)\n", inode->bi_flags);
> I think it's ok. Maybe use prt_newline(out) is better than prt_printf(out,
> "\n");

prt_printf("\n") is the same as prt_newline() now