[7/8] efi: pstore: Follow convention for the efi-pstore backend name

For some reason, the efi-pstore backend name (exposed through the
pstore infrastructure) is hardcoded as "efi", whereas all the other
backends follow a kind of convention in using the module name.

Let's do it here as well, to make user's life easier (they might
use this info for unloading the module backend, for example).

Cc: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---
 drivers/firmware/efi/efi-pstore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Thu, Oct 06, 2022 at 07:42:11PM -0300, Guilherme G. Piccoli wrote:
> For some reason, the efi-pstore backend name (exposed through the
> pstore infrastructure) is hardcoded as "efi", whereas all the other
> backends follow a kind of convention in using the module name.
> 
> Let's do it here as well, to make user's life easier (they might
> use this info for unloading the module backend, for example).
> 
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Looks fine to me. Ard, if you don't object, I can carry this in the
pstore tree.

On Fri, 7 Oct 2022 at 01:16, Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 06, 2022 at 07:42:11PM -0300, Guilherme G. Piccoli wrote:
> > For some reason, the efi-pstore backend name (exposed through the
> > pstore infrastructure) is hardcoded as "efi", whereas all the other
> > backends follow a kind of convention in using the module name.
> >
> > Let's do it here as well, to make user's life easier (they might
> > use this info for unloading the module backend, for example).
> >
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
>
> Looks fine to me. Ard, if you don't object, I can carry this in the
> pstore tree.
>

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

On Thu, 6 Oct 2022 19:42:11 -0300, Guilherme G. Piccoli wrote:
> For some reason, the efi-pstore backend name (exposed through the
> pstore infrastructure) is hardcoded as "efi", whereas all the other
> backends follow a kind of convention in using the module name.
> 
> Let's do it here as well, to make user's life easier (they might
> use this info for unloading the module backend, for example).
> 
> [...]

Applied to for-next/pstore, thanks!

[7/8] efi: pstore: Follow convention for the efi-pstore backend name
      https://git.kernel.org/kees/c/39bae0ee0656

Quoting Guilherme G. Piccoli (2022-10-06 15:42:11)
> For some reason, the efi-pstore backend name (exposed through the
> pstore infrastructure) is hardcoded as "efi", whereas all the other
> backends follow a kind of convention in using the module name.
>
> Let's do it here as well, to make user's life easier (they might
> use this info for unloading the module backend, for example).

This patch broke ChromeOS' crash reporter when running on EFI[1], which
luckily isn't the typical mode of operation for Chromebooks. The problem
was that we had hardcoded something like dmesg-efi-<number> into the
regex logic that finds EFI pstore records. I didn't write the original
code but I think the idea was to speed things up by parsing the
filenames themselves to collect the files related to a crash record
instead of opening and parsing the header from the files to figure out
which file corresponds to which record.

I suspect the fix is pretty simple (make the driver name match either
one via a regex) but I just wanted to drop a note here that this made
some lives harder, not easier.

>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
>  drivers/firmware/efi/efi-pstore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
> index 3bddc152fcd4..97a9e84840a0 100644
> --- a/drivers/firmware/efi/efi-pstore.c
> +++ b/drivers/firmware/efi/efi-pstore.c
> @@ -207,7 +207,7 @@ static int efi_pstore_erase(struct pstore_record *record)
>
>  static struct pstore_info efi_pstore_info = {
>         .owner          = THIS_MODULE,
> -       .name           = "efi",
> +       .name           = KBUILD_MODNAME,
>         .flags          = PSTORE_FLAGS_DMESG,
>         .open           = efi_pstore_open,
>         .close          = efi_pstore_close,

[1] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform2/crash-reporter/kernel_collector.cc;l=54;drc=7a522166f0b2b32ece60f520b5d3d571c7545b0b

On 03/06/2024 20:02, Stephen Boyd wrote:
> [...]
> This patch broke ChromeOS' crash reporter when running on EFI[1], which
> luckily isn't the typical mode of operation for Chromebooks. The problem
> was that we had hardcoded something like dmesg-efi-<number> into the
> regex logic that finds EFI pstore records. I didn't write the original
> code but I think the idea was to speed things up by parsing the
> filenames themselves to collect the files related to a crash record
> instead of opening and parsing the header from the files to figure out
> which file corresponds to which record.
> 
> I suspect the fix is pretty simple (make the driver name match either
> one via a regex) but I just wanted to drop a note here that this made
> some lives harder, not easier.

Oh, many apologies for that Stephen - of course if I was aware of the
hardcoding in the tool, I'd not mess it up or would fix the tooling
first, before changing the kernel code.

At least, as a bright side here you found the tool's limitation and
there's the obvious improvement/fix for that =)

Cheers,


Guilherme