* [PATCH] ath10k: Fix crash due to tasklet race.
@ 2013-10-29 21:38 greearb
2013-10-30 9:03 ` Michal Kazior
2013-11-06 13:06 ` Kalle Valo
0 siblings, 2 replies; 6+ messages in thread
From: greearb @ 2013-10-29 21:38 UTC (permalink / raw)
To: ath10k; +Cc: kvalo, Ben Greear
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;
+
+ nentries_mask = src_ring->nentries_mask;
+ sw_index = src_ring->sw_index;
+
if (src_ring->hw_index == sw_index) {
/*
* The SW completion index has caught up with the cached
--
1.7.11.7
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ath10k: Fix crash due to tasklet race.
2013-10-29 21:38 [PATCH] ath10k: Fix crash due to tasklet race greearb
@ 2013-10-30 9:03 ` Michal Kazior
2013-11-06 11:50 ` Kalle Valo
2013-11-06 13:06 ` Kalle Valo
1 sibling, 1 reply; 6+ messages in thread
From: Michal Kazior @ 2013-10-30 9:03 UTC (permalink / raw)
To: Ben Greear; +Cc: Kalle Valo, ath10k
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;
> +
> + nentries_mask = src_ring->nentries_mask;
> + sw_index = src_ring->sw_index;
> +
> if (src_ring->hw_index == sw_index) {
> /*
> * The SW completion index has caught up with the cached
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.
Michał
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ath10k: Fix crash due to tasklet race.
2013-10-30 9:03 ` Michal Kazior
@ 2013-11-06 11:50 ` Kalle Valo
2013-11-06 11:54 ` Michal Kazior
0 siblings, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2013-11-06 11:50 UTC (permalink / raw)
To: Michal Kazior; +Cc: Ben Greear, ath10k
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath10k: Fix crash due to tasklet race.
2013-11-06 11:50 ` Kalle Valo
@ 2013-11-06 11:54 ` Michal Kazior
0 siblings, 0 replies; 6+ messages in thread
From: Michal Kazior @ 2013-11-06 11:54 UTC (permalink / raw)
To: Kalle Valo; +Cc: Ben Greear, ath10k
On 6 November 2013 12:50, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> 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.
This issue alone should actually be fixed in my pci fixes RFT.
Michał
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath10k: Fix crash due to tasklet race.
2013-10-29 21:38 [PATCH] ath10k: Fix crash due to tasklet race greearb
2013-10-30 9:03 ` Michal Kazior
@ 2013-11-06 13:06 ` Kalle Valo
2013-11-06 17:00 ` Ben Greear
1 sibling, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2013-11-06 13:06 UTC (permalink / raw)
To: greearb; +Cc: ath10k
greearb@candelatech.com writes:
> 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>
I'll drop this as Michal's patch has a proper fix for this issue.
--
Kalle Valo
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath10k: Fix crash due to tasklet race.
2013-11-06 13:06 ` Kalle Valo
@ 2013-11-06 17:00 ` Ben Greear
0 siblings, 0 replies; 6+ messages in thread
From: Ben Greear @ 2013-11-06 17:00 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k
On 11/06/2013 05:06 AM, Kalle Valo wrote:
> greearb@candelatech.com writes:
>
>> 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>
>
> I'll drop this as Michal's patch has a proper fix for this issue.
Ok, that sounds fine. I'll rebase when you push his patches and
drop my patch for continued testing.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-06 17:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-29 21:38 [PATCH] ath10k: Fix crash due to tasklet race greearb
2013-10-30 9:03 ` Michal Kazior
2013-11-06 11:50 ` Kalle Valo
2013-11-06 11:54 ` Michal Kazior
2013-11-06 13:06 ` Kalle Valo
2013-11-06 17:00 ` Ben Greear
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.