At 11:54 PM 4/28/2003, Arnaldo Carvalho de Melo wrote:
>Em Mon, Apr 28, 2003 at 10:27:28PM -0700, David S. Miller escreveu:
>> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxxxxxxxx>
>> Date: Tue, 29 Apr 2003 03:12:27 -0300
>>
>> Max, take a look and see if this same approach can be used in
>> bluetooth, I bet it can, its just a matter of not using struct
>> net_proto_family for bt_proto, just like pppox already was doing
>> before my changes :-)
>>
>> Something similar can be done for ipv4/ipv6 by adding a struct module
>> *owner member to struct inet_protosw etc. etc.
>
>yes
>
>> Although the idea is conceptually sound, you miss one crucial thing.
>> Such struct sock's reference _TWO_ modules, the "PPPOE" module
>> and the "PPPOX" module.
This is not a problem. PPPOX (or ipv4/ipv6) module is not going to
go away simply because PPPOE (or TCP/UDP) uses symbols from it.
There is no need to bump refcounters here.
I think it's safe to say that protocols within the family always use
symbols from main family module (they have to at list register/unregister).
So this is naturally taken care of.
>But what is the problem? at pppox_sk_alloc time I bump the PPPOE module refcnt,
>making it safe, then it calls sk_alloc where it bumps the PPPOX module, making
>it safe as well, so I'm taking care of both PPPOE and PPPOX.
Bumping refcount for PPPOX is an overhead without any benefits. Read above.
>> struct module *owner;
>
>This one is the net_families[net_family]->owner
>
>> struct module *sub_owner;
>
>this one is the pppox_protos[protocol]->owner
>
>I thought about it, but I don't see why the current scheme doesn't handle
>it, care to elaborate a bit more? I don't doubt that I may be missing some
>subtlety :-)
Ok. I'm going to ask this here again (to make sure that it's answered :))
- Why do we have to bump module refcount for 'struct sock' with _default_
callbacks ?
My answer is that we don't have to. I'd even say that we shouldn't. Module is
not
referenced from 'struct sock' in that case.
- Why are we not allowing net_family unregistration until all family sockets
are gone ?
From what I see it's only because current code does
'module_put(net_families[family]->owner)'.
But we don't care whether net_family is registered or not if we know who owns
the socket (i.e. sock->owner).
So far nobody gave me a clear answer to those questions. If we don't have to
meet those
two restriction I don't see the point in creating all this
net_family_get()/xxx_family_get(),
etc, infrastructure. Patches and BK stuff that I've seen so far added more code
(and a bit
more overhead) than my original patch with sock->owner/sk->owner/sk_set_owner().
Yet they provide no additional flexibility or functionality.
I hate to repeat myself and please forgive me for that. But how is the current
infrastructure better than following ?
----
sock_create()
{
...
if (!try_module_get(npf->owner))
goto fail;
sock->owner = npf->owner;
if (family_sock_create() < 0) {
sock_release(sock);
goto fail;
}
...
}
From this point on we don't care if net_proto_family is still registered or
not, we know
who owns the socket.
sock_accept()
{
...
__module_get(sock->owner);
newsock->owner = sock->owner;
...
}
sock_release()
{
...
module_put(sock->owner);
...
}
family_sock_create() (i.e. bt_sock_create(), etc)
{
...
#ifdef SPECIAL_FAMILY_THAT_HANDLES_PROTOCOLS_IN_SEPARATE_MODULES
if (!try_module_get(proto_module))
goto fail;
prev_owner = sock->owner;
sock->owner = proto_module;
module_put(prev_owner);
#else
/* Other families do not have to do anything */
#endif
...
}
That's it for 'struct socket'. And I mean that is _it_. Family can be
unregistered
and stuff, we don't care, and why should we.
Now 'struct sock'. It can exist independently from 'struct socket' and therefor
is a separate issue.
We only care about callbacks (sk->data_ready(), etc) and private sk slab
caches. Some
protocols simply use default callback which belong to non modular part of the
kernel.
Those don't have to do anything. They will just work.
Other protocols replace callbacks and therefor have to make sure that module is
still
loaded while sk exists. For those modules we need the following
void sk_set_owner(struct module *owner)
{
/* Module must already hold reference when it calls this function. */
__module_get(owner);
sk->owner = owner;
}
sk_free()
{
...
module_put(sk->owner);
...
}
xxx_proto_set_sk_callbacks()
{
...
sk_set_owner(THIS_MODULE);
sk->destroy = proto_destroy;
...
}
That's it. There is no need to modify sk_alloc() or anything else. So it's not
going
to introduce any overhead during socket creation from protocols that aren't
modules or
use default callback. And it gives us a flexibility of being able to pass
ownership of
the socket to another module or release module without having to access global
family
array.
For example if protocol wants to be unloaded but socket is still being used by
net code
it could do something like
write_lock(&sk->callback_lock);
sk->data_ready = default_data_ready;
... other non default callbacks ...
module_put(sk->owner);
sk_set_owner(NULL);
write_unlock(&sk->callback_lock);
---
Thanks
Max
|