On Tue, 2010-11-23 at 07:51 +0200, Juuso Oikarinen wrote: > On Mon, 2010-11-22 at 14:52 +0200, Luciano Coelho wrote: > > On Thu, 2010-11-18 at 15:19 +0200, juuso.oikarinen@xxxxxxxxx wrote: > > > From: Juuso Oikarinen <juuso.oikarinen@xxxxxxxxx> > > > > > > It is possible that the op_remove_interface function is invoked exactly at > > > the same time has hw recovery is started. In this case it is possible for the > > > interface to be already removed in the op_remove_interface call, which > > > currently leads to a kernel warning and a subsequent kernel crash. > > > > > > Fix this by ignoring the op_remove_interface call if the interface is already > > > down at that point. > > > > > > Signed-off-by: Juuso Oikarinen <juuso.oikarinen@xxxxxxxxx> > > > --- > > > > [...] > > > > > diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c > > > index 31f0e2f..11b0477 100644 > > > --- a/drivers/net/wireless/wl12xx/main.c > > > +++ b/drivers/net/wireless/wl12xx/main.c > > > @@ -1157,10 +1157,16 @@ static void wl1271_op_remove_interface(struct ieee80211_hw *hw, > > > struct wl1271 *wl = hw->priv; > > > > > > mutex_lock(&wl->mutex); > > > - WARN_ON(wl->vif != vif); > > > - __wl1271_op_remove_interface(wl); > > > - mutex_unlock(&wl->mutex); > > > + /* > > > + * wl->vif can be null here if someone shuts down the interface > > > + * just when hardware recovery has been started. > > > + */ > > > + if (wl->vif) { > > > + WARN_ON(wl->vif != vif); > > > + __wl1271_op_remove_interface(wl); > > > + } > > > > Should you still remove the interface if the vif you received is wrong? > > Surely, something is totally wrong if you get a different vif, but maybe > > removing the interface here will just confuse things even more? > > Dunno if it would be better to remove or leave unremoved - probably does > not matter. If the vif is wrong, there is some serious bug somewhere, > and probably the result is serious instability anyway. > > The WARN_ON is there just to validate the assumption the driver makes > about the vif, and this function call. If there is a bug somewhere, or > the assumption somehow changes, the warning will be a clear indication > of it. Yep, it's okay like this. And it was like that earlier anyway. I was just wondering if it would be possible for a similar thing as wl->vif being NULL (which your patch addresses) happen, such as wl->vif changing before we had the chance to fully remove the interface. It will probably not happen and, as you say, the WARN_ON will be a good indication that something went wrong. Applied to wl12xx/master. Thanks! -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html