Received-SPF: pass (google.com: domain of linux-kernel+bounces-199805-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20221006224212.569555-8-gpiccoli@igalia.com>
References: <20221006224212.569555-1-gpiccoli@igalia.com> <20221006224212.569555-8-gpiccoli@igalia.com>
From: Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.10
Date: Mon, 3 Jun 2024 16:02:38 -0700
Message-ID: <CAE-0n50vo5xkUNK0-cF9HZRXShsxbikqmdVnmMzRsn+Z7MEJTg@mail.gmail.com>
Subject: Re: [PATCH 7/8] efi: pstore: Follow convention for the efi-pstore
 backend name
To: "Guilherme G. Piccoli" <gpiccoli@igalia.com>, linux-fsdevel@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Cc: kernel-dev@igalia.com, kernel@gpiccoli.net, keescook@chromium.org, 
	anton@enomsg.org, ccross@android.com, tony.luck@intel.com, 
	linux-efi@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>
Content-Type: text/plain; charset="UTF-8"

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