diff for duplicates of <1486383699520.26295@qti.qualcomm.com> diff --git a/a/1.txt b/N1/1.txt index b8e1262..19a5004 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,129 +1,143 @@ -Hi, - -even with the below patch applied ? -https://patchwork.kernel.org/patch/9452265/ - -regards -shafi -________________________________________ -From: Michael Ney <neym@vorklift.com> -Sent: 06 February 2017 17:46 -To: Mohammed Shafi Shajakhan -Cc: Valo, Kalle; linux-wireless@vger.kernel.org; ath10k@lists.infradead.org; Shajakhan, Mohammed Shafi (Mohammed Shafi) -Subject: Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails - -Symmetry is still broken on firmware crash (at least with 6174). ath10k_pci_hif_stop gets called twice, once from the driver restart (warm restart) and once from ieee80211 start (cold restart), resulting in napi_synchrionize/napi_disable getting called twice and sticking the driver in an infinite wait loop (napi_synchronize waits until NAPI_STATE_SCHED is off, while napi_disable leaves NAPI_STATE_SCHED to on when leaving). - - -> On Feb 6, 2017, at 5:04 AM, Mohammed Shafi Shajakhan <mohammed@codeaurora.org> wrote: -> -> Hi Kalle, -> -> the change suggested by you helps, and the device probe, scan -> is successful as well. Still good to have this change part of your -> basic sanity and regression testing ! -> -> regards, -> shafi -> -> On Wed, Jan 25, 2017 at 01:46:28PM +0000, Valo, Kalle wrote: ->> Kalle Valo <kvalo@qca.qualcomm.com> writes: ->> ->>> Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> writes: ->>> ->>>> From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> ->>>> ->>>> This fixes the below crash when ath10k probe firmware fails, ->>>> NAPI polling tries to access a rx ring resource which was never ->>>> allocated, fix this by disabling NAPI right away once the probe ->>>> firmware fails by calling 'ath10k_hif_stop'. Its good to note ->>>> that the error is never propogated to 'ath10k_pci_probe' when ->>>> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup ->>>> PCI related things seems to be ok ->>>> ->>>> BUG: unable to handle kernel NULL pointer dereference at (null) ->>>> IP: __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core] ->>>> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core] ->>>> ->>>> Call Trace: ->>>> ->>>> [<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90 ->>>> [ath10k_core] ->>>> [<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0 ->>>> [ath10k_core] ->>>> [<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80 ->>>> [<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150 ->>>> [<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci] ->>>> [<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci] ->>>> [<ffffffff817863af>] net_rx_action+0x20f/0x370 ->>>> ->>>> Reported-by: Ben Greear <greearb@candelatech.com> ->>>> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support") ->>>> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> ->>> ->>> Is there an easy way to reproduce this bug? I don't see it on my x86 ->>> laptop with qca988x and I call rmmod all the time. I would like to test ->>> this myself. ->>> ->>>> --- a/drivers/net/wireless/ath/ath10k/core.c ->>>> +++ b/drivers/net/wireless/ath/ath10k/core.c ->>>> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar) ->>>> ath10k_core_free_firmware_files(ar); ->>>> ->>>> err_power_down: ->>>> + ath10k_hif_stop(ar); ->>>> ath10k_hif_power_down(ar); ->>>> ->>>> return ret; ->>> ->>> This breaks the symmetry, we should not be calling ath10k_hif_stop() if ->>> we haven't called ath10k_hif_start() from the same function. This can ->>> just create a bigger mess later, for example with other bus support like ->>> sdio or usb. In theory it should enough that we call ->>> ath10k_hif_power_down() and pci.c does the rest correctly "behind the ->>> scenes". ->>> ->>> I investigated this a bit and I think the real cause is that we call ->>> napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from ->>> ath10k_pci_hif_stop(). Does anyone remember why? ->>> ->>> I was expecting that we would call napi_enable()/napi_disable() either ->>> in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not ->>> mixed like it's currently. ->> ->> So below is something I was thinking of, now napi_enable() is called ->> from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would ->> that work? ->> ->> --- a/drivers/net/wireless/ath/ath10k/pci.c ->> +++ b/drivers/net/wireless/ath/ath10k/pci.c ->> @@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar) ->> ->> ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n"); ->> ->> + napi_enable(&ar->napi); ->> + ->> ath10k_pci_irq_enable(ar); ->> ath10k_pci_rx_post(ar); ->> ->> @@ -2532,7 +2534,6 @@ 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; ->> ->> -- ->> Kalle Valo -> -> _______________________________________________ -> ath10k mailing list -> ath10k@lists.infradead.org -> http://lists.infradead.org/mailman/listinfo/ath10k - - -_______________________________________________ -ath10k mailing list -ath10k@lists.infradead.org -http://lists.infradead.org/mailman/listinfo/ath10k +Hi,=0A= +=0A= +even with the below patch applied ?=0A= +https://patchwork.kernel.org/patch/9452265/=0A= +=0A= +regards=0A= +shafi=0A= +________________________________________=0A= +From: Michael Ney <neym@vorklift.com>=0A= +Sent: 06 February 2017 17:46=0A= +To: Mohammed Shafi Shajakhan=0A= +Cc: Valo, Kalle; linux-wireless@vger.kernel.org; ath10k@lists.infradead.org= +; Shajakhan, Mohammed Shafi (Mohammed Shafi)=0A= +Subject: Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware = +fails=0A= +=0A= +Symmetry is still broken on firmware crash (at least with 6174). ath10k_pci= +_hif_stop gets called twice, once from the driver restart (warm restart) an= +d once from ieee80211 start (cold restart), resulting in napi_synchrionize/= +napi_disable getting called twice and sticking the driver in an infinite wa= +it loop (napi_synchronize waits until NAPI_STATE_SCHED is off, while napi_d= +isable leaves NAPI_STATE_SCHED to on when leaving).=0A= +=0A= +=0A= +> On Feb 6, 2017, at 5:04 AM, Mohammed Shafi Shajakhan <mohammed@codeaurora= +.org> wrote:=0A= +>=0A= +> Hi Kalle,=0A= +>=0A= +> the change suggested by you helps, and the device probe, scan=0A= +> is successful as well. Still good to have this change part of your=0A= +> basic sanity and regression testing !=0A= +>=0A= +> regards,=0A= +> shafi=0A= +>=0A= +> On Wed, Jan 25, 2017 at 01:46:28PM +0000, Valo, Kalle wrote:=0A= +>> Kalle Valo <kvalo@qca.qualcomm.com> writes:=0A= +>>=0A= +>>> Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> writes:=0A= +>>>=0A= +>>>> From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>=0A= +>>>>=0A= +>>>> This fixes the below crash when ath10k probe firmware fails,=0A= +>>>> NAPI polling tries to access a rx ring resource which was never=0A= +>>>> allocated, fix this by disabling NAPI right away once the probe=0A= +>>>> firmware fails by calling 'ath10k_hif_stop'. Its good to note=0A= +>>>> that the error is never propogated to 'ath10k_pci_probe' when=0A= +>>>> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup=0A= +>>>> PCI related things seems to be ok=0A= +>>>>=0A= +>>>> BUG: unable to handle kernel NULL pointer dereference at (null)=0A= +>>>> IP: __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]=0A= +>>>> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]=0A= +>>>>=0A= +>>>> Call Trace:=0A= +>>>>=0A= +>>>> [<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90=0A= +>>>> [ath10k_core]=0A= +>>>> [<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0=0A= +>>>> [ath10k_core]=0A= +>>>> [<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80=0A= +>>>> [<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150=0A= +>>>> [<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]=0A= +>>>> [<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]=0A= +>>>> [<ffffffff817863af>] net_rx_action+0x20f/0x370=0A= +>>>>=0A= +>>>> Reported-by: Ben Greear <greearb@candelatech.com>=0A= +>>>> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")=0A= +>>>> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>=0A= +>>>=0A= +>>> Is there an easy way to reproduce this bug? I don't see it on my x86=0A= +>>> laptop with qca988x and I call rmmod all the time. I would like to test= +=0A= +>>> this myself.=0A= +>>>=0A= +>>>> --- a/drivers/net/wireless/ath/ath10k/core.c=0A= +>>>> +++ b/drivers/net/wireless/ath/ath10k/core.c=0A= +>>>> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *a= +r)=0A= +>>>> ath10k_core_free_firmware_files(ar);=0A= +>>>>=0A= +>>>> err_power_down:=0A= +>>>> + ath10k_hif_stop(ar);=0A= +>>>> ath10k_hif_power_down(ar);=0A= +>>>>=0A= +>>>> return ret;=0A= +>>>=0A= +>>> This breaks the symmetry, we should not be calling ath10k_hif_stop() if= +=0A= +>>> we haven't called ath10k_hif_start() from the same function. This can= +=0A= +>>> just create a bigger mess later, for example with other bus support lik= +e=0A= +>>> sdio or usb. In theory it should enough that we call=0A= +>>> ath10k_hif_power_down() and pci.c does the rest correctly "behind the= +=0A= +>>> scenes".=0A= +>>>=0A= +>>> I investigated this a bit and I think the real cause is that we call=0A= +>>> napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from=0A= +>>> ath10k_pci_hif_stop(). Does anyone remember why?=0A= +>>>=0A= +>>> I was expecting that we would call napi_enable()/napi_disable() either= +=0A= +>>> in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not=0A= +>>> mixed like it's currently.=0A= +>>=0A= +>> So below is something I was thinking of, now napi_enable() is called=0A= +>> from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would= +=0A= +>> that work?=0A= +>>=0A= +>> --- a/drivers/net/wireless/ath/ath10k/pci.c=0A= +>> +++ b/drivers/net/wireless/ath/ath10k/pci.c=0A= +>> @@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar)= +=0A= +>>=0A= +>> ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");=0A= +>>=0A= +>> + napi_enable(&ar->napi);=0A= +>> +=0A= +>> ath10k_pci_irq_enable(ar);=0A= +>> ath10k_pci_rx_post(ar);=0A= +>>=0A= +>> @@ -2532,7 +2534,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *= +ar)=0A= +>> ath10k_err(ar, "could not wake up target CPU: %d\n", ret);= +=0A= +>> goto err_ce;=0A= +>> }=0A= +>> - napi_enable(&ar->napi);=0A= +>>=0A= +>> return 0;=0A= +>>=0A= +>> --=0A= +>> Kalle Valo=0A= +>=0A= +> _______________________________________________=0A= +> ath10k mailing list=0A= +> ath10k@lists.infradead.org=0A= +> http://lists.infradead.org/mailman/listinfo/ath10k=0A= +=0A= diff --git a/a/content_digest b/N1/content_digest index 0073999..9c25600 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -14,134 +14,148 @@ " ath10k@lists.infradead.org <ath10k@lists.infradead.org>\0" "\00:1\0" "b\0" - "Hi,\n" - "\n" - "even with the below patch applied ?\n" - "https://patchwork.kernel.org/patch/9452265/\n" - "\n" - "regards\n" - "shafi\n" - "________________________________________\n" - "From: Michael Ney <neym@vorklift.com>\n" - "Sent: 06 February 2017 17:46\n" - "To: Mohammed Shafi Shajakhan\n" - "Cc: Valo, Kalle; linux-wireless@vger.kernel.org; ath10k@lists.infradead.org; Shajakhan, Mohammed Shafi (Mohammed Shafi)\n" - "Subject: Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails\n" - "\n" - "Symmetry is still broken on firmware crash (at least with 6174). ath10k_pci_hif_stop gets called twice, once from the driver restart (warm restart) and once from ieee80211 start (cold restart), resulting in napi_synchrionize/napi_disable getting called twice and sticking the driver in an infinite wait loop (napi_synchronize waits until NAPI_STATE_SCHED is off, while napi_disable leaves NAPI_STATE_SCHED to on when leaving).\n" - "\n" - "\n" - "> On Feb 6, 2017, at 5:04 AM, Mohammed Shafi Shajakhan <mohammed@codeaurora.org> wrote:\n" - ">\n" - "> Hi Kalle,\n" - ">\n" - "> the change suggested by you helps, and the device probe, scan\n" - "> is successful as well. Still good to have this change part of your\n" - "> basic sanity and regression testing !\n" - ">\n" - "> regards,\n" - "> shafi\n" - ">\n" - "> On Wed, Jan 25, 2017 at 01:46:28PM +0000, Valo, Kalle wrote:\n" - ">> Kalle Valo <kvalo@qca.qualcomm.com> writes:\n" - ">>\n" - ">>> Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> writes:\n" - ">>>\n" - ">>>> From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>\n" - ">>>>\n" - ">>>> This fixes the below crash when ath10k probe firmware fails,\n" - ">>>> NAPI polling tries to access a rx ring resource which was never\n" - ">>>> allocated, fix this by disabling NAPI right away once the probe\n" - ">>>> firmware fails by calling 'ath10k_hif_stop'. Its good to note\n" - ">>>> that the error is never propogated to 'ath10k_pci_probe' when\n" - ">>>> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup\n" - ">>>> PCI related things seems to be ok\n" - ">>>>\n" - ">>>> BUG: unable to handle kernel NULL pointer dereference at (null)\n" - ">>>> IP: __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]\n" - ">>>> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]\n" - ">>>>\n" - ">>>> Call Trace:\n" - ">>>>\n" - ">>>> [<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90\n" - ">>>> [ath10k_core]\n" - ">>>> [<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0\n" - ">>>> [ath10k_core]\n" - ">>>> [<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80\n" - ">>>> [<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150\n" - ">>>> [<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]\n" - ">>>> [<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]\n" - ">>>> [<ffffffff817863af>] net_rx_action+0x20f/0x370\n" - ">>>>\n" - ">>>> Reported-by: Ben Greear <greearb@candelatech.com>\n" - ">>>> Fixes: 3c97f5de1f28 (\"ath10k: implement NAPI support\")\n" - ">>>> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>\n" - ">>>\n" - ">>> Is there an easy way to reproduce this bug? I don't see it on my x86\n" - ">>> laptop with qca988x and I call rmmod all the time. I would like to test\n" - ">>> this myself.\n" - ">>>\n" - ">>>> --- a/drivers/net/wireless/ath/ath10k/core.c\n" - ">>>> +++ b/drivers/net/wireless/ath/ath10k/core.c\n" - ">>>> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)\n" - ">>>> ath10k_core_free_firmware_files(ar);\n" - ">>>>\n" - ">>>> err_power_down:\n" - ">>>> + ath10k_hif_stop(ar);\n" - ">>>> ath10k_hif_power_down(ar);\n" - ">>>>\n" - ">>>> return ret;\n" - ">>>\n" - ">>> This breaks the symmetry, we should not be calling ath10k_hif_stop() if\n" - ">>> we haven't called ath10k_hif_start() from the same function. This can\n" - ">>> just create a bigger mess later, for example with other bus support like\n" - ">>> sdio or usb. In theory it should enough that we call\n" - ">>> ath10k_hif_power_down() and pci.c does the rest correctly \"behind the\n" - ">>> scenes\".\n" - ">>>\n" - ">>> I investigated this a bit and I think the real cause is that we call\n" - ">>> napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from\n" - ">>> ath10k_pci_hif_stop(). Does anyone remember why?\n" - ">>>\n" - ">>> I was expecting that we would call napi_enable()/napi_disable() either\n" - ">>> in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not\n" - ">>> mixed like it's currently.\n" - ">>\n" - ">> So below is something I was thinking of, now napi_enable() is called\n" - ">> from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would\n" - ">> that work?\n" - ">>\n" - ">> --- a/drivers/net/wireless/ath/ath10k/pci.c\n" - ">> +++ b/drivers/net/wireless/ath/ath10k/pci.c\n" - ">> @@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar)\n" - ">>\n" - ">> ath10k_dbg(ar, ATH10K_DBG_BOOT, \"boot hif start\\n\");\n" - ">>\n" - ">> + napi_enable(&ar->napi);\n" - ">> +\n" - ">> ath10k_pci_irq_enable(ar);\n" - ">> ath10k_pci_rx_post(ar);\n" - ">>\n" - ">> @@ -2532,7 +2534,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)\n" - ">> ath10k_err(ar, \"could not wake up target CPU: %d\\n\", ret);\n" - ">> goto err_ce;\n" - ">> }\n" - ">> - napi_enable(&ar->napi);\n" - ">>\n" - ">> return 0;\n" - ">>\n" - ">> --\n" - ">> Kalle Valo\n" - ">\n" - "> _______________________________________________\n" - "> ath10k mailing list\n" - "> ath10k@lists.infradead.org\n" - "> http://lists.infradead.org/mailman/listinfo/ath10k\n" - "\n" - "\n" - "_______________________________________________\n" - "ath10k mailing list\n" - "ath10k@lists.infradead.org\n" - http://lists.infradead.org/mailman/listinfo/ath10k + "Hi,=0A=\n" + "=0A=\n" + "even with the below patch applied ?=0A=\n" + "https://patchwork.kernel.org/patch/9452265/=0A=\n" + "=0A=\n" + "regards=0A=\n" + "shafi=0A=\n" + "________________________________________=0A=\n" + "From: Michael Ney <neym@vorklift.com>=0A=\n" + "Sent: 06 February 2017 17:46=0A=\n" + "To: Mohammed Shafi Shajakhan=0A=\n" + "Cc: Valo, Kalle; linux-wireless@vger.kernel.org; ath10k@lists.infradead.org=\n" + "; Shajakhan, Mohammed Shafi (Mohammed Shafi)=0A=\n" + "Subject: Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware =\n" + "fails=0A=\n" + "=0A=\n" + "Symmetry is still broken on firmware crash (at least with 6174). ath10k_pci=\n" + "_hif_stop gets called twice, once from the driver restart (warm restart) an=\n" + "d once from ieee80211 start (cold restart), resulting in napi_synchrionize/=\n" + "napi_disable getting called twice and sticking the driver in an infinite wa=\n" + "it loop (napi_synchronize waits until NAPI_STATE_SCHED is off, while napi_d=\n" + "isable leaves NAPI_STATE_SCHED to on when leaving).=0A=\n" + "=0A=\n" + "=0A=\n" + "> On Feb 6, 2017, at 5:04 AM, Mohammed Shafi Shajakhan <mohammed@codeaurora=\n" + ".org> wrote:=0A=\n" + ">=0A=\n" + "> Hi Kalle,=0A=\n" + ">=0A=\n" + "> the change suggested by you helps, and the device probe, scan=0A=\n" + "> is successful as well. Still good to have this change part of your=0A=\n" + "> basic sanity and regression testing !=0A=\n" + ">=0A=\n" + "> regards,=0A=\n" + "> shafi=0A=\n" + ">=0A=\n" + "> On Wed, Jan 25, 2017 at 01:46:28PM +0000, Valo, Kalle wrote:=0A=\n" + ">> Kalle Valo <kvalo@qca.qualcomm.com> writes:=0A=\n" + ">>=0A=\n" + ">>> Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> writes:=0A=\n" + ">>>=0A=\n" + ">>>> From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>=0A=\n" + ">>>>=0A=\n" + ">>>> This fixes the below crash when ath10k probe firmware fails,=0A=\n" + ">>>> NAPI polling tries to access a rx ring resource which was never=0A=\n" + ">>>> allocated, fix this by disabling NAPI right away once the probe=0A=\n" + ">>>> firmware fails by calling 'ath10k_hif_stop'. Its good to note=0A=\n" + ">>>> that the error is never propogated to 'ath10k_pci_probe' when=0A=\n" + ">>>> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup=0A=\n" + ">>>> PCI related things seems to be ok=0A=\n" + ">>>>=0A=\n" + ">>>> BUG: unable to handle kernel NULL pointer dereference at (null)=0A=\n" + ">>>> IP: __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]=0A=\n" + ">>>> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]=0A=\n" + ">>>>=0A=\n" + ">>>> Call Trace:=0A=\n" + ">>>>=0A=\n" + ">>>> [<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90=0A=\n" + ">>>> [ath10k_core]=0A=\n" + ">>>> [<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0=0A=\n" + ">>>> [ath10k_core]=0A=\n" + ">>>> [<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80=0A=\n" + ">>>> [<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150=0A=\n" + ">>>> [<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]=0A=\n" + ">>>> [<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]=0A=\n" + ">>>> [<ffffffff817863af>] net_rx_action+0x20f/0x370=0A=\n" + ">>>>=0A=\n" + ">>>> Reported-by: Ben Greear <greearb@candelatech.com>=0A=\n" + ">>>> Fixes: 3c97f5de1f28 (\"ath10k: implement NAPI support\")=0A=\n" + ">>>> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>=0A=\n" + ">>>=0A=\n" + ">>> Is there an easy way to reproduce this bug? I don't see it on my x86=0A=\n" + ">>> laptop with qca988x and I call rmmod all the time. I would like to test=\n" + "=0A=\n" + ">>> this myself.=0A=\n" + ">>>=0A=\n" + ">>>> --- a/drivers/net/wireless/ath/ath10k/core.c=0A=\n" + ">>>> +++ b/drivers/net/wireless/ath/ath10k/core.c=0A=\n" + ">>>> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *a=\n" + "r)=0A=\n" + ">>>> ath10k_core_free_firmware_files(ar);=0A=\n" + ">>>>=0A=\n" + ">>>> err_power_down:=0A=\n" + ">>>> + ath10k_hif_stop(ar);=0A=\n" + ">>>> ath10k_hif_power_down(ar);=0A=\n" + ">>>>=0A=\n" + ">>>> return ret;=0A=\n" + ">>>=0A=\n" + ">>> This breaks the symmetry, we should not be calling ath10k_hif_stop() if=\n" + "=0A=\n" + ">>> we haven't called ath10k_hif_start() from the same function. This can=\n" + "=0A=\n" + ">>> just create a bigger mess later, for example with other bus support lik=\n" + "e=0A=\n" + ">>> sdio or usb. In theory it should enough that we call=0A=\n" + ">>> ath10k_hif_power_down() and pci.c does the rest correctly \"behind the=\n" + "=0A=\n" + ">>> scenes\".=0A=\n" + ">>>=0A=\n" + ">>> I investigated this a bit and I think the real cause is that we call=0A=\n" + ">>> napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from=0A=\n" + ">>> ath10k_pci_hif_stop(). Does anyone remember why?=0A=\n" + ">>>=0A=\n" + ">>> I was expecting that we would call napi_enable()/napi_disable() either=\n" + "=0A=\n" + ">>> in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not=0A=\n" + ">>> mixed like it's currently.=0A=\n" + ">>=0A=\n" + ">> So below is something I was thinking of, now napi_enable() is called=0A=\n" + ">> from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would=\n" + "=0A=\n" + ">> that work?=0A=\n" + ">>=0A=\n" + ">> --- a/drivers/net/wireless/ath/ath10k/pci.c=0A=\n" + ">> +++ b/drivers/net/wireless/ath/ath10k/pci.c=0A=\n" + ">> @@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar)=\n" + "=0A=\n" + ">>=0A=\n" + ">> ath10k_dbg(ar, ATH10K_DBG_BOOT, \"boot hif start\\n\");=0A=\n" + ">>=0A=\n" + ">> + napi_enable(&ar->napi);=0A=\n" + ">> +=0A=\n" + ">> ath10k_pci_irq_enable(ar);=0A=\n" + ">> ath10k_pci_rx_post(ar);=0A=\n" + ">>=0A=\n" + ">> @@ -2532,7 +2534,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *=\n" + "ar)=0A=\n" + ">> ath10k_err(ar, \"could not wake up target CPU: %d\\n\", ret);=\n" + "=0A=\n" + ">> goto err_ce;=0A=\n" + ">> }=0A=\n" + ">> - napi_enable(&ar->napi);=0A=\n" + ">>=0A=\n" + ">> return 0;=0A=\n" + ">>=0A=\n" + ">> --=0A=\n" + ">> Kalle Valo=0A=\n" + ">=0A=\n" + "> _______________________________________________=0A=\n" + "> ath10k mailing list=0A=\n" + "> ath10k@lists.infradead.org=0A=\n" + "> http://lists.infradead.org/mailman/listinfo/ath10k=0A=\n" + =0A= -6e3ea4565ec991c8847f37c3012b09ba78a292a9e031eb6a2b589969eb5e7783 +dc994ec1426f72319335b649675baf9cd4f04474cd609174a884cdc85ad98a80
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.