Re: [PATCH] ath10k: Fix crash due to tasklet race.

[Kalle Valo <kvalo@qca.qualcomm.com>]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 29 October 2013 22:38,  <greearb@candelatech.com> wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> The tasklet can run after the rings have been cleaned up,
>> so check for NULL before de-referencing the ring.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/ce.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
>> index d243f28..dfcfda9 100644
>> --- a/drivers/net/wireless/ath/ath10k/ce.c
>> +++ b/drivers/net/wireless/ath/ath10k/ce.c
>> @@ -547,12 +547,18 @@ static int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
>>         struct ath10k_ce_ring *src_ring = ce_state->src_ring;
>>         u32 ctrl_addr = ce_state->ctrl_addr;
>>         struct ath10k *ar = ce_state->ar;
>> -       unsigned int nentries_mask = src_ring->nentries_mask;
>> -       unsigned int sw_index = src_ring->sw_index;
>> +       unsigned int nentries_mask;
>> +       unsigned int sw_index;
>>         struct ce_desc *sdesc, *sbase;
>>         unsigned int read_index;
>>         int ret;
>>
>> +       if (!src_ring)
>> +               return -EIO;
>
> I don't think this is a proper way to fix the problem. What should be
> done is initialization clean up and some reordering to prevent this
> from happening in the first place.

I agree with Michal, but as we don't have any better fix for this issue
I'm inclined to take the patch anyway. Maybe there just should be a
comment stating that it's an ugly workaround and a WARN_ON() to make
sure that we properly fix the interrupt initialisation.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k