On systems that have a valid ACPI ERST table, if the pstore.backend kernel
parameter selects a specific facility other than erst, then during boot the
following console message is displayed:
ERST: Could not register with persistent store
This message makes it seem that something has gone wrong. In fact the same
message is used when you want ERST to be used by pstore and some real error
precludes registration.
The primary purpose of this 3 patch set is to avoid displaying this console
error message when another facility is explicitly selected as the backend for
pstore. Details are in the individual patches.
Lenny Szubowicz (3):
pstore: Return unique error if backend registration excluded by kernel
param
acpi: Eliminate console msg if pstore.backend excludes ERST
efivars: If pstore_register fails, free unneeded pstore buffer
drivers/acpi/apei/erst.c | 20 ++++++++++++++------
drivers/firmware/efi/efi-pstore.c | 6 +++++-
fs/pstore/platform.c | 11 ++++++-----
3 files changed, 25 insertions(+), 12 deletions(-)
--
1.8.2.1
This is patch 3/3 of a patch set that cleans up pstore_register failure paths.
If efivars fails to register with pstore, there is no point to keeping
the 4 KB buffer around. It's only used by the pstore read/write routines.
Signed-off-by: Lenny Szubowicz <[email protected]>
Reported-by: Naotaka Hamaguchi <[email protected]>
---
drivers/firmware/efi/efi-pstore.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 202d2c8..66c0c76 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -236,7 +236,11 @@ static __init int efivars_pstore_init(void)
efi_pstore_info.bufsize = 1024;
spin_lock_init(&efi_pstore_info.buf_lock);
- pstore_register(&efi_pstore_info);
+ if (pstore_register(&efi_pstore_info)) {
+ kfree(efi_pstore_info.buf);
+ efi_pstore_info.buf = NULL;
+ efi_pstore_info.bufsize = 0;
+ }
return 0;
}
--
1.8.2.1
This is patch 1/3 of a patch set that avoids what misleadingly appears
to be a error during boot:
ERST: Could not register with persistent store
This message is displayed if the system has a valid ACPI ERST table and the
pstore.backend kernel parameter has been used to disable use of ERST by
pstore. But this same message is used for errors that preclude registration.
As part of fixing this, return a unique error status from pstore_register
if the pstore.backend kernel parameter selects a specific facility other
than the requesting facility and check for this condition before any others.
This allows the caller to distinquish this benign case from the other failure
cases.
Also, print an informational console message about which facility
successfully registered as the pstore backend. Since there are various
kernel parameters, config build options, and boot-time errors that can
influence which facility registers with pstore, it's useful to have a
positive indication.
Signed-off-by: Lenny Szubowicz <[email protected]>
Reported-by: Naotaka Hamaguchi <[email protected]>
---
fs/pstore/platform.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 86d1038..84f3ca7 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -239,17 +239,15 @@ int pstore_register(struct pstore_info *psi)
{
struct module *owner = psi->owner;
+ if (backend && strcmp(backend, psi->name))
+ return -EINVAL;
+
spin_lock(&pstore_lock);
if (psinfo) {
spin_unlock(&pstore_lock);
return -EBUSY;
}
- if (backend && strcmp(backend, psi->name)) {
- spin_unlock(&pstore_lock);
- return -EINVAL;
- }
-
if (!psi->write)
psi->write = pstore_write_compat;
psinfo = psi;
@@ -274,6 +272,9 @@ int pstore_register(struct pstore_info *psi)
add_timer(&pstore_timer);
}
+ pr_info("pstore: Registered %s as persistent store backend\n",
+ psi->name);
+
return 0;
}
EXPORT_SYMBOL_GPL(pstore_register);
--
1.8.2.1
This is patch 2/3 of a patch set that avoids what misleadingly appears
to be a error during boot:
ERST: Could not register with persistent store
This message is displayed if the system has a valid ACPI ERST table and the
pstore.backend kernel parameter has been used to disable use of ERST by
pstore. But this same message is used for errors that preclude registration.
In erst_init don't complain if the setting of kernel parameter pstore.backend
precludes use of ACPI ERST for pstore. Routine pstore_register will inform
about the facility that does register.
Also, don't leave a dangling pointer to deallocated mem for the pstore
buffer when registration fails.
Signed-off-by: Lenny Szubowicz <[email protected]>
Reported-by: Naotaka Hamaguchi <[email protected]>
---
drivers/acpi/apei/erst.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
index 6d894bf..f7b3b39 100644
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -1180,20 +1180,28 @@ static int __init erst_init(void)
if (!erst_erange.vaddr)
goto err_release_erange;
+ pr_info(ERST_PFX
+ "Error Record Serialization Table (ERST) support is initialized.\n");
+
buf = kmalloc(erst_erange.size, GFP_KERNEL);
spin_lock_init(&erst_info.buf_lock);
if (buf) {
erst_info.buf = buf + sizeof(struct cper_pstore_record);
erst_info.bufsize = erst_erange.size -
sizeof(struct cper_pstore_record);
- if (pstore_register(&erst_info)) {
- pr_info(ERST_PFX "Could not register with persistent store\n");
+ rc = pstore_register(&erst_info);
+ if (rc) {
+ if (rc != -EPERM)
+ pr_info(ERST_PFX
+ "Could not register with persistent store\n");
+ erst_info.buf = NULL;
+ erst_info.bufsize = 0;
kfree(buf);
}
- }
-
- pr_info(ERST_PFX
- "Error Record Serialization Table (ERST) support is initialized.\n");
+ } else
+ pr_err(ERST_PFX
+ "Failed to allocate %lld bytes for persistent store error log\n",
+ erst_erange.size);
return 0;
--
1.8.2.1
On Fri, Jun 28, 2013 at 1:14 PM, Lenny Szubowicz <[email protected]> wrote:
> - if (pstore_register(&erst_info)) {
> - pr_info(ERST_PFX "Could not register with persistent store\n");
> + rc = pstore_register(&erst_info);
> + if (rc) {
> + if (rc != -EPERM)
> + pr_info(ERST_PFX
> + "Could not register with persistent store\n");
> + erst_info.buf = NULL;
> + erst_info.bufsize = 0;
Mismatch between part 1 and part 2 here ... we return -EINVAL if
our name doesn't match the desired backend ... but you only suppress
the "Could not register" message for -EPERM
Or am I confused while just looking at patch fragments?
-Tony
----- Original Message -----
> From: "Tony Luck" <[email protected]>
> To: "Lenny Szubowicz" <[email protected]>
> Cc: "Anton Vorontsov" <[email protected]>, "Matt Fleming" <[email protected]>, "Linux Kernel Mailing List"
> <[email protected]>, "n hamaguchi" <[email protected]>, "Don Zickus" <[email protected]>
> Sent: Friday, June 28, 2013 4:44:51 PM
> Subject: Re: [PATCH 2/3] acpi: Eliminate console msg if pstore.backend excludes ERST
>
> On Fri, Jun 28, 2013 at 1:14 PM, Lenny Szubowicz <[email protected]> wrote:
>
> > - if (pstore_register(&erst_info)) {
> > - pr_info(ERST_PFX "Could not register with
> > persistent store\n");
> > + rc = pstore_register(&erst_info);
> > + if (rc) {
> > + if (rc != -EPERM)
> > + pr_info(ERST_PFX
> > + "Could not register with persistent
> > store\n");
> > + erst_info.buf = NULL;
> > + erst_info.bufsize = 0;
>
> Mismatch between part 1 and part 2 here ... we return -EINVAL if
> our name doesn't match the desired backend ... but you only suppress
> the "Could not register" message for -EPERM
>
> Or am I confused while just looking at patch fragments?
>
> -Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Yes, you are absolutely correct. My [PATCH 1/3] is not what I intended. Thanks!
-Lenny.
On Fri, Jun 28, 2013 at 1:14 PM, Lenny Szubowicz <[email protected]> wrote:
> On systems that have a valid ACPI ERST table, if the pstore.backend kernel
> parameter selects a specific facility other than erst, then during boot the
> following console message is displayed:
>
> ERST: Could not register with persistent store
Applied (using revised version of part 1).
Thanks
-Tony