[PATCH] drivers/cros_ec: Emergency sync filesystem on EC panic

Rob Barnes posted 1 patch 2 years, 6 months ago
drivers/platform/chrome/cros_ec_lpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] drivers/cros_ec: Emergency sync filesystem on EC panic
Posted by Rob Barnes 2 years, 6 months ago
Perform an emergency filesystem sync when an EC panic is reported. An
emergency sync actually performs two syncs internally in case some
inodes or pages are temporarily locked.

hw_protection_shutdown is replaced for a few reasons. It is unnecessary
because the EC will force reset either way. hw_protection_shutdown does
not reliably sync filesystem before shutting down. emergency_sync is not
synchronous so hw_protection_shutdown may interrupt emergency_sync.

Signed-off-by: Rob Barnes <robbarnes@google.com>
---

 drivers/platform/chrome/cros_ec_lpc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 500a61b093e47..9652fc073c2a4 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -327,8 +327,8 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
 		dev_emerg(ec_dev->dev, "CrOS EC Panic Reported. Shutdown is imminent!");
 		blocking_notifier_call_chain(&ec_dev->panic_notifier, 0, ec_dev);
 		kobject_uevent_env(&ec_dev->dev->kobj, KOBJ_CHANGE, (char **)env);
-		/* Begin orderly shutdown. Force shutdown after 1 second. */
-		hw_protection_shutdown("CrOS EC Panic", 1000);
+		/* Force sync the filesystem before EC resets */
+		emergency_sync();
 		/* Do not query for other events after a panic is reported */
 		return;
 	}
-- 
2.41.0.255.g8b1d071c50-goog
Re: [PATCH] drivers/cros_ec: Emergency sync filesystem on EC panic
Posted by Benson Leung 2 years, 6 months ago
Hi Rob,

On Mon, Jul 17, 2023 at 4:29 PM Rob Barnes <robbarnes@google.com> wrote:
>
> Perform an emergency filesystem sync when an EC panic is reported. An
> emergency sync actually performs two syncs internally in case some
> inodes or pages are temporarily locked.
>
> hw_protection_shutdown is replaced for a few reasons. It is unnecessary
> because the EC will force reset either way. hw_protection_shutdown does
> not reliably sync filesystem before shutting down. emergency_sync is not
> synchronous so hw_protection_shutdown may interrupt emergency_sync.
>
> Signed-off-by: Rob Barnes <robbarnes@google.com>

Reviewed-by: Benson Leung <bleung@chromium.org>


> ---
>
>  drivers/platform/chrome/cros_ec_lpc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 500a61b093e47..9652fc073c2a4 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -327,8 +327,8 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
>                 dev_emerg(ec_dev->dev, "CrOS EC Panic Reported. Shutdown is imminent!");
>                 blocking_notifier_call_chain(&ec_dev->panic_notifier, 0, ec_dev);
>                 kobject_uevent_env(&ec_dev->dev->kobj, KOBJ_CHANGE, (char **)env);
> -               /* Begin orderly shutdown. Force shutdown after 1 second. */
> -               hw_protection_shutdown("CrOS EC Panic", 1000);
> +               /* Force sync the filesystem before EC resets */
> +               emergency_sync();
>                 /* Do not query for other events after a panic is reported */
>                 return;
>         }
> --
> 2.41.0.255.g8b1d071c50-goog
>


-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org
Re: [PATCH] drivers/cros_ec: Emergency sync filesystem on EC panic
Posted by Guenter Roeck 2 years, 6 months ago
On Mon, Jul 17, 2023 at 4:29 PM Rob Barnes <robbarnes@google.com> wrote:
>
> Perform an emergency filesystem sync when an EC panic is reported. An
> emergency sync actually performs two syncs internally in case some
> inodes or pages are temporarily locked.
>
> hw_protection_shutdown is replaced for a few reasons. It is unnecessary
> because the EC will force reset either way. hw_protection_shutdown does
> not reliably sync filesystem before shutting down. emergency_sync is not
> synchronous so hw_protection_shutdown may interrupt emergency_sync.
>
> Signed-off-by: Rob Barnes <robbarnes@google.com>

Makes sense.

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>
>  drivers/platform/chrome/cros_ec_lpc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 500a61b093e47..9652fc073c2a4 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -327,8 +327,8 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
>                 dev_emerg(ec_dev->dev, "CrOS EC Panic Reported. Shutdown is imminent!");
>                 blocking_notifier_call_chain(&ec_dev->panic_notifier, 0, ec_dev);
>                 kobject_uevent_env(&ec_dev->dev->kobj, KOBJ_CHANGE, (char **)env);
> -               /* Begin orderly shutdown. Force shutdown after 1 second. */
> -               hw_protection_shutdown("CrOS EC Panic", 1000);
> +               /* Force sync the filesystem before EC resets */
> +               emergency_sync();
>                 /* Do not query for other events after a panic is reported */
>                 return;
>         }
> --
> 2.41.0.255.g8b1d071c50-goog
>
Re: [PATCH] drivers/cros_ec: Emergency sync filesystem on EC panic
Posted by patchwork-bot+chrome-platform@kernel.org 2 years, 6 months ago
Hello:

This patch was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Mon, 17 Jul 2023 23:29:32 +0000 you wrote:
> Perform an emergency filesystem sync when an EC panic is reported. An
> emergency sync actually performs two syncs internally in case some
> inodes or pages are temporarily locked.
> 
> hw_protection_shutdown is replaced for a few reasons. It is unnecessary
> because the EC will force reset either way. hw_protection_shutdown does
> not reliably sync filesystem before shutting down. emergency_sync is not
> synchronous so hw_protection_shutdown may interrupt emergency_sync.
> 
> [...]

Here is the summary with links:
  - drivers/cros_ec: Emergency sync filesystem on EC panic
    https://git.kernel.org/chrome-platform/c/a6edd5f5d9cc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH] drivers/cros_ec: Emergency sync filesystem on EC panic
Posted by patchwork-bot+chrome-platform@kernel.org 2 years, 6 months ago
Hello:

This patch was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Mon, 17 Jul 2023 23:29:32 +0000 you wrote:
> Perform an emergency filesystem sync when an EC panic is reported. An
> emergency sync actually performs two syncs internally in case some
> inodes or pages are temporarily locked.
> 
> hw_protection_shutdown is replaced for a few reasons. It is unnecessary
> because the EC will force reset either way. hw_protection_shutdown does
> not reliably sync filesystem before shutting down. emergency_sync is not
> synchronous so hw_protection_shutdown may interrupt emergency_sync.
> 
> [...]

Here is the summary with links:
  - drivers/cros_ec: Emergency sync filesystem on EC panic
    https://git.kernel.org/chrome-platform/c/a6edd5f5d9cc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html