From: Luis Chamberlain <mcgrof@kernel.org> To: Johannes Berg <johannes@sipsolutions.net> Cc: Steve deRosier <derosier@gmail.com>, Ben Greear <greearb@candelatech.com>, jeyu@kernel.org, akpm@linux-foundation.org, arnd@arndb.de, rostedt@goodmis.org, mingo@redhat.com, aquini@redhat.com, cai@lca.pw, dyoung@redhat.com, bhe@redhat.com, peterz@infradead.org, tglx@linutronix.de, gpiccoli@canonical.com, pmladek@suse.com, Takashi Iwai <tiwai@suse.de>, schlad@suse.de, andriy.shevchenko@linux.intel.com, keescook@chromium.org, daniel.vetter@ffwll.ch, will@kernel.org, mchehab+samsung@kernel.org, Kalle Valo <kvalo@codeaurora.org>, "David S. Miller" <davem@davemloft.net>, Network Development <netdev@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, linux-wireless <linux-wireless@vger.kernel.org>, ath10k@lists.infradead.org Subject: Re: [PATCH v2 12/15] ath10k: use new module_firmware_crashed() Date: Mon, 18 May 2020 21:18:09 +0000 [thread overview] Message-ID: <20200518211809.GQ11244@42.do-not-panic.com> (raw) In-Reply-To: <bb0b9a2da99c16a28c1dbee93d08abfa2aecdc8b.camel@sipsolutions.net> On Mon, May 18, 2020 at 10:07:49PM +0200, Johannes Berg wrote: > On Mon, 2020-05-18 at 19:59 +0000, Luis Chamberlain wrote: > > > > Err, no. Those two are most definitely related. Have you looked at (most > > > or some or whatever) staging drivers recently? Those contain all kinds > > > of garbage that might do whatever with your kernel. > > > > No, I stay away :) > > :) > > > > That's all fine, I just don't think it's appropriate to pretend that > > > your kernel is now 'tainted' (think about the meaning of that word) when > > > the firmware of some random device crashed. > > > > If the firmware crash *does* require driver remove / addition again, > > or a reboot, would you think that this is a situation that merits a taint? > > Not really. In my experience, that's more likely a hardware issue (card > not properly seated, for example) that a bus reset happens to "fix". > > > > It's pretty clear, but even then, first of all I doubt this is the case > > > for many of the places that you've sprinkled the annotation on, > > > > We can remove it, for this driver I can vouch for its location as it did > > reach a state where I required a reboot. And its not the first time this > > has happened. This got me thinking about the bigger picture of the lack > > of proper way to address these cases in the kernel, and how the user is > > left dumbfounded. > > Fair, so the driver is still broken wrt. recovery here. I still don't > think that's a situation where e.g. the system should say "hey you have > a taint here, if your graphics go bad now you should not report that > bug" (which is effectively what the single taint bit does). But again, let's think about the generic type of issue, and the unexpected type of state that can be reached. The circumstance here *does* lead to a case which is not recoverable. Now, consider how many cases in the kernel where similar situations can happen and leave the device or driver in a non-functional state. > > > and secondly it actually hides useful information. > > > > What is it hiding? > > Most importantly, which device crashed. Secondarily I'd say how many > times (*). The device is implied by the module, the taint is applied to both. If you had multiple devices, however, yes, it would not be possible to distinguish from the taint which exact device it happened on. So the only thing *generic* which would be left out is count. > The information "firmware crashed" is really only useful in relation to > the device. If you have to reboot to get a functional network again then the device is quite useless for many people, regardless of which device that happened on. But from a support perspective a sysfs interface which provides a tiny bit more generic information indeed provides more value than a taint. Luis
WARNING: multiple messages have this Message-ID (diff)
From: Luis Chamberlain <mcgrof@kernel.org> To: Johannes Berg <johannes@sipsolutions.net> Cc: linux-wireless <linux-wireless@vger.kernel.org>, aquini@redhat.com, peterz@infradead.org, daniel.vetter@ffwll.ch, mchehab+samsung@kernel.org, will@kernel.org, Ben Greear <greearb@candelatech.com>, bhe@redhat.com, ath10k@lists.infradead.org, Steve deRosier <derosier@gmail.com>, Takashi Iwai <tiwai@suse.de>, mingo@redhat.com, dyoung@redhat.com, pmladek@suse.com, keescook@chromium.org, arnd@arndb.de, gpiccoli@canonical.com, rostedt@goodmis.org, cai@lca.pw, tglx@linutronix.de, andriy.shevchenko@linux.intel.com, Kalle Valo <kvalo@codeaurora.org>, Network Development <netdev@vger.kernel.org>, schlad@suse.de, LKML <linux-kernel@vger.kernel.org>, jeyu@kernel.org, akpm@linux-foundation.org, "David S. Miller" <davem@davemloft.net> Subject: Re: [PATCH v2 12/15] ath10k: use new module_firmware_crashed() Date: Mon, 18 May 2020 21:18:09 +0000 [thread overview] Message-ID: <20200518211809.GQ11244@42.do-not-panic.com> (raw) In-Reply-To: <bb0b9a2da99c16a28c1dbee93d08abfa2aecdc8b.camel@sipsolutions.net> On Mon, May 18, 2020 at 10:07:49PM +0200, Johannes Berg wrote: > On Mon, 2020-05-18 at 19:59 +0000, Luis Chamberlain wrote: > > > > Err, no. Those two are most definitely related. Have you looked at (most > > > or some or whatever) staging drivers recently? Those contain all kinds > > > of garbage that might do whatever with your kernel. > > > > No, I stay away :) > > :) > > > > That's all fine, I just don't think it's appropriate to pretend that > > > your kernel is now 'tainted' (think about the meaning of that word) when > > > the firmware of some random device crashed. > > > > If the firmware crash *does* require driver remove / addition again, > > or a reboot, would you think that this is a situation that merits a taint? > > Not really. In my experience, that's more likely a hardware issue (card > not properly seated, for example) that a bus reset happens to "fix". > > > > It's pretty clear, but even then, first of all I doubt this is the case > > > for many of the places that you've sprinkled the annotation on, > > > > We can remove it, for this driver I can vouch for its location as it did > > reach a state where I required a reboot. And its not the first time this > > has happened. This got me thinking about the bigger picture of the lack > > of proper way to address these cases in the kernel, and how the user is > > left dumbfounded. > > Fair, so the driver is still broken wrt. recovery here. I still don't > think that's a situation where e.g. the system should say "hey you have > a taint here, if your graphics go bad now you should not report that > bug" (which is effectively what the single taint bit does). But again, let's think about the generic type of issue, and the unexpected type of state that can be reached. The circumstance here *does* lead to a case which is not recoverable. Now, consider how many cases in the kernel where similar situations can happen and leave the device or driver in a non-functional state. > > > and secondly it actually hides useful information. > > > > What is it hiding? > > Most importantly, which device crashed. Secondarily I'd say how many > times (*). The device is implied by the module, the taint is applied to both. If you had multiple devices, however, yes, it would not be possible to distinguish from the taint which exact device it happened on. So the only thing *generic* which would be left out is count. > The information "firmware crashed" is really only useful in relation to > the device. If you have to reboot to get a functional network again then the device is quite useless for many people, regardless of which device that happened on. But from a support perspective a sysfs interface which provides a tiny bit more generic information indeed provides more value than a taint. Luis _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2020-05-18 21:18 UTC|newest] Thread overview: 128+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-15 21:28 [PATCH v2 00/15] net: taint when the device driver firmware crashes Luis Chamberlain 2020-05-15 21:28 ` [PATCH v2 01/15] taint: add module firmware crash taint support Luis Chamberlain 2020-05-16 4:03 ` Rafael Aquini 2020-05-19 16:42 ` Jessica Yu 2020-05-22 5:17 ` Luis Chamberlain 2020-05-15 21:28 ` [PATCH v2 02/15] ethernet/839: use new module_firmware_crashed() Luis Chamberlain 2020-05-16 4:04 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 03/15] bnx2x: " Luis Chamberlain 2020-05-16 4:05 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 04/15] bnxt: " Luis Chamberlain 2020-05-16 4:06 ` Rafael Aquini 2020-05-16 5:14 ` Vasundhara Volam 2020-05-15 21:28 ` [PATCH v2 05/15] bna: " Luis Chamberlain 2020-05-16 4:07 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 06/15] liquidio: " Luis Chamberlain 2020-05-16 4:07 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 07/15] cxgb4: " Luis Chamberlain 2020-05-16 4:09 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 08/15] ehea: " Luis Chamberlain 2020-05-16 4:09 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 09/15] qed: " Luis Chamberlain 2020-05-16 4:10 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 10/15] soc: qcom: ipa: " Luis Chamberlain 2020-05-16 4:10 ` Rafael Aquini 2020-05-19 22:34 ` Alex Elder 2020-05-22 5:28 ` Luis Chamberlain 2020-05-22 20:52 ` Alex Elder 2020-05-22 21:53 ` Luis Chamberlain 2020-05-15 21:28 ` [PATCH v2 11/15] wimax/i2400m: " Luis Chamberlain 2020-05-16 4:11 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 12/15] ath10k: " Luis Chamberlain 2020-05-15 21:28 ` Luis Chamberlain 2020-05-16 4:11 ` Rafael Aquini 2020-05-16 4:11 ` Rafael Aquini 2020-05-16 13:24 ` Johannes Berg 2020-05-16 13:24 ` Johannes Berg 2020-05-16 13:50 ` Johannes Berg 2020-05-16 13:50 ` Johannes Berg 2020-05-18 16:56 ` Luis Chamberlain 2020-05-18 16:56 ` Luis Chamberlain 2020-05-19 1:23 ` Brian Norris 2020-05-19 1:23 ` Brian Norris 2020-05-19 14:02 ` Luis Chamberlain 2020-05-19 14:02 ` Luis Chamberlain 2020-05-20 0:47 ` Brian Norris 2020-05-20 0:47 ` Brian Norris 2020-05-20 5:37 ` Emmanuel Grumbach 2020-05-20 5:37 ` Emmanuel Grumbach 2020-05-20 8:32 ` Andy Shevchenko 2020-05-20 8:32 ` Andy Shevchenko 2020-05-21 19:01 ` Brian Norris 2020-05-21 19:01 ` Brian Norris 2020-05-22 5:12 ` Emmanuel Grumbach 2020-05-22 5:12 ` Emmanuel Grumbach 2020-05-22 5:23 ` Luis Chamberlain 2020-05-22 5:23 ` Luis Chamberlain 2020-05-18 16:51 ` Luis Chamberlain 2020-05-18 16:51 ` Luis Chamberlain 2020-05-18 16:58 ` Ben Greear 2020-05-18 16:58 ` Ben Greear 2020-05-18 17:09 ` Luis Chamberlain 2020-05-18 17:09 ` Luis Chamberlain 2020-05-18 17:15 ` Ben Greear 2020-05-18 17:15 ` Ben Greear 2020-05-18 17:18 ` Luis Chamberlain 2020-05-18 17:18 ` Luis Chamberlain 2020-05-18 18:06 ` Steve deRosier 2020-05-18 18:06 ` Steve deRosier 2020-05-18 19:09 ` Luis Chamberlain 2020-05-18 19:09 ` Luis Chamberlain 2020-05-18 19:25 ` Johannes Berg 2020-05-18 19:25 ` Johannes Berg 2020-05-18 19:59 ` Luis Chamberlain 2020-05-18 19:59 ` Luis Chamberlain 2020-05-18 20:07 ` Johannes Berg 2020-05-18 20:07 ` Johannes Berg 2020-05-18 21:18 ` Luis Chamberlain [this message] 2020-05-18 21:18 ` Luis Chamberlain 2020-05-18 20:28 ` Jakub Kicinski 2020-05-18 20:28 ` Jakub Kicinski 2020-05-18 20:29 ` Johannes Berg 2020-05-18 20:29 ` Johannes Berg 2020-05-18 20:35 ` Jakub Kicinski 2020-05-18 20:35 ` Jakub Kicinski 2020-05-18 20:41 ` Johannes Berg 2020-05-18 20:41 ` Johannes Berg 2020-05-18 20:46 ` Jakub Kicinski 2020-05-18 20:46 ` Jakub Kicinski 2020-05-18 21:22 ` Luis Chamberlain 2020-05-18 21:22 ` Luis Chamberlain 2020-05-18 22:16 ` Jakub Kicinski 2020-05-18 22:16 ` Jakub Kicinski 2020-05-19 1:05 ` Luis Chamberlain 2020-05-19 1:05 ` Luis Chamberlain 2020-05-19 21:15 ` [RFC 1/2] devlink: add simple fw crash helpers Jakub Kicinski 2020-05-19 21:15 ` Jakub Kicinski 2020-05-22 5:20 ` Luis Chamberlain 2020-05-22 5:20 ` Luis Chamberlain 2020-05-22 17:17 ` Jakub Kicinski 2020-05-22 17:17 ` Jakub Kicinski 2020-05-22 20:46 ` Johannes Berg 2020-05-22 20:46 ` Johannes Berg 2020-05-22 21:51 ` Luis Chamberlain 2020-05-22 21:51 ` Luis Chamberlain 2020-05-22 23:23 ` Steve deRosier 2020-05-22 23:23 ` Steve deRosier 2020-05-22 23:44 ` Luis Chamberlain 2020-05-22 23:44 ` Luis Chamberlain 2020-05-25 9:07 ` Andy Shevchenko 2020-05-25 9:07 ` Andy Shevchenko 2020-05-25 17:08 ` Ben Greear 2020-05-25 17:08 ` Ben Greear 2020-05-25 20:57 ` Jakub Kicinski 2020-05-25 20:57 ` Jakub Kicinski 2020-07-30 13:56 ` Johannes Berg 2020-07-30 13:56 ` Johannes Berg 2020-05-22 21:49 ` Luis Chamberlain 2020-05-22 21:49 ` Luis Chamberlain 2020-05-19 21:15 ` [RFC 2/2] i2400m: use devlink health reporter Jakub Kicinski 2020-05-19 21:15 ` Jakub Kicinski 2020-05-15 21:28 ` [PATCH v2 13/15] ath6kl: use new module_firmware_crashed() Luis Chamberlain 2020-05-15 21:28 ` Luis Chamberlain 2020-05-16 4:12 ` Rafael Aquini 2020-05-16 4:12 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 14/15] brcm80211: " Luis Chamberlain 2020-05-16 4:13 ` Rafael Aquini 2020-05-15 21:28 ` [PATCH v2 15/15] mwl8k: " Luis Chamberlain 2020-05-16 4:13 ` Rafael Aquini
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200518211809.GQ11244@42.do-not-panic.com \ --to=mcgrof@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=andriy.shevchenko@linux.intel.com \ --cc=aquini@redhat.com \ --cc=arnd@arndb.de \ --cc=ath10k@lists.infradead.org \ --cc=bhe@redhat.com \ --cc=cai@lca.pw \ --cc=daniel.vetter@ffwll.ch \ --cc=davem@davemloft.net \ --cc=derosier@gmail.com \ --cc=dyoung@redhat.com \ --cc=gpiccoli@canonical.com \ --cc=greearb@candelatech.com \ --cc=jeyu@kernel.org \ --cc=johannes@sipsolutions.net \ --cc=keescook@chromium.org \ --cc=kvalo@codeaurora.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-wireless@vger.kernel.org \ --cc=mchehab+samsung@kernel.org \ --cc=mingo@redhat.com \ --cc=netdev@vger.kernel.org \ --cc=peterz@infradead.org \ --cc=pmladek@suse.com \ --cc=rostedt@goodmis.org \ --cc=schlad@suse.de \ --cc=tglx@linutronix.de \ --cc=tiwai@suse.de \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.