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]

Date:	Mon, 23 Jun 2014 23:38:23 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Casey Leedom <leedom@...lsio.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

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