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

[David Sterba <dsterba@suse.cz>]

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?

We need to print either string name or value, and in one go. Unlike
in progs, I'd rather avoid split stings in kernel and using
pr_cont/KERN_CONT.

The option is see is to supply a buffer to the pretty printer that would
be used in case we need the numerical id. Like:

	while (allocated_roots) {
		char root_name[24];

		...
		btrfs_err(..., "leaked root %s ...",
			get_root_name(id, root_name));
	}

where

	char *get_root_name(u64 id, char *buf) {
		if (numeric root) {
			sprintf(buf, "%llu", ud);
			return buf;
		}
		return <root name from the table>;
	}