From: Kees Cook <keescook@chromium.org>
To: Bhaskara Budiredla <bbudiredla@marvell.com>
Cc: "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
"ccross@android.com" <ccross@android.com>,
"tony.luck@intel.com" <tony.luck@intel.com>,
Sunil Kovvuri Goutham <sgoutham@marvell.com>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"outgoing2/0000-cover-letter.patch@mx0b-0016f401.pphosted.com"
<outgoing2/0000-cover-letter.patch@mx0b-0016f401.pphosted.com>
Subject: Re: [EXT] Re: [PATCH v2 1/2] mmc: Support kmsg dumper based on pstore/blk
Date: Wed, 2 Dec 2020 11:32:12 -0800 [thread overview]
Message-ID: <202012021126.20ACBBD@keescook> (raw)
In-Reply-To: <CY4PR1801MB207097391BD8DE37A0F49102DEF30@CY4PR1801MB2070.namprd18.prod.outlook.com>
On Wed, Dec 02, 2020 at 06:36:21AM +0000, Bhaskara Budiredla wrote:
> >From: Kees Cook <keescook@chromium.org>
> >On Mon, Nov 23, 2020 at 04:49:24PM +0530, Bhaskara Budiredla wrote:
> >Why isn't this just written as:
> >
> >config MMC_PSTORE
> > bool "Log panic/oops to a MMC buffer"
> > depends on MMC_BLOCK
> > select PSTORE_BLK
> > help
> > This option will let you create platform backend to store kmsg
> > crash dumps to a user specified MMC device. This is primarily
> > based on pstore/blk.
> >
>
> The idea was to compile MMC_PSTORE as part of MMC_BLOCK driver,
> provided it is optionally enabled.
> The above arrangement compiles MMC_PSTORE
> as module: if (CONFIG_MMC_PSTORE_BACKEND == y && CONFIG_MMC_BLOCK == m)
> as static: if (CONFIG_MMC_PSTORE_BACKEND == y && CONFIG_MMC_BLOCK == y)
Ah, okay. If it's a tri-state, wouldn't it track CONFIG_MMC_BLOCK's
state? As in, does this work:
config MMC_PSTORE
tristate "Log panic/oops to a MMC buffer"
depends on MMC_BLOCK
select PSTORE_BLK
help
This option will let you create platform backend to store kmsg
crash dumps to a user specified MMC device. This is primarily
based on pstore/blk.
> >> + if (strncmp(cxt->dev_name, disk_name, strlen(disk_name)))
> >> + return;
> >
> >Why isn't this just strcmp()?
>
> The mmc disk name (disk_name) doesn't include the partition number.
> strncmp is restricted to something like /dev/mmcblk0, it doesn't cover full /dev/mmcblk0pn.
> The partition number check is carried out in the next statement.
Okay, gotcha; thanks!
> >> + dev->flags = PSTORE_FLAGS_DMESG;
> >
> >Can't this support more than just DMESG? I don't see anything specific to that.
> >This is using pstore/zone ultimately, which can support whatever frontends it
> >needs to.
>
> Yes, as of now the support is only for DMESG. We will extend this to other frontends
> on need basis.
Okay -- I assume this has mostly to do with not having erasure (below).
> >> + dev->erase = NULL;
> >
> >No way to remove the records?
>
> Yes, at this time, no removal of records.
Okay. (I think this might be worth mentioning in docs somewhere.)
--
Kees Cook
next prev parent reply other threads:[~2020-12-02 19:33 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 11:19 [PATCH v2 1/2] mmc: Support kmsg dumper based on pstore/blk Bhaskara Budiredla
2020-11-23 11:19 ` [PATCH v2 2/2] mmc: cavium: Add MMC polling method to support kmsg panic/oops write Bhaskara Budiredla
2020-11-24 11:37 ` [PATCH v2 1/2] mmc: Support kmsg dumper based on pstore/blk Christoph Hellwig
2020-11-24 14:22 ` Ulf Hansson
2020-12-01 20:28 ` Kees Cook
2020-12-01 20:26 ` Kees Cook
2020-12-02 6:36 ` [EXT] " Bhaskara Budiredla
2020-12-02 19:32 ` Kees Cook [this message]
2020-12-03 5:41 ` Bhaskara Budiredla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202012021126.20ACBBD@keescook \
--to=keescook@chromium.org \
--cc=bbudiredla@marvell.com \
--cc=ccross@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=outgoing2/0000-cover-letter.patch@mx0b-0016f401.pphosted.com \
--cc=sgoutham@marvell.com \
--cc=tony.luck@intel.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).