[v1] bcachefs: Align the display format of `btrees/inodes/keys`

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

Posted by Youling Tang 2 weeks, 1 day ago

From: Youling Tang <tangyouling@kylinos.cn>

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 <tangyouling@kylinos.cn>
---
 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

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

Posted by Kent Overstreet 2 weeks, 1 day ago

On Wed, Apr 17, 2024 at 09:54:48AM +0800, Youling Tang wrote:
> From: Youling Tang <tangyouling@kylinos.cn>
> 
> 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...

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

Posted by Youling Tang 2 weeks, 1 day ago

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 <tangyouling@kylinos.cn>
>>
>> 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.

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

Posted by Kent Overstreet 2 weeks, 1 day ago

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 <tangyouling@kylinos.cn>
> > > 
> > > 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.

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

Posted by Hongbo Li 2 weeks, 1 day ago


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 <tangyouling@kylinos.cn>
>>>>
>>>> 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.
>

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

Posted by Hongbo Li 2 weeks, 1 day ago


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 <tangyouling@kylinos.cn>
>>>>>
>>>>> 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.
>>
>

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

Posted by Youling Tang 2 weeks, 1 day ago

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 <tangyouling@kylinos.cn>
>>>>>>
>>>>>> 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.
>>>
>>

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

Posted by Hongbo Li 2 weeks, 1 day ago


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 <tangyouling@kylinos.cn>
>>>>>>>
>>>>>>> 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.
>>>>
>>>