lkml.org 
[lkml]   [2009]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/5] can: af_can.c use rcu_barrier() on module unload.
    Paul E. McKenney wrote:
    > On Mon, Jun 08, 2009 at 03:11:38PM +0200, Jesper Dangaard Brouer wrote:
    >> This module uses rcu_call() thus it should use rcu_barrier()
    >> on module unload.
    >
    > This does appear to make things better!!!
    >
    > However, I don't understand why it is safe to do the following in
    > can_exit():
    >
    > hlist_for_each_entry_safe(d, n, next, &can_rx_dev_list, list) {
    > hlist_del(&d->list);
    > kfree(d);
    > }
    >
    > Given that this list is scanned by RCU readers, shouldn't this kfree()
    > be something like "call_rcu(&d->rcu, can_rx_delete_device);"?
    >
    > Also, what frees up the "struct receiver" structures?

    Hi Paul,

    af_can.c only provides an infrastructure for PF_CAN modules like can-raw.ko,
    can-bcm.ko or can-isotp.ko.

    Please take a look into can_notifier() in net/can/af_can.c and raw_notifier()
    in net/can/raw.c:

    The receivers are removed when the appropriate socket is closed that created
    the belonging receivers. And you can not remove can.ko (af_can.c) when another
    PF_CAN protocol like can-raw.ko is using it.

    So when a netdev notifier removes the interface both the PF_CAN protocol (e.g.
    can-raw.ko) and the PF_CAN core (af_can.c) cleans up all receivers and finally
    removes the per-interface structure dev_rcv_lists (e.g. for can0).

    In can_exit() all the dev_rcv_lists for ARPHRD_CAN interfaces are removed that
    had been created by NETDEV_REGISTER notifier and are unused by any of the
    PF_CAN protocols and therefore without any receivers attached to them.

    The list is protected by spin_lock(&can_rcvlists_lock) - which is probably not
    even needed in this particular case - and there is no PF_CAN protocol
    registered at this time. So it's really save to remove the empty dev_rcv_lists
    structs here that do not link to any receivers.

    Puh - much text. But i hope it clarifies it.

    Thinking about the rcu stuff again, rcu_barrier() still makes sense when you
    are unloading the module chain of can-raw.ko and can.ko very fast.

    Regards,
    Oliver


    >> Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
    >> ---
    >>
    >> net/can/af_can.c | 2 ++
    >> 1 files changed, 2 insertions(+), 0 deletions(-)
    >>
    >> diff --git a/net/can/af_can.c b/net/can/af_can.c
    >> index 10f0528..e733725 100644
    >> --- a/net/can/af_can.c
    >> +++ b/net/can/af_can.c
    >> @@ -903,6 +903,8 @@ static __exit void can_exit(void)
    >> }
    >> spin_unlock(&can_rcvlists_lock);
    >>
    >> + rcu_barrier(); /* Wait for completion of call_rcu()'s */
    >> +
    >> kmem_cache_destroy(rcv_cache);
    >> }


    \
     
     \ /
      Last update: 2009-06-08 19:03    [W:2.082 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site