Hm, this is indeed one step further, than i thought :-)
Thanks for this nifty solution!

I will doublecheck your suggestion with Urs and then we'll change it in our next patch update (after some more feedback on this mailing list).
Additional feedback is welcome.

Tnx & best regards,
Oliver


Paul E. McKenney wrote:
On Fri, May 18, 2007 at 11:19:01AM +0200, Oliver Hartkopp wrote:
Hi Urs, Hello Paul,

i assume Paul refers to the can_rx_delete_all() function that adds each receive list entry for rcu removal using the can_rx_delete RCU callback, right?

So the idea would be to create a second RCU callback - e.g. can_rx_delete_list() - that removes the complete list inside the RCU callback?!? The list removal would therefore be processed inside this new can_rx_delete_list() in RCU context and not inside can_rx_delete_all().

@Paul: Was this your intention?

My intention was that the list-removing be placed into can_rcv_lists_delete(),
perhaps as follows:

static void can_rx_delete_all(struct hlist_head *rl)
{
        struct receiver *r;
        struct hlist_node *n;

        hlist_for_each_entry(r, n, rl, list) {
                hlist_del(&r->list);
                kmem_cache_free(rcv_cache, r);
        }
}

static void can_rcv_lists_delete(struct rcu_head *rp)
{
        struct dev_rcv_lists *d = container_of(rp, struct dev_rcv_lists, rcu);

        /* remove all receivers hooked at this netdevice */
        can_rx_delete_all(&d->rx_err);
        can_rx_delete_all(&d->rx_all);
        can_rx_delete_all(&d->rx_fil);
        can_rx_delete_all(&d->rx_inv);
        can_rx_delete_all(&d->rx_eff);
        for (i = 0; i < 2048; i++)
                can_rx_delete_all(&d->rx_sff[i]);
        kfree(d);
}

Then the code in can_notifier() can reduce to the following:

        if (d) {
                hlist_del_rcu(&d->list);

                /* used to be a string of can_rx_delete_all(). */
        } else
                printk(KERN_ERR "can: notifier: receive list not "
                       "found for dev %s\n", dev->name);

        spin_lock_bh(&rcv_lists_lock);

        if (d) {
                call_rcu(&d->rcu, can_rcv_lists_delete);
        }

This moves the traversal work into the callback function.  This is not
a problem for CONFIG_PREEMPT_RT and non-CONFIG_PREEMPT, but not sure
about CONFIG_PREEMPT.

But it sure has the potential to cut down on a bunch of call_rcu()
work...

                                                Thanx, Paul


-
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

Reply via email to