Re: [PATCH 2/2] genl: Hold reference on correct module while netlink-dump.

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2013-08-22 at 10:44 -0700, Pravin Shelar wrote:

> > This may be more correct than my patch, but I'm not sure it's worth
> > spending the memory. Is there going to be any generic netlink family
> > that actually puts operations into a different module than the family
> > itself? I doubt that.
> >
> I tried to do same, but I do not have access to ops in
> genl_lock_done() function. therefore I decided to store direct pointer
> to module in ops struct.

You mean the family? I still don't like this, it'll be a massive
increase in memory for something like nl80211 that has a lot of
operations. In fact I think we should get rid of being allowed to
register single ops and just force a single array to remove the linked
list as well.

> >> +                     c.module = THIS_MODULE;
> >
> > THIS_MODULE here is useless, this code is always built-in.
> >
> >> +                     if (!try_module_get(ops->module))
> >> +                             return -EPROTONOSUPPORT;
> >
> > Why open-code it? You can just point c.module to the ops module here as
> > well (because generic netlink is built-in) and save yourself the
> > try_module_get stuff.
> >
> 
> In locked genl case genl-dump operation has call graph as follows:
> netlink_dump() -> genl_lock_dumpit() -> ops->dump()
> Therefore I need to take ref on genl module and ops->module.

There's no "genl module", generic netlink is always built in. That was
my point, you don't need to hold any reference to the "genl module"
since it doesn't exist, so you can just point to the ops (family)
module.

johannes