netdev - Re: [PATCH][RFC] cxgb4: Not need to hold the adap_rcu_lock lock when read adap_rcu

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]

Message-ID: <53A9AA62.7070503@chelsio.com>
Date:	Tue, 24 Jun 2014 09:42:10 -0700
From:	Casey Leedom <leedom@...lsio.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	David Miller <davem@...emloft.net>, rongqing.li@...driver.com,
	netdev@...r.kernel.org, hariprasad@...lsio.com,
	greearb@...delatech.com
Subject: Re: [PATCH][RFC] cxgb4: Not need to hold the adap_rcu_lock lock when
 read adap_rcu_list

   Okay, so it looks like everyone is happy with Li's patch.  So I think 
I'm supposed to say:

   Acked-by: Casey Leedom <leedom@...lsio.com>

Casey

On 06/23/14 23:38, Eric Dumazet wrote:
> On Mon, 2014-06-23 at 15:35 -0700, Casey Leedom wrote:
>> On 06/23/14 14:50, David Miller wrote:
>>> From: <rongqing.li@...driver.com>
>>> Date: Fri, 20 Jun 2014 17:32:36 +0800
>>>
>>>> cxgb4_netdev maybe lead to dead lock, since it uses a spin lock, and be called
>>>> in both thread and softirq context, but not disable BH, the lockdep report is
>>>> below; In fact, cxgb4_netdev only reads adap_rcu_list with RCU protection, so
>>>> not need to hold spin lock again.
>>> I think this change is fine, and correct, but I would like to see some
>>> reviews from the cxgb4 maintainers.
>>     Thanks David.  Hari is gone on PTO so I think I'm the next logical
>> person ... :-)
>>
>>     I've gone back and reviewed the original patch, Eric Dumazet6's reply
>> and revised patch and compared that against this proposed patch.  Li
>> RongQing is submitting the same patch that Eric suggested with the
>> addition of a call to synchronize_rcu() the in driver remove()
>> function.  I'm not super familiar with the RCU system but that addition
>> certainly seems innocuous enough.  Other than that, everything looks fine.
> Yes, the synchronize_rcu() is needed.
>
> In fact I do not really understand why RCU was even used in this slow
> path...
>
>
>

--
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