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

[Hongbo Li]

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@xxxxxxxxxx>
> > > > > > >
> > > > > > > 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.