Re: [PATCH v2 2/2] net: add debugfs files for showing netns refcount tracking info

[Jeff Layton]

On Thu, 2025-04-10 at 16:12 +0200, Andrew Lunn wrote:
> > Oh, ok. I guess you mean these names?
> > 
> >         ref_tracker_dir_init(&net->refcnt_tracker, 128, "net refcnt");
> >         ref_tracker_dir_init(&net->notrefcnt_tracker, 128, "net notrefcnt");
> > 
> > Two problems there:
> > 
> > 1/ they have an embedded space in the name which is just painful. Maybe we can replace those with underscores?
> > 2/ they aren't named in a per-net namespace way
> 
> So the first question is, are the names ABI? Are they exposed to
> userspace anywhere? Can we change them?
> 
> If we can change them, space to _ is a simple change. Another option
> is what hwmon does, hwmon_sanitize_name() which turns a name into
> something which is legal in a filesystem. If all of this code can be
> pushed into the core tracker, so all trackers appear in debugfs, such
> a sanitiser is the way i would go.
> 
> And if we can change the name, putting the netns into the name would
> also work. There is then no need for the directory, if they have
> unique names.
> 
> Looking at other users of ref_tracker_dir_init():
> 
> ~/linux$ grep -r ref_tracker_dir_init
> lib/test_ref_tracker.c:	ref_tracker_dir_init(&ref_dir, 100, "selftest");
> 
> Can only be loaded once, so is unique.
>
> drivers/gpu/drm/i915/intel_wakeref.c:	ref_tracker_dir_init(&wf->debug, INTEL_REFTRACK_DEAD_COUNT, name);
> 
> Looks like it is unique for one GPU, but if you have multiple GPUs
> they are not unique.
> 

We'll need some input from the i915 folks then.

> drivers/gpu/drm/i915/intel_runtime_pm.c:	ref_tracker_dir_init(&rpm->debug, INTEL_REFTRACK_DEAD_COUNT, dev_name(rpm->kdev));
> 
> At a guess kdev is unique.
> 

Yeah, looks like it.

> drivers/gpu/drm/display/drm_dp_tunnel.c:	ref_tracker_dir_init(&mgr->ref_tracker, 16, "dptun");
> 
> Probably not unique.
> 

Looking more here, perhaps we can incorporate mgr->dev->unique into the
name? I don't know much about dp drivers. Can multiple mgrs point to
the same device?

> net/core/net_namespace.c:	ref_tracker_dir_init(&net->refcnt_tracker, 128, "net refcnt");
> net/core/net_namespace.c:	ref_tracker_dir_init(&net->notrefcnt_tracker, 128, "net notrefcnt");
> 
> Not unique across name spaces, but ...

That's easily fixable. net->ns.inum is unique.

> 
> So could the tracker core check if the debugfs file already exists,
> emit a warning if it does, and keep going? I think debugfs_lookup()
> will tell you if a file already exists, or debugfs_create_file() will
> return -EEXIST, which is probably safer, no race condition.
> 


Yeah, that should be possible. I think too that we can eliminate the
buffer management in this codepath by allowing pr_ostream() to output
directly to a seq_file. I'll look into that as well.

Thanks for the input!
-- 
Jeff Layton <jlayton@xxxxxxxxxx>