> -----Original Message----- > From: Eric Dumazet <eric.duma...@gmail.com> > Sent: 10-Jul-19 04:00 > To: Jon Maloy <jon.ma...@ericsson.com>; Eric Dumazet > <eric.duma...@gmail.com>; Chris Packham > <chris.pack...@alliedtelesis.co.nz>; ying....@windriver.com; > da...@davemloft.net > Cc: net...@vger.kernel.org; tipc-discuss...@lists.sourceforge.net; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH] tipc: ensure skb->lock is initialised > > > > On 7/9/19 10:15 PM, Jon Maloy wrote: > > > > It is not only for lockdep purposes, -it is essential. But please provide > > details > about where you see that more fixes are needed. > > > > Simple fact that you detect a problem only when skb_queue_purge() is called > should talk by itself. > > As I stated, there are many places where the list is manipulated _without_ its > spinlock being held.
Yes, and that is the way it should be on the send path. > > You want consistency, then > > - grab the spinlock all the time. > - Or do not ever use it. That is exactly what we are doing. - The send path doesn't need the spinlock, and never grabs it. - The receive path does need it, and always grabs it. However, since we don't know from the beginning which path a created message will follow, we initialize the queue spinlock "just in case" when it is created, even though it may never be used later. You can see this as a violation of the principle you are stating above, but it is a prize that is worth paying, given savings in code volume, complexity and performance. > > Do not initialize the spinlock just in case a path will use skb_queue_purge() > (instead of using __skb_queue_purge()) I am ok with that. I think we can agree that Chris goes for that solution, so we can get this bug fixed. ///jon
