Return-path: Received: from mail.deathmatch.net ([72.66.92.28]:2933 "EHLO mail.deathmatch.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753285Ab1H3Lva (ORCPT ); Tue, 30 Aug 2011 07:51:30 -0400 Date: Tue, 30 Aug 2011 07:50:22 -0400 From: Bob Copeland To: Thomas Pedersen Cc: "John W. Linville" , linux-wireless@vger.kernel.org, jirislaby@gmail.com, mickflemm@gmail.com, lrodriguez@atheros.com, Javier Cardona Subject: Re: [PATCH] ath5k: Invoke irqsafe version of ieee80211_tx_status() to avoid deadlock Message-ID: <20110830115022.GA22125@hash.lan> (sfid-20110830_135133_055948_96BF34A0) References: <1314236064-6339-1-git-send-email-thomas@cozybit.com> <20110826142210.GE2579@tuxdriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 In-Reply-To: Sender: linux-wireless-owner@vger.kernel.org List-ID: On Mon, Aug 29, 2011 at 11:13:10AM -0700, Thomas Pedersen wrote: > I meant to say "In addition to the above discussion, > ieee80211_tx_status() should not be called from interrupt context > anyway". > > > the very change the patch added. ?It's running in a bottom half > > though, not a hard irq. > > Even in a bottom half we're still in "interrupt" context, right? Yes, but my interpretation is that _irqsafe() in this case refers to hard irq context and not softirq context. So it's still atomic, but tx_status() does things like kfree_skb() directly instead of deferring that to another softirq. There are 3 contexts in Linux: process, softirq, and hardirq, and given that we have 3 corresponding tx_status functions (tx_status_ni, tx_status, tx_status_irqsafe), this interpretation makes sense, no? Someone correct me if I'm wrong here. It seems to me this patch warrants some discussion or some comments in the .h files about how drivers can be called back into themselves; otherwise it's very hard for a driver writer to get locking right without examining the stack. And dropping locks around various API calls means you have to validate the protected state when you get the lock back, it's not a panacea. -- Bob Copeland %% www.bobcopeland.com