* [PATCH] drivers/cros_ec: Reduce log polling period to 2s
@ 2023-09-21 14:19 Rob Barnes
2023-09-23 8:41 ` Tzung-Bi Shih
0 siblings, 1 reply; 5+ messages in thread
From: Rob Barnes @ 2023-09-21 14:19 UTC (permalink / raw
To: chrome-platform; +Cc: gwendal, groweck, Rob Barnes
Lowering the log polling interval to 2 seconds provides two advantages:
1. Minimizes the risk of EC internal log buffer overfilling and truncating.
2. Yields more recent logs prior to a crash, facilitating easier debugging.
Signed-off-by: Rob Barnes <robbarnes@google.com>
---
drivers/platform/chrome/cros_ec_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 9044c6bab770c..c96493504127d 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -21,7 +21,7 @@
#define LOG_SHIFT 14
#define LOG_SIZE (1 << LOG_SHIFT)
-#define LOG_POLL_SEC 10
+#define LOG_POLL_SEC 2
#define CIRC_ADD(idx, size, value) (((idx) + (value)) & ((size) - 1))
--
2.42.0.459.ge4e396fd5e-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/cros_ec: Reduce log polling period to 2s
2023-09-21 14:19 [PATCH] drivers/cros_ec: Reduce log polling period to 2s Rob Barnes
@ 2023-09-23 8:41 ` Tzung-Bi Shih
2023-09-26 18:57 ` Rob Barnes
0 siblings, 1 reply; 5+ messages in thread
From: Tzung-Bi Shih @ 2023-09-23 8:41 UTC (permalink / raw
To: Rob Barnes; +Cc: chrome-platform, gwendal, groweck
On Thu, Sep 21, 2023 at 02:19:35PM +0000, Rob Barnes wrote:
> 2. Yields more recent logs prior to a crash, facilitating easier debugging.
The approach depends on multiple parties before a system reboot: EC reports
on panic, AP sends EC commands for reading the logs, userland programs
(e.g. timberslide) write the logs to filesystem, and filesystem
synchronization. If anyone in the path didn't work in time, the logs
disappear.
An random idea: could we save the EC console logs to pstore when AP gets
notification on EC panic?
> ---
> drivers/platform/chrome/cros_ec_debugfs.c | 2 +-
If you have chance to send next version, please refer `git log` on the file
to see a good candidate of title prefixes.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/cros_ec: Reduce log polling period to 2s
2023-09-23 8:41 ` Tzung-Bi Shih
@ 2023-09-26 18:57 ` Rob Barnes
2023-09-28 7:07 ` Tzung-Bi Shih
0 siblings, 1 reply; 5+ messages in thread
From: Rob Barnes @ 2023-09-26 18:57 UTC (permalink / raw
To: Tzung-Bi Shih; +Cc: chrome-platform, gwendal, groweck
On Sat, Sep 23, 2023 at 2:42 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Thu, Sep 21, 2023 at 02:19:35PM +0000, Rob Barnes wrote:
> > 2. Yields more recent logs prior to a crash, facilitating easier debugging.
>
> The approach depends on multiple parties before a system reboot: EC reports
> on panic, AP sends EC commands for reading the logs, userland programs
> (e.g. timberslide) write the logs to filesystem, and filesystem
> synchronization. If anyone in the path didn't work in time, the logs
> disappear.
This change is just affecting the continuous polling period of the
cros_ec driver.
Syncing the log immediately after a panic is a separate path and will only work
when EC supports system safe mode recovery, most don't.
The hope with shortening the period is that cros_ec.previous will contain more
relevant logs after a crash, even when safe mode isn't enabled.
>
> An random idea: could we save the EC console logs to pstore when AP gets
> notification on EC panic?
This could work. It would be a separate effort.
>
> > ---
> > drivers/platform/chrome/cros_ec_debugfs.c | 2 +-
>
> If you have chance to send next version, please refer `git log` on the file
> to see a good candidate of title prefixes.
Ack.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/cros_ec: Reduce log polling period to 2s
2023-09-26 18:57 ` Rob Barnes
@ 2023-09-28 7:07 ` Tzung-Bi Shih
2024-05-02 17:13 ` Rob Barnes
0 siblings, 1 reply; 5+ messages in thread
From: Tzung-Bi Shih @ 2023-09-28 7:07 UTC (permalink / raw
To: Rob Barnes; +Cc: chrome-platform, gwendal, groeck
On Tue, Sep 26, 2023 at 12:57:22PM -0600, Rob Barnes wrote:
> On Sat, Sep 23, 2023 at 2:42 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Thu, Sep 21, 2023 at 02:19:35PM +0000, Rob Barnes wrote:
> > > 2. Yields more recent logs prior to a crash, facilitating easier debugging.
> >
> > The approach depends on multiple parties before a system reboot: EC reports
> > on panic, AP sends EC commands for reading the logs, userland programs
> > (e.g. timberslide) write the logs to filesystem, and filesystem
> > synchronization. If anyone in the path didn't work in time, the logs
> > disappear.
>
> This change is just affecting the continuous polling period of the
> cros_ec driver.
> Syncing the log immediately after a panic is a separate path and will only work
> when EC supports system safe mode recovery, most don't.
I guess I misunderstood. Is the understanding "EC can't respond to further
host commands from AP after EC crashed unless some more special supports
(e.g. system safe mode)" correct?
If yes, how about after EC reported a panic but before EC crashed? Can EC
respond to host commands during the period?
> The hope with shortening the period is that cros_ec.previous will contain more
> relevant logs after a crash, even when safe mode isn't enabled.
What is current approach for getting EC crash dumps/logs from AP? If the
system didn't save them before system reset, are they available after next
boot?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/cros_ec: Reduce log polling period to 2s
2023-09-28 7:07 ` Tzung-Bi Shih
@ 2024-05-02 17:13 ` Rob Barnes
0 siblings, 0 replies; 5+ messages in thread
From: Rob Barnes @ 2024-05-02 17:13 UTC (permalink / raw
To: Tzung-Bi Shih; +Cc: chrome-platform, gwendal, groeck
Sorry for dropping this conversation. I'm pursuing a different
approach for setting the ec log period. See
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/5509884
for the draft. I will send the patch to this list after internal
review.
Also answering your questions below.
On Thu, Sep 28, 2023 at 1:07 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, Sep 26, 2023 at 12:57:22PM -0600, Rob Barnes wrote:
> > On Sat, Sep 23, 2023 at 2:42 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > On Thu, Sep 21, 2023 at 02:19:35PM +0000, Rob Barnes wrote:
> > > > 2. Yields more recent logs prior to a crash, facilitating easier debugging.
> > >
> > > The approach depends on multiple parties before a system reboot: EC reports
> > > on panic, AP sends EC commands for reading the logs, userland programs
> > > (e.g. timberslide) write the logs to filesystem, and filesystem
> > > synchronization. If anyone in the path didn't work in time, the logs
> > > disappear.
> >
> > This change is just affecting the continuous polling period of the
> > cros_ec driver.
> > Syncing the log immediately after a panic is a separate path and will only work
> > when EC supports system safe mode recovery, most don't.
>
> I guess I misunderstood. Is the understanding "EC can't respond to further
> host commands from AP after EC crashed unless some more special supports
> (e.g. system safe mode)" correct?
>
> If yes, how about after EC reported a panic but before EC crashed? Can EC
> respond to host commands during the period?
System safe mode allows the EC to temporarily recover from some types
of panics. The EC is able to respond to host commands in this
scenario.
>
>
> > The hope with shortening the period is that cros_ec.previous will contain more
> > relevant logs after a crash, even when safe mode isn't enabled.
>
> What is current approach for getting EC crash dumps/logs from AP? If the
> system didn't save them before system reset, are they available after next
>
> boot?
The EC does save a small panicinfo struct in uninitialized RAM that is
preserved across reset. This struct does not include logs, so any EC
logs that are not sync'd to the OS before reset are lost.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-02 17:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-21 14:19 [PATCH] drivers/cros_ec: Reduce log polling period to 2s Rob Barnes
2023-09-23 8:41 ` Tzung-Bi Shih
2023-09-26 18:57 ` Rob Barnes
2023-09-28 7:07 ` Tzung-Bi Shih
2024-05-02 17:13 ` Rob Barnes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).