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

[Andrew Lunn]

> > > We'll need some input from the i915 folks then.
> > 
> > That is why i think it would be better to add a warning, give the i915
> > folks a heads up, and leave them to fix it how they want. We want the
> > warning anyway to cover new refcount instances being added.
> > 
> 
> There will definitely be a pr_warn() if creation fails. I was hoping to
> send a suggested change alongside the patchset, but I may have to just
> leave it up to them.
> 
> I threw together a draft patchset that auto-registers a debugfs file
> for every call to ref_tracker_dir_init(). The problem I've hit now
> though is that at least in the networking cases, the kernel calls
> ref_tracker_dir_init() very early in the process of creating a new
> objects, so we're not getting good names here:
> 
> The "name" in alloc_netdev_mqs is actually a format string, so we end
> up with a name like "eth%d" here:
> 
> net/core/dev.c: ref_tracker_dir_init(&dev->refcnt_tracker, 128, name);
 
Oh, yes, never thought about that...

It also seems like it might be a common problem, the subsystem defined
name has not been given yet, although the core dev_name(dev) should
work. But in netdev drivers, alloc_netdev_mqs() does not have access
to that.

> At the point that we call these (preinit), net->ns.inum hasn't been
> assigned yet, so we can't incorporate it properly into the dentry name:
> 
> 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");
> 
> We could do the ref_tracker_dir_init() calls later, but we may end up
> missing some references that way (or end up crashing because the "dir"
> isn't initialized.
> 
> My current thinking is to add a new ref_tracker_dir_finalize() function
> that we could use to finalize the name and register the debugfs files
> after we have the requisite info. It's an extra manual step, but I like
> that better than moving around the ref_tracker_dir_init() calls.

Agree. Although, bike shedding, i don't really like the _finalize in
ref_tracker_dir_finalize(). Ideally this should be optional and
populate debugfs.  _finalize does not sound particularly optional. So
maybe ref_tracker_dir_debugfs()?

This also has the advantage of we need to worry less about GPU
drivers. They see no change in behaviour, but we can give them a heads
up the new call exists if they want to use it to populate debugfs.

	Andrew