master-pending: ODEBUG: free active (active state 0) object type: timer_list hint: ath10k_htt_rx_ring_refill_retry+0x0/0x10 [ath10k_core]

Commit Message

Rajkumar Manoharan Aug. 24, 2016, 3:39 p.m. UTC

[...]
>>>>
>>>> [  277.244673] ------------[ cut here ]------------
>>>> [  277.244685] WARNING: CPU: 2 PID: 2162 at lib/debugobjects.c:263 debug_print_object+0x85/0xa0
>>>> [  277.244707] ODEBUG: free active (active state 0) object type: timer_list hint: ath10k_htt_rx_ring_refill_retry+0x0/0x10 [ath10k_core]
>>>>
>>>> I bisected this to:
>>>>
>>>> [v2] ath10k: implement NAPI support
>>>
>>> https://patchwork.kernel.org/patch/9240975/
>>>
>>> I'll reply to that patch for more info.
>>>
>> Oops.. my bad.. Sorry for the regression..Good that it is captured at
>> earliest. Thanks Kalle for reporting the issue.
>>
>> It seems during deinit, driver is not waiting for NAPI completion.
>> What debug options should be enabled to see this warning?
>
> I usually test with all sane[1] kernel debug infrastructure enabled. I have
> attached a kernel config I use with my x86 32-bit laptop.
> 
> [1] sane == not too spammy and still keeps the laptop barely usable
> 
Yes indeed. With your kernel config, I can able to repro the issue but machine is really slow ;)
Thanks a lot for sharing the config. It really helps.

>> If possible, could you please try below change?
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
>> index 33380fba5948..29fc9052e51a 100644
>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>> @@ -1759,6 +1759,7 @@ void ath10k_pci_ce_deinit(struct ath10k *ar)
>>
>>  void ath10k_pci_flush(struct ath10k *ar)
>>  {
>> +       napi_synchronize(&ar->napi);
>>         ath10k_pci_rx_retry_sync(ar);
>>         ath10k_pci_buffer_cleanup(ar);
>>  }
>
> Sorry, didn't help:
>
It seems napi synchronization alone is not sufficient. NAPI should be disabled. Need your help again. ;)
With below change WARNING & KASAN BUG is gone. Could you please help to verify below change?

ath10k mailing list
[e-mail]
http://lists.infradead.org/mailman/listinfo/ath10k

Comments

Kalle Valo Aug. 26, 2016, 5:43 p.m. UTC | #1

"Manoharan, Rajkumar" <[e-mail]> writes:

>>> If possible, could you please try below change?
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
>>> index 33380fba5948..29fc9052e51a 100644
>>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>>> @@ -1759,6 +1759,7 @@ void ath10k_pci_ce_deinit(struct ath10k *ar)
>>>
>>>  void ath10k_pci_flush(struct ath10k *ar)
>>>  {
>>> +       napi_synchronize(&ar->napi);
>>>         ath10k_pci_rx_retry_sync(ar);
>>>         ath10k_pci_buffer_cleanup(ar);
>>>  }
>>
>> Sorry, didn't help:
>>
> It seems napi synchronization alone is not sufficient. NAPI should be disabled. Need your help again. ;)
> With below change WARNING & KASAN BUG is gone. Could you please help to verify below change?
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index 6e21fa1478ea..04ff45deb173 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -1775,6 +1775,8 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
>          */
>         ath10k_pci_safe_chip_reset(ar);
>  
> +       napi_synchronize(&ar->napi);
> +       napi_disable(&ar->napi);
>         ath10k_pci_irq_disable(ar);
>         ath10k_pci_irq_sync(ar);
>         ath10k_pci_flush(ar);
> @@ -2531,6 +2533,7 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
>                 ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
>                 goto err_ce;
>         }
> +       napi_enable(&ar->napi);
>  
>         return 0;
>  
> @@ -2878,7 +2881,6 @@ void ath10k_pci_init_napi(struct ath10k *ar)
>  {
>         netif_napi_add(&ar->napi_dev, &ar->napi, ath10k_pci_napi_poll,
>                        ATH10K_NAPI_BUDGET);
> -       napi_enable(&ar->napi);
>  }

The patch was corrupted but I did the same changes manually. And now I
can't get it to crash anymore, earlier that was really easy.

Patch

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 6e21fa1478ea..04ff45deb173 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1775,6 +1775,8 @@  static void ath10k_pci_hif_stop(struct ath10k *ar)
         */
        ath10k_pci_safe_chip_reset(ar);
 
+       napi_synchronize(&ar->napi);
+       napi_disable(&ar->napi);
        ath10k_pci_irq_disable(ar);
        ath10k_pci_irq_sync(ar);
        ath10k_pci_flush(ar);
@@ -2531,6 +2533,7 @@  static int ath10k_pci_hif_power_up(struct ath10k *ar)
                ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
                goto err_ce;
        }
+       napi_enable(&ar->napi);
 
        return 0;
 
@@ -2878,7 +2881,6 @@  void ath10k_pci_init_napi(struct ath10k *ar)
 {
        netif_napi_add(&ar->napi_dev, &ar->napi, ath10k_pci_napi_poll,
                       ATH10K_NAPI_BUDGET);
-       napi_enable(&ar->napi);
 }
 
-Rajkumar
_______________________________________________