Re: [PATCH 2/2] btrfs: pretty print leaked root name

[Josef Bacik <josef@toxicpanda.com>]

On 8/21/20 6:13 AM, David Sterba wrote:
> On Fri, Aug 21, 2020 at 10:35:38AM +0300, Nikolay Borisov wrote:
>>
>>
>> On 20.08.20 г. 23:00 ч., Josef Bacik wrote:
>>> I'm a actual human being so am incapable of converting u64 to s64 in my
>>> head, so add a helper to get the pretty name of a root objectid and use
>>> that helper to spit out the name for any special roots for leaked roots,
>>> so I don't have to scratch my head and figure out which root I messed up
>>> the refs for.
>>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> ---
>>>   fs/btrfs/disk-io.c    |  8 +++++---
>>>   fs/btrfs/print-tree.c | 37 +++++++++++++++++++++++++++++++++++++
>>>   fs/btrfs/print-tree.h |  1 +
>>>   3 files changed, 43 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>> index ac6d6fddd5f4..a7358e0f59de 100644
>>> --- a/fs/btrfs/disk-io.c
>>> +++ b/fs/btrfs/disk-io.c
>>> @@ -1506,11 +1506,13 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info)
>>>   	struct btrfs_root *root;
>>>   
>>>   	while (!list_empty(&fs_info->allocated_roots)) {
>>> +		const char *name = btrfs_root_name(root->root_key.objectid);
>>> +
>>>   		root = list_first_entry(&fs_info->allocated_roots,
>>>   					struct btrfs_root, leak_list);
>>> -		btrfs_err(fs_info, "leaked root %llu-%llu refcount %d",
>>> -			  root->root_key.objectid, root->root_key.offset,
>>> -			  refcount_read(&root->refs));
>>> +		btrfs_err(fs_info, "leaked root %s%lld-%llu refcount %d",
>>
>> nit: Won't this string result in some rather awkward looking strings,
>> such as:
>>
>> "leaked root ROOT_TREE<objectid>-<offset>..." i.e shouldn't the
>> (objectid,offset) pair be marked with parentheses?
> 
> I don't understand why need/want to print the offset here. It is from
> the key.offset but for a message we should print it in an understandable
> way.
> 

The offset matters for the TREE_RELOC roots, because it's the object id 
of the fs root that the reloc root refers to.  Thanks,

Josef