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

David Sterba
2020-09-09T07:38:59Z

On Mon, Sep 07, 2020 at 02:28:54PM +0200, David Sterba wrote:
> On Thu, Sep 03, 2020 at 02:29:51PM -0400, Josef Bacik wrote:
> > --- a/fs/btrfs/print-tree.c
> > +++ b/fs/btrfs/print-tree.c
> > @@ -7,6 +7,45 @@
> > #include "disk-io.h"
> > #include "print-tree.h"
> >
> > +struct name_map {
> > + u64 id;
> > + char *name;
>
> const char* or maybe even a fixed length string, removing the
> _TREE suffix
>
> > +};
> > +
> > +static const struct name_map root_map[] = {
> > + { BTRFS_ROOT_TREE_OBJECTID, "ROOT_TREE" },
> > + { BTRFS_EXTENT_TREE_OBJECTID, "EXTENT_TREE" },
> > + { BTRFS_CHUNK_TREE_OBJECTID, "CHUNK_TREE" },
> > + { BTRFS_DEV_TREE_OBJECTID, "DEV_TREE" },
> > + { BTRFS_FS_TREE_OBJECTID, "FS_TREE" },
> > + { BTRFS_ROOT_TREE_DIR_OBJECTID, "ROOT_TREE_DIR" },
>
> This is not a tree id
>
> > + { BTRFS_CSUM_TREE_OBJECTID, "CSUM_TREE" },
> > + { BTRFS_TREE_LOG_OBJECTID, "TREE_LOG" },
> > + { BTRFS_QUOTA_TREE_OBJECTID, "QUOTA_TREE" },
> > + { BTRFS_UUID_TREE_OBJECTID, "UUID_TREE" },
> > + { BTRFS_FREE_SPACE_TREE_OBJECTID, "FREE_SPACE_TREE" },
> > + { BTRFS_DATA_RELOC_TREE_OBJECTID, "DATA_RELOC_TREE" },
>
> I've noticed we have the id -> str mapping already in
> btrfs_lockdep_keysets, so there could be just one such structure and
> we'd add wrappers around that.
>
> Also the C99 initializers should be used.
>
> > +};
> > +
> > +char *btrfs_root_name(u64 objectid, char *buf)
> > +{
> > + int i;
> > +
> > + if (objectid == BTRFS_TREE_RELOC_OBJECTID) {
> > + snprintf(buf, BTRFS_ROOT_NAME_BUF_LEN,
> > + "TREE_RELOC offset=%llu", objectid);
>
> The reloc tree is an outlier here so I wonder if we'd rather have a nice
> pretty printer of the tree root name and deal with the oddities where
> required rather than inside the helper. In this case there would have to
> be some conditionals in btrfs_check_leaked_roots, as we can't do that in
> the format string.

I had another thought, we want a pretty printer, the above still holds
but needs more restructuring so let's take this. I'll do the fixups and
add it to misc-next.