From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Toppins Subject: Re: [PATCH net] tg3: prevent scheduling while atomic splat Date: Wed, 14 Mar 2018 13:27:41 -0400 Message-ID: <266eae62-bef2-6c51-415f-cbacc1669e30@redhat.com> References: Reply-To: jtoppins@redhat.com Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Netdev , Andy Gospodarek , Siva Reddy Kallam , Prashant Sreedharan , Michael Chan , open list To: Michael Chan Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 03/14/2018 01:22 PM, Michael Chan wrote: > On Wed, Mar 14, 2018 at 9:36 AM, Jonathan Toppins wrote: >> The problem was introduced in commit >> 506b0a395f26 ("[netdrv] tg3: APE heartbeat changes"). The bug occurs >> because tp->lock spinlock is held which is obtained in tg3_start >> by way of tg3_full_lock(), line 11571. The documentation for usleep_range() >> specifically states it cannot be used inside a spinlock. >> >> Fixes: 506b0a395f26 ("[netdrv] tg3: APE heartbeat changes") >> Signed-off-by: Jonathan Toppins >> --- >> >> Notes: >> The thing I need reviewed from Broadcom is if the udelay should be 20 >> instead of 10, due to any timing changes introduced by the offending >> patch. > > Thanks. 10 us is correct. > > As a future improvement, we might want to see if we can release the > spinlock and go back to usleep_range(). The wait time is potentially > up to 20 msec which is quite long. Agreed, glad it is not just me wondering why a lock needs to be held for reads. :-)