[lkml]   [2003]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH/RFC] New module refcounting for net_proto_family
    At 07:54 PM 2/18/2003, Rusty Russell wrote:
    >In message <> you write:
    >> At 07:46 PM 2/17/2003, David S. Miller wrote:
    >> >After talking to Alexey, I don't like this patch.
    >> >
    >> >The new module subsystem was supposed to deal with things
    >> >like this cleanly, and this patch is merely a hack to cover
    >> >up for it's shortcomings.
    >I don't quite understand.
    >There are some issue with this patch, however.
    >Firstly, the owner field should probably be in struct proto_ops not
    >struct socket, where the function pointers are.
    struct proto_ops doesn't exists on its own without struct socket.
    I think it make sense to simply keep track of the sockets but I don't
    see any problem with putting it in proto_ops.

    struct sock is different though. callbacks are inside.

    >The sk thing looks reasonable at first glance. Getting a reference to
    >npf->owner, then holding it for the socket is a little confusing, but
    >an obvious optimization over a naive "get, use, drop, get".
    That was an optimization indeed. There is no point in dropping reference

    >In sys_accept:
    >> @@ -1196,9 +1198,13 @@
    >> if (!(newsock = sock_alloc()))
    >> goto out_put;
    >> - newsock->type = sock->type;
    >> - newsock->ops = sock->ops;
    >> + newsock->type = sock->type;
    >> + newsock->ops = sock->ops;
    >> + newsock->owner = sock->owner;
    >> + try_module_get(sock->owner);
    >> + newsock->owner = sock->owner;
    >> +
    >> err = sock->ops->accept(sock, newsock, sock->file->f_flags);
    >> if (err < 0)
    >> goto out_release;
    >You still need to check the result of try_module_get, and fail if it
    >fails. The *only* time this will fail is when someone is doing an
    >"rmmod --wait" on the module, which presumably means they really do
    >not want you to increase the reference count furthur.
    Ohh, I see. My assumption here was that we know for sure
    that module is alive at this point since we already hold a reference to the
    first socket. Actually I was going to send another email and ask for unconditional
    module_get() specifically for the cases like that.

    Even after your explanation I still think we need unconditional module_get() there.
    Because in this case 'rmmod --wait' will simply brake accept() logic. I mean it'll
    keep waiting until listening socket is destroyed (i.e. until socket app is killed)
    but accept() will mysteriously fail for no good reason.
    Comments ?


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 13:33    [W:0.032 / U:0.564 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site