diff mbox series

[V2,1/3] pstore: Alert on backend write error

Message ID 20221013210648.137452-2-gpiccoli@igalia.com
State New
Headers
Series Some pstore improvements V2 |

Commit Message

Guilherme G. Piccoli Oct. 13, 2022, 9:06 p.m. UTC 
  The pstore dump function doesn't alert at all on errors - despite
pstore is usually a last resource and if it fails users won't be
able to read the kernel log, this is not the case for server users
with serial access, for example.

So, let's at least attempt to inform such advanced users on the first
backend writing error detected during the kmsg dump - this is also
very useful for pstore debugging purposes.

Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---


V2:
- Show error message late, outside of the critical region
(thanks Kees for the idea!).


 fs/pstore/platform.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Ard Biesheuvel Oct. 14, 2022, 2:46 p.m. UTC | #1 
On Thu, 13 Oct 2022 at 23:11, Guilherme G. Piccoli <gpiccoli@igalia.com> wrote:
>
> The pstore dump function doesn't alert at all on errors - despite
> pstore is usually a last resource and if it fails users won't be
> able to read the kernel log, this is not the case for server users
> with serial access, for example.
>
> So, let's at least attempt to inform such advanced users on the first
> backend writing error detected during the kmsg dump - this is also
> very useful for pstore debugging purposes.
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>
>
> V2:
> - Show error message late, outside of the critical region
> (thanks Kees for the idea!).
>
>
>  fs/pstore/platform.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 06c2c66af332..cbc0b468c1ab 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -393,6 +393,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
>         const char      *why;
>         unsigned int    part = 1;
>         unsigned long   flags = 0;
> +       int             saved_ret = 0;
>         int             ret;
>
>         why = kmsg_dump_reason_str(reason);
> @@ -463,12 +464,21 @@ static void pstore_dump(struct kmsg_dumper *dumper,
>                 if (ret == 0 && reason == KMSG_DUMP_OOPS) {
>                         pstore_new_entry = 1;
>                         pstore_timer_kick();
> +               } else {
> +                       /* Preserve only the first non-zero returned value. */
> +                       if (!saved_ret)
> +                               saved_ret = ret;
>                 }
>
>                 total += record.size;
>                 part++;
>         }
>         spin_unlock_irqrestore(&psinfo->buf_lock, flags);
> +
> +       if (saved_ret) {
> +               pr_err_once("backend (%s) writing error (%d)\n", psinfo->name,
> +                           saved_ret);
> +       }
>  }
>
>  static struct kmsg_dumper pstore_dumper = {
> --
> 2.38.0
>
Kees Cook Oct. 14, 2022, 5:41 p.m. UTC | #2 
On Thu, 13 Oct 2022 18:06:46 -0300, Guilherme G. Piccoli wrote:
> The pstore dump function doesn't alert at all on errors - despite
> pstore is usually a last resource and if it fails users won't be
> able to read the kernel log, this is not the case for server users
> with serial access, for example.
> 
> So, let's at least attempt to inform such advanced users on the first
> backend writing error detected during the kmsg dump - this is also
> very useful for pstore debugging purposes.
> 
> [...]

Applied to for-next/pstore, thanks!

[1/3] pstore: Alert on backend write error
      https://git.kernel.org/kees/c/f181c1af1385
diff mbox series

Patch

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 06c2c66af332..cbc0b468c1ab 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -393,6 +393,7 @@  static void pstore_dump(struct kmsg_dumper *dumper,
 	const char	*why;
 	unsigned int	part = 1;
 	unsigned long	flags = 0;
+	int		saved_ret = 0;
 	int		ret;
 
 	why = kmsg_dump_reason_str(reason);
@@ -463,12 +464,21 @@  static void pstore_dump(struct kmsg_dumper *dumper,
 		if (ret == 0 && reason == KMSG_DUMP_OOPS) {
 			pstore_new_entry = 1;
 			pstore_timer_kick();
+		} else {
+			/* Preserve only the first non-zero returned value. */
+			if (!saved_ret)
+				saved_ret = ret;
 		}
 
 		total += record.size;
 		part++;
 	}
 	spin_unlock_irqrestore(&psinfo->buf_lock, flags);
+
+	if (saved_ret) {
+		pr_err_once("backend (%s) writing error (%d)\n", psinfo->name,
+			    saved_ret);
+	}
 }
 
 static struct kmsg_dumper pstore_dumper = {