Re: [PATCH v4 7/7] net: register debugfs file for net_device refcnt tracker

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2025-04-23 at 17:32 -0700, Jakub Kicinski wrote:
> On Wed, 23 Apr 2025 20:04:58 -0400 Jeff Layton wrote:
> > On Wed, 2025-04-23 at 16:53 -0700, Jakub Kicinski wrote:
> > > Names are not unique and IIUC debugfs is not namespaced.

Correct, debugfs is not namespaced.

I meant to say earlier that I'm open to suggestions on how to make the
netdev names unique. Low-level netdev stuff is not my area of
expertise. We can drop this patch if doing so is problematic.

> > > How much naming the objects in a "user readable" fashion actually
> > > matter? It'd be less churn to create some kind of "object class"
> > > with a directory level named after what's already passed to
> > > ref_tracker_dir_init() and then id the objects by the pointer value 
> > > as sub-dirs of that?  
> > 
> > That sounds closer to what I had done originally. Andrew L. suggested
> > the flat directory that this version represents. I'm fine with whatever
> > hierarchy, but let's decide that before I respin again.
> 
> Sorry about that :(
> 

No worries...but we do need to decide what this directory hierarchy
should look like.

Andrew's point earlier was that this is just debugfs, so a flat
"ref_tracker" directory full of files is fine. I tend to agree with
him; NAME_MAX is 255, so we have plenty of room to make uniquely-named
files.

We could build a dir hierarchy though. Something like:

- ref_tracker
    + netdev
    + netns
    
...and then each object class subdir would hold uniquely-named files.
Each subsys would need to create the directory and then the files
within it, but we could add helpers for that.

Note that this is debugfs, so we can always change that later as it's
not counted as part of ABI.

Which way should we do it?
 
> > When I was tracking down net namespace leaks recently, it was very nice
> > to have the inode number of the leaked netns's in the filenames. I
> > would have probably had to grovel around with drgn to figure out the
> > address if that weren't embedded in the name. I think we probably ought
> > to leave it up to each subsystem how it names its files. The
> > discriminators between different types of objects can vary wildly.
> > 
> > One thing that might be simpler is to make ref_tracker_dir_debugfs() a
> > varargs function and allow passing in a format string and set of
> > arguments for it. That might make things simpler for the callers.
> 
> Yes, cutting out the formatting in the callers would definitely 
> be a win. Maybe that'd make the whole thing sufficiently palatable :)

Ok, I'll plan to change that. That does make the callers a lot cleaner.
-- 
Jeff Layton <jlayton@kernel.org>