netdev - Re: [patch 2/7] CAN: Add PF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 18 May 2007 17:03:41 +0200
From:	Oliver Hartkopp <socketcan@...tkopp.net>
To:	paulmck@...ux.vnet.ibm.com, Urs Thuermann <urs@...ogud.escape.de>
CC:	netdev@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Oliver Hartkopp <oliver.hartkopp@...kswagen.de>,
	Urs Thuermann <urs.thuermann@...kswagen.de>
Subject: Re: [patch 2/7] CAN: Add PF_CAN core module

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html