[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <72e1b795-7219-41dd-83f2-10a3be2680b2@huawei.com>
Date: Wed, 17 Apr 2024 17:49:41 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Youling Tang <youling.tang@...ux.dev>, Kent Overstreet
<kent.overstreet@...ux.dev>
CC: <linux-bcachefs@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Youling
Tang <tangyouling@...inos.cn>
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 <tangyouling@...inos.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.
>>>>
>>>
Powered by blists - more mailing lists