Linux-Wireless Archive on lore.kernel.org
 help / Atom feed
* [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash
@ 2018-11-14  2:50 Wen Gong
  2018-11-14  7:48 ` Michał Kazior
  0 siblings, 1 reply; 5+ messages in thread
From: Wen Gong @ 2018-11-14  2:50 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

When test simulate firmware crash, it is easy to trigger error.
command:
echo soft > /sys/kernel/debug/ieee80211/phyxx/ath10k/simulate_fw_crash.

If input more than two times continuously, then it will have error.
Error message:
ath10k_pci 0000:02:00.0: failed to set vdev 1 RX wake policy: -108
ath10k_pci 0000:02:00.0: device is wedged, will not restart

It is because the state has not changed to ATH10K_STATE_ON immediately,
then it will have more than two simulate crash process running meanwhile,
and complete/wakeup some field twice, it destroy the normal recovery
process.

Tested with QCA6174 PCI with firmware
WLAN.RM.4.4.1-00109-QCARMSWPZ-1, but this will also affect QCA9377 PCI.
It's not a regression with new firmware releases.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/debug.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index ada29a4..dc8700b 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -569,8 +569,7 @@ static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
 
 	mutex_lock(&ar->conf_mutex);
 
-	if (ar->state != ATH10K_STATE_ON &&
-	    ar->state != ATH10K_STATE_RESTARTED) {
+	if (ar->state != ATH10K_STATE_ON) {
 		ret = -ENETDOWN;
 		goto exit;
 	}
-- 
1.9.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash
  2018-11-14  2:50 [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash Wen Gong
@ 2018-11-14  7:48 ` Michał Kazior
  2019-01-07  7:16   ` Wen Gong
  0 siblings, 1 reply; 5+ messages in thread
From: Michał Kazior @ 2018-11-14  7:48 UTC (permalink / raw)
  To: Wen Gong; +Cc: ath10k, linux-wireless

On Wed, 14 Nov 2018 at 03:51, Wen Gong <wgong@codeaurora.org> wrote:
>
> When test simulate firmware crash, it is easy to trigger error.
> command:
> echo soft > /sys/kernel/debug/ieee80211/phyxx/ath10k/simulate_fw_crash.
>
> If input more than two times continuously, then it will have error.
> Error message:
> ath10k_pci 0000:02:00.0: failed to set vdev 1 RX wake policy: -108
> ath10k_pci 0000:02:00.0: device is wedged, will not restart
>
> It is because the state has not changed to ATH10K_STATE_ON immediately,
> then it will have more than two simulate crash process running meanwhile,
> and complete/wakeup some field twice, it destroy the normal recovery
> process.

This was intended to allow testing not only firmware crash path (and
recovery) but also firmware crash while recovering from a firmware
crash.


Michał

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash
  2018-11-14  7:48 ` Michał Kazior
@ 2019-01-07  7:16   ` Wen Gong
  2019-01-07  8:35     ` Michał Kazior
  0 siblings, 1 reply; 5+ messages in thread
From: Wen Gong @ 2019-01-07  7:16 UTC (permalink / raw)
  To: Michał Kazior, Wen Gong; +Cc: linux-wireless, ath10k

> > It is because the state has not changed to ATH10K_STATE_ON
> > immediately, then it will have more than two simulate crash process
> > running meanwhile, and complete/wakeup some field twice, it destroy
> > the normal recovery process.
> 
> This was intended to allow testing not only firmware crash path (and
> recovery) but also firmware crash while recovering from a firmware crash.
> 
If firmware is recovering from crash, then simulate a new crash will trigger error.
So remove it.
> 
> Michał
> 
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash
  2019-01-07  7:16   ` Wen Gong
@ 2019-01-07  8:35     ` Michał Kazior
  2019-01-08  8:45       ` Wen Gong
  0 siblings, 1 reply; 5+ messages in thread
From: Michał Kazior @ 2019-01-07  8:35 UTC (permalink / raw)
  To: Wen Gong; +Cc: Wen Gong, linux-wireless, ath10k

On Mon, 7 Jan 2019 at 08:16, Wen Gong <wgong@qti.qualcomm.com> wrote:
>
> > > It is because the state has not changed to ATH10K_STATE_ON
> > > immediately, then it will have more than two simulate crash process
> > > running meanwhile, and complete/wakeup some field twice, it destroy
> > > the normal recovery process.
> >
> > This was intended to allow testing not only firmware crash path (and
> > recovery) but also firmware crash while recovering from a firmware crash.
> >
> If firmware is recovering from crash, then simulate a new crash will trigger error.
> So remove it.

That's actually a feature, not a bug. If firmware crashes while driver
is restarting after a crash then its likely going to fail again and
again causing a crash-restart loop which can affect system performance
and responsiveness. It's better to give up and let the system admin
take over.

If it's still bothering you then please consider a crash counter
threshold so that, e.g. after 5 crash-while-restarting it's going to
give up. However I doubt it's worth the effort. My experience tells me
firmware crashes during recovery are rarely, if at all, transient.

The simulated fw crash is not representative here. It's a mere tool to
test driver code.


Michał

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash
  2019-01-07  8:35     ` Michał Kazior
@ 2019-01-08  8:45       ` Wen Gong
  0 siblings, 0 replies; 5+ messages in thread
From: Wen Gong @ 2019-01-08  8:45 UTC (permalink / raw)
  To: Michał Kazior; +Cc: Wen Gong, linux-wireless, ath10k

> >
> > > > It is because the state has not changed to ATH10K_STATE_ON
> > > > immediately, then it will have more than two simulate crash
> > > > process running meanwhile, and complete/wakeup some field twice,
> > > > it destroy the normal recovery process.
> > >
> > > This was intended to allow testing not only firmware crash path (and
> > > recovery) but also firmware crash while recovering from a firmware crash.
> > >
> > If firmware is recovering from crash, then simulate a new crash will trigger
> error.
> > So remove it.
> 
> That's actually a feature, not a bug. If firmware crashes while driver is
> restarting after a crash then its likely going to fail again and again causing a
> crash-restart loop which can affect system performance and responsiveness.
> It's better to give up and let the system admin take over.
> 
> If it's still bothering you then please consider a crash counter threshold so
> that, e.g. after 5 crash-while-restarting it's going to give up. However I doubt
> it's worth the effort. My experience tells me firmware crashes during
> recovery are rarely, if at all, transient.
> 
> The simulated fw crash is not representative here. It's a mere tool to test
> driver code.

The simulated fw crash is only a tool for user to trigger fw crash with command,
This change's purpose is to disallow user to trigger fw crash if the fw is not in a
Normal state.

If the fw is in recovering state triggered by user's command or by fw, then it will
disallow user to run command to trigger fw crash again until fw become to a normal
State.

> 
> 
> Michał

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-14  2:50 [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash Wen Gong
2018-11-14  7:48 ` Michał Kazior
2019-01-07  7:16   ` Wen Gong
2019-01-07  8:35     ` Michał Kazior
2019-01-08  8:45       ` Wen Gong

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox