David S. Miller wrote:
From: Ben Greear <[EMAIL PROTECTED]>
Date: Mon, 29 Aug 2005 16:01:11 -0700
Latest netdevice ref-count debugging patch is up. The
patch is against 2.6.13:
http://www.candelatech.com/oss/rfcnt.patch
I reviewed this, and I think the approach can be refined and made more
robust.
The worst part is that you do a kmalloc() on ever reference, and you
shouldn't need to do that.
A quick optimization is to kmalloc chunks of 1000 or so structs at once
and then write a cheap foomalloc method to grab and release them. We
already take a lock (at least in my implementation), so this should be
small overhead.
As you have noted, a netdev refcount grab falls into two categories:
1) something->dev = dev;
dev_get(dev);
2) dev_get(dev);
/* some code where dev might get released but we want
* to preserve the device over all the calls we're doing
*/
func1();
func2();
whatever();
/* Ok we're done, now can let it go for real. */
dev_put(dev);
For the first case, just make a structure to hold this as the
place where the device pointer gets assigned to. This would
allow abstraction, get rid of the memory allocation, and
for proper refcounting in fact. So you'd have something like:
struct netdev;
struct netdev_pointer {
struct netdev *dev;
#ifdef CONFIG_NETDEV_REFCNT_DEBUG
struct hlist_head hash;
const char *file;
int line;
#endif
};
Then in places where we have "struct netdev *dev", you replace it
with "struct netdev_pointer *__dev". You have a routine like:
#define dev_set(netdev_pointer, netdev) \
do { \
netdev_pointer->dev = netdev; \
dev_get(netdev_pointer, netdev); \
netdev_pointer->file = __FILE__; \
netdev_pointer->line = __LINE__; \
} while (0)
that you invoke instead of the standard:
x->dev = dev;
dev_get(dev);
sequence.
Maybe it's just too late..but I am not understanding this. How
do you print all of the current users of the device when you are
trying to release the device and something has a reference to it?
Class #2 is so transient that you can probably just use an array
of dummy struct netdev_pointers that sit in struct netdev itself.
Use, say, 4, and allocate them one by one until you run out.
And you do the linkage into there, and thus these act as the "object"
you're assigning the netdev pointer to that requires the refcount.
You'll be able to store the __FILE__ and __LINE__ information
quite neatly in there.
As soon as we choose 4, someone will need 5. I'd prefer to keep
the storage of the reference information separate, though I am
fine with optimizing so that we should have very few kmallocs/frees
by allocing large chunks of objects at once.
Perhaps an even better idea for class #2 is to use a stack local
"struct netdev_pointer", since that is the mode in which this
case typically occurs, all within the same function so the function
stack frame is live during the entire dummy refrence grab.
Could you actually detect a reference leak in this case? The struct goes out
of scope when the method ends..but since we don't have auto-destructors
like C++, then there is nothing to force the release of the reference.
This whole concept is actually quite generic, and we could thus
apply it to sockets, routes, neighbour table entries, etc.
And Ben you really need to get up to snuff with Linux coding
standards. All of your "StudLyCaps" function names and bad
tabbing hurt my eyes quite a bit and make your patches nearly
impossible to review :-/
Bad caps I can quickly fix..but I actually went to some trouble to
try to get the tabbing correct. Is my patch coming through with spaces
instead of tabs or something like that?
Thanks,
Ben
--
Ben Greear <[EMAIL PROTECTED]>
Candela Technologies Inc http://www.candelatech.com
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
