[PATCH] ath10k: fix kernel panic while shutting down AP

[Michal Kazior]

On 8 October 2014 12:48, Rajkumar Manoharan <rmanohar at qti.qualcomm.com> wrote:
> On Wed, Oct 08, 2014 at 12:52:04PM +0300, Kalle Valo wrote:
>> Rajkumar Manoharan <rmanohar at qti.qualcomm.com> writes:
>>
>> > The commit "ath10k: workaround fw beaconing bug" is freeing
>> > DMA-coherent memory in irq context which is hitting BUG ON
>> > in ARM platforms. Fix this by moving dma_free out of spin
>> > lock.
>
>> [...]
>>
>> > @@ -2404,8 +2389,15 @@ void ath10k_halt(struct ath10k *ar)
>> >
>> >     spin_lock_bh(&ar->data_lock);
>> >     list_for_each_entry(arvif, &ar->arvifs, list)
>> > -           ath10k_mac_vif_beacon_cleanup(arvif);
>> > +           ath10k_mac_vif_beacon_free(arvif);
>> >     spin_unlock_bh(&ar->data_lock);
>> > +   list_for_each_entry(arvif, &ar->arvifs, list) {
>> > +           if (!arvif->beacon_buf)
>> > +                   continue;
>> > +           dma_free_coherent(ar->dev, IEEE80211_MAX_FRAME_LEN,
>> > +                             arvif->beacon_buf, arvif->beacon_paddr);
>> > +           arvif->beacon_buf = NULL;
>> > +   }
>> >  }
>>
>> Until now we have protected arvif->beacon_buf with data_lock. How do we
>> know that this is safe to do without taking data_lock?
>>
> As said, spin_lock can not be used for dma_free_coherent.
> arvif->beacon_buf is already protected by conf_mutex. At this state
> in ath10k_halt path, no one can access beacon_buf. So mutex lock itself
> is sufficient.

beacon_buf is protected by conf_mutex implicitly. It wasn't the main
intent. It is protected with data_lock spinlock.

Do not trust the device - if there's a spurious SWBA event while
ath10k_remove_interface() is running you could end up with invalid
memory access.

It might be acceptable to drop the spinlock for ath10k_halt() since
the device is guaranteed to be stopped at that point (effectively
reset) though.

Anyway I'm hoping this bug can be fixed with the gfp flag.


Michał