[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4E795A9C.4040508@linux.intel.com>
Date: Wed, 21 Sep 2011 11:31:40 +0800
From: Chen Gong <gong.chen@...ux.intel.com>
To: Matthew Garrett <mjg@...hat.com>
CC: tony.luck@...el.com, linux-kernel@...r.kernel.org,
mikew@...gle.com, saguchi@...hat.com
Subject: Re: [PATCH] efi: Avoid sysfs spew on reboot and panic
δΊ 2011/9/21 5:07, Matthew Garrett ει:
> Right now all pstore accesses to efivars will delete or create new sysfs
> nodes. This is less than ideal if we've paniced or are rebooting, since
> we're in entirely the wrong context to do this kind of thing. Add the
> reason to the pstore callback and make sure we only manipulate the files
> if we're still going to be alive afterwards.
>
> Signed-off-by: Matthew Garrett<mjg@...hat.com>
> Reported-by: Seiji Aguchi<saguchi@...hat.com>
> ---
> drivers/acpi/apei/erst.c | 6 ++++--
> drivers/firmware/efivars.c | 17 +++++++++++++----
> fs/pstore/platform.c | 7 ++++---
> include/linux/kmsg_dump.h | 1 +
> include/linux/pstore.h | 5 ++++-
> 5 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
> index 2ca59dc..a6b5dc1 100644
> --- a/drivers/acpi/apei/erst.c
> +++ b/drivers/acpi/apei/erst.c
> @@ -934,7 +934,8 @@ static int erst_close_pstore(struct pstore_info *psi);
> static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
> struct timespec *time, struct pstore_info *psi);
> static u64 erst_writer(enum pstore_type_id type, unsigned int part,
> - size_t size, struct pstore_info *psi);
> + enum kmsg_dump_reason reason, size_t size,
> + struct pstore_info *psi);
> static int erst_clearer(enum pstore_type_id type, u64 id,
> struct pstore_info *psi);
>
> @@ -1041,7 +1042,8 @@ out:
> }
>
> static u64 erst_writer(enum pstore_type_id type, unsigned int part,
> - size_t size, struct pstore_info *psi)
> + enum kmsg_dump_reason reason, size_t size,
> + struct pstore_info *psi)
> {
> struct cper_pstore_record *rcd = (struct cper_pstore_record *)
> (erst_info.buf - sizeof(*rcd));
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index eb80b54..0510319 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -491,7 +491,8 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
> }
>
> static u64 efi_pstore_write(enum pstore_type_id type, unsigned int part,
> - size_t size, struct pstore_info *psi)
> + enum kmsg_dump_reason reason, size_t size,
> + struct pstore_info *psi)
> {
> char name[DUMP_NAME_LEN];
> char stub_name[DUMP_NAME_LEN];
> @@ -544,6 +545,14 @@ static u64 efi_pstore_write(enum pstore_type_id type, unsigned int part,
>
> spin_unlock(&efivars->lock);
>
> + /*
> + * If it's more severe than KMSG_DUMP_OOPS then we're already dead.
> + * Don't bother playing with sysfs.
> + */
> +
> + if (reason> KMSG_DUMP_OOPS)
> + return part;
> +
> if (found)
> efivar_unregister(found);
>
> @@ -552,14 +561,13 @@ static u64 efi_pstore_write(enum pstore_type_id type, unsigned int part,
> utf16_strsize(efi_name,
> DUMP_NAME_LEN * 2),
> efi_name,&vendor);
> -
> return part;
> };
>
> static int efi_pstore_erase(enum pstore_type_id type, u64 id,
> struct pstore_info *psi)
> {
> - efi_pstore_write(type, id, 0, psi);
> + efi_pstore_write(type, id, KMSG_DUMP_UNKNOWN, 0, psi);
>
> return 0;
> }
> @@ -581,7 +589,8 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
> }
>
> static u64 efi_pstore_write(enum pstore_type_id type, unsigned int part,
> - size_t size, struct pstore_info *psi)
> + enum kmsg_dump_reason reason, size_t size,
> + struct pstore_info *psi)
> {
> return 0;
> }
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index c5300ec..02117cc 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -51,7 +51,8 @@ void pstore_set_kmsg_bytes(int bytes)
> static int oopscount;
>
> static char *reason_str[] = {
> - "Oops", "Panic", "Kexec", "Restart", "Halt", "Poweroff", "Emergency"
> + "Unknown", "Oops", "Panic", "Kexec", "Restart", "Halt", "Poweroff",
> + "Emergency"
> };
>
> /*
> @@ -97,7 +98,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
> memcpy(dst, s1 + s1_start, l1_cpy);
> memcpy(dst + l1_cpy, s2 + s2_start, l2_cpy);
>
> - id = psinfo->write(PSTORE_TYPE_DMESG, part,
> + id = psinfo->write(PSTORE_TYPE_DMESG, part, reason,
> hsize + l1_cpy + l2_cpy, psinfo);
> if (reason == KMSG_DUMP_OOPS&& pstore_is_mounted())
> pstore_mkfile(PSTORE_TYPE_DMESG, psinfo->name, id,
> @@ -207,7 +208,7 @@ int pstore_write(enum pstore_type_id type, char *buf, size_t size)
>
> mutex_lock(&psinfo->buf_mutex);
> memcpy(psinfo->buf, buf, size);
> - id = psinfo->write(type, 0, size, psinfo);
> + id = psinfo->write(type, 0, KMSG_DUMP_UNKNOWN, size, psinfo);
I can't say it is wrong because no real caller for this function, but I can't
say it is right, yet. KMSG_DUMP_UNKNOWN here looks too arbirary. Do you have
any reason to use this type here ?
> if (pstore_is_mounted())
> pstore_mkfile(PSTORE_TYPE_DMESG, psinfo->name, id, psinfo->buf,
> size, CURRENT_TIME, psinfo);
> diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
> index ee0c952..b5ffe79 100644
> --- a/include/linux/kmsg_dump.h
> +++ b/include/linux/kmsg_dump.h
> @@ -16,6 +16,7 @@
> #include<linux/list.h>
>
> enum kmsg_dump_reason {
> + KMSG_DUMP_UNKNOWN,
> KMSG_DUMP_OOPS,
> KMSG_DUMP_PANIC,
> KMSG_DUMP_KEXEC,
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index cc03bbf..fa049e8 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -22,6 +22,8 @@
> #ifndef _LINUX_PSTORE_H
> #define _LINUX_PSTORE_H
>
> +#include<linux/kmsg_dump.h>
> +
> /* types */
> enum pstore_type_id {
> PSTORE_TYPE_DMESG = 0,
> @@ -40,7 +42,8 @@ struct pstore_info {
> ssize_t (*read)(u64 *id, enum pstore_type_id *type,
> struct timespec *time, struct pstore_info *psi);
> u64 (*write)(enum pstore_type_id type, unsigned int part,
> - size_t size, struct pstore_info *psi);
> + enum kmsg_dump_reason reason, size_t size,
> + struct pstore_info *psi);
> int (*erase)(enum pstore_type_id type, u64 id,
> struct pstore_info *psi);
> void *data;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists