From: WeiXiong Liao <liaoweixiong@allwinnertech.com>
To: Kees Cook <keescook@chromium.org>
Cc: Rob Herring <robh@kernel.org>, Tony Luck <tony.luck@intel.com>,
Vignesh Raghavendra <vigneshr@ti.com>,
Jonathan Corbet <corbet@lwn.net>,
Richard Weinberger <richard@nod.at>,
Anton Vorontsov <anton@enomsg.org>,
linux-doc@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, Colin Cross <ccross@android.com>,
linux-mtd@lists.infradead.org,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH v2 08/11] blkoops: respect for device to pick recorders
Date: Sun, 22 Mar 2020 21:06:25 +0800 [thread overview]
Message-ID: <909c27a3-38d7-4825-6085-5b941c904e57@allwinnertech.com> (raw)
In-Reply-To: <202003181136.5F115BFCC1@keescook>
hi Kees Cook,
On 2020/3/19 AM 2:42, Kees Cook wrote:
> In the subject and through-out:
>
> s/recorders/pstore front-ends/
>
OK.
> On Fri, Feb 07, 2020 at 08:25:52PM +0800, WeiXiong Liao wrote:
>> It's one of a series of patches for adaptive to MTD device.
>
> typo: adapting
>
Fixed.
>>
>> MTD device is not block device. The sector of flash (MTD device) will be
>> broken if erase over limited cycles. Avoid damaging block so fast, we
>> can not write to a sector frequently. So, the recorders of pstore/blk
>> like console and ftrace recorder should not be supported.
>>
>> Besides, mtd device need aligned write/erase size. To avoid
>> over-erasing/writing flash, we should keep a aligned cache and read old
>> data to cache before write/erase, which make codes more complex. So,
>> pmsg do not be supported now because it writes misaligned.
>>
>> How about dmesg? Luckly, pstore/blk keeps several aligned chunks for
>> dmesg and uses one by one for wear balance.
>>
>> So, MTD device for pstore should pick recorders, that is why the patch
>> here.
>>
>> Signed-off-by: WeiXiong Liao <liaoweixiong@allwinnertech.com>
>> ---
>> Documentation/admin-guide/pstore-block.rst | 9 +++++++++
>> fs/pstore/blkoops.c | 29 +++++++++++++++++++++--------
>> include/linux/blkoops.h | 14 +++++++++++++-
>> 3 files changed, 43 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/pstore-block.rst b/Documentation/admin-guide/pstore-block.rst
>> index be865dfc1a28..299142b3d8e6 100644
>> --- a/Documentation/admin-guide/pstore-block.rst
>> +++ b/Documentation/admin-guide/pstore-block.rst
>> @@ -166,6 +166,15 @@ It is only required by block device which is registered by
>> ``blkoops_register_blkdev``. It's the major device number of registered
>> devices, by which blkoops can get the matching driver for @blkdev.
>>
>> +flags
>> +~~~~~
>> +
>> +Refer to macro starting with *BLKOOPS_DEV_SUPPORT_* which is defined in
>> +*linux/blkoops.h*. They tell us that which pstore/blk recorders this device
>> +supports. Default zero means all recorders for compatible, witch is the same
>
> typo: witch -> which
>
Fixed.
>> +as BLKOOPS_DEV_SUPPORT_ALL. Recorder works only when chunk size is not zero
>> +and device support.
>
> There are already flags for this, please see "Supported frontends"
> in include/linux/pstore.h
>
yes. You are right. I will change it.
>> +
>> total_size
>> ~~~~~~~~~~
>>
>> diff --git a/fs/pstore/blkoops.c b/fs/pstore/blkoops.c
>> index c76bab671b0b..01170b344f00 100644
>> --- a/fs/pstore/blkoops.c
>> +++ b/fs/pstore/blkoops.c
>> @@ -128,9 +128,16 @@ int blkoops_register_device(struct blkoops_device *bo_dev)
>> return -ENOMEM;
>> }
>>
>> -#define verify_size(name, defsize, alignsize) { \
>> - long _##name_ = (name); \
>> - if (_##name_ < 0) \
>> + /* zero means all recorders for compatible */
>> + if (bo_dev->flags == BLKOOPS_DEV_SUPPORT_DEFAULT)
>> + bo_dev->flags = BLKOOPS_DEV_SUPPORT_ALL;
>> +#define verify_size(name, defsize, alignsize, enable) { \
>> + long _##name_; \
>> + if (!(enable)) \
>> + _##name_ = 0; \
>> + else if ((name) >= 0) \
>> + _##name_ = (name); \
>> + else \
>> _##name_ = (defsize); \
>> _##name_ = _##name_ <= 0 ? 0 : (_##name_ * 1024); \
>> if (_##name_ & ((alignsize) - 1)) { \
>> @@ -142,10 +149,14 @@ int blkoops_register_device(struct blkoops_device *bo_dev)
>> bzinfo->name = _##name_; \
>> }
>>
>> - verify_size(dmesg_size, DEFAULT_DMESG_SIZE, 4096);
>> - verify_size(pmsg_size, DEFAULT_PMSG_SIZE, 4096);
>> - verify_size(console_size, DEFAULT_CONSOLE_SIZE, 4096);
>> - verify_size(ftrace_size, DEFAULT_FTRACE_SIZE, 4096);
>> + verify_size(dmesg_size, DEFAULT_DMESG_SIZE, 4096,
>> + bo_dev->flags & BLKOOPS_DEV_SUPPORT_DMESG);
>> + verify_size(pmsg_size, DEFAULT_PMSG_SIZE, 4096,
>> + bo_dev->flags & BLKOOPS_DEV_SUPPORT_PMSG);
>> + verify_size(console_size, DEFAULT_CONSOLE_SIZE, 4096,
>> + bo_dev->flags & BLKOOPS_DEV_SUPPORT_CONSOLE);
>> + verify_size(ftrace_size, DEFAULT_FTRACE_SIZE, 4096,
>> + bo_dev->flags & BLKOOPS_DEV_SUPPORT_FTRACE);
>
> I'd kind of prefer this patch be moved much earlier in the series so
> that the later additions of front-end support doesn't have to be touched
> twice. i.e. when PMSG support is added, it is added as a whole here and
> does the flag check in that patch, etc.
>
OK.
>> #undef verify_size
>> dump_oops = !!(dump_oops < 0 ? DEFAULT_DUMP_OOPS : dump_oops);
>>
>> @@ -336,6 +347,7 @@ static ssize_t blkoops_blk_panic_write(const char *buf, size_t size,
>> * register block device to blkoops
>> * @major: the major device number of registering device
>> * @panic_write: the write interface for panic case.
>> + * @flags: Refer to macro starting with BLKOOPS_DEV_SUPPORT.
>> *
>> * It is ONLY used for block device to register to blkoops. In this case,
>> * the module parameter @blkdev must be valid. Generic read/write interfaces
>> @@ -349,7 +361,7 @@ static ssize_t blkoops_blk_panic_write(const char *buf, size_t size,
>> * panic occurs but pstore/blk does not recover yet, the first zone of dmesg
>> * will be used.
>> */
>> -int blkoops_register_blkdev(unsigned int major,
>> +int blkoops_register_blkdev(unsigned int major, unsigned int flags,
>> blkoops_blk_panic_write_op panic_write)
>> {
>> struct block_device *bdev;
>> @@ -372,6 +384,7 @@ int blkoops_register_blkdev(unsigned int major,
>> if (bo_dev.total_size == 0)
>> goto err_put_bdev;
>> bo_dev.panic_write = panic_write ? blkoops_blk_panic_write : NULL;
>> + bo_dev.flags = flags;
>> bo_dev.read = blkoops_generic_blk_read;
>> bo_dev.write = blkoops_generic_blk_write;
>>
>> diff --git a/include/linux/blkoops.h b/include/linux/blkoops.h
>> index 71c596fd4cc8..bc7665d14a98 100644
>> --- a/include/linux/blkoops.h
>> +++ b/include/linux/blkoops.h
>> @@ -6,6 +6,7 @@
>> #include <linux/types.h>
>> #include <linux/blkdev.h>
>> #include <linux/pstore_blk.h>
>> +#include <linux/bitops.h>
>>
>> /**
>> * struct blkoops_device - backend blkoops driver structure.
>> @@ -14,6 +15,10 @@
>> * blkoops_register_device(). If block device, you are strongly recommended
>> * to use blkoops_register_blkdev().
>> *
>> + * @flags:
>> + * Refer to macro starting with BLKOOPS_DEV_SUPPORT_. These macros tell
>> + * us that which pstore/blk recorders this device supports. Zero means
>> + * all recorders for compatible.
>> * @total_size:
>> * The total size in bytes pstore/blk can use. It must be greater than
>> * 4096 and be multiple of 4096.
>> @@ -38,6 +43,13 @@
>> * On error, negative number should be returned.
>> */
>> struct blkoops_device {
>> + unsigned int flags;
>> +#define BLKOOPS_DEV_SUPPORT_ALL UINT_MAX
>> +#define BLKOOPS_DEV_SUPPORT_DEFAULT (0)
>> +#define BLKOOPS_DEV_SUPPORT_DMESG BIT(0)
>> +#define BLKOOPS_DEV_SUPPORT_PMSG BIT(1)
>> +#define BLKOOPS_DEV_SUPPORT_CONSOLE BIT(2)
>> +#define BLKOOPS_DEV_SUPPORT_FTRACE BIT(3)
>> unsigned long total_size;
>> blkz_read_op read;
>> blkz_write_op write;
>> @@ -54,7 +66,7 @@ typedef int (*blkoops_blk_panic_write_op)(const char *buf, sector_t start_sect,
>>
>> int blkoops_register_device(struct blkoops_device *bo_dev);
>> void blkoops_unregister_device(struct blkoops_device *bo_dev);
>> -int blkoops_register_blkdev(unsigned int major,
>> +int blkoops_register_blkdev(unsigned int major, unsigned int flags,
>> blkoops_blk_panic_write_op panic_write);
>> void blkoops_unregister_blkdev(unsigned int major);
>> int blkoops_blkdev_info(dev_t *devt, sector_t *nr_sects, sector_t *start_sect);
>> --
>> 1.9.1
>>
>
--
WeiXiong Liao
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2020-03-22 13:06 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-07 12:25 [PATCH v2 00/11] pstore: mtd: support crash log to block and mtd device WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 01/11] pstore/blk: new support logger for block devices WeiXiong Liao
2020-02-26 0:52 ` Kees Cook
2020-02-27 8:21 ` liaoweixiong
2020-03-18 17:23 ` Kees Cook
2020-03-20 1:50 ` WeiXiong Liao
2020-03-20 18:20 ` Kees Cook
2020-03-22 10:28 ` WeiXiong Liao
2020-03-09 0:52 ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 02/11] blkoops: add blkoops, a warpper for pstore/blk WeiXiong Liao
2020-03-18 18:06 ` Kees Cook
2020-03-22 10:00 ` WeiXiong Liao
2020-03-22 15:44 ` Kees Cook
2020-02-07 12:25 ` [PATCH v2 03/11] pstore/blk: blkoops: support pmsg recorder WeiXiong Liao
2020-03-18 18:13 ` Kees Cook
2020-03-22 11:14 ` WeiXiong Liao
2020-03-22 15:59 ` Kees Cook
2020-02-07 12:25 ` [PATCH v2 04/11] pstore/blk: blkoops: support console recorder WeiXiong Liao
2020-03-18 18:16 ` Kees Cook
2020-03-22 11:35 ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 05/11] pstore/blk: blkoops: support ftrace recorder WeiXiong Liao
2020-03-18 18:19 ` Kees Cook
2020-03-22 11:42 ` WeiXiong Liao
2020-03-22 15:16 ` Kees Cook
2020-02-07 12:25 ` [PATCH v2 06/11] Documentation: pstore/blk: blkoops: create document for pstore_blk WeiXiong Liao
2020-03-18 18:31 ` Kees Cook
2020-03-22 12:20 ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 07/11] pstore/blk: skip broken zone for mtd device WeiXiong Liao
2020-03-18 18:35 ` Kees Cook
2020-03-22 12:27 ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 08/11] blkoops: respect for device to pick recorders WeiXiong Liao
2020-03-18 18:42 ` Kees Cook
2020-03-22 13:06 ` WeiXiong Liao [this message]
2020-02-07 12:25 ` [PATCH v2 09/11] pstore/blk: blkoops: support special removing jobs for dmesg WeiXiong Liao
2020-03-18 18:47 ` Kees Cook
2020-03-22 13:03 ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 10/11] blkoops: add interface for dirver to get information of blkoops WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 11/11] mtd: new support oops logger based on pstore/blk WeiXiong Liao
2020-02-18 10:34 ` Miquel Raynal
2020-02-19 1:13 ` liaoweixiong
2020-03-18 18:57 ` Kees Cook
2020-03-22 13:51 ` WeiXiong Liao
2020-03-22 15:13 ` Kees Cook
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=909c27a3-38d7-4825-6085-5b941c904e57@allwinnertech.com \
--to=liaoweixiong@allwinnertech.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=anton@enomsg.org \
--cc=ccross@android.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=gregkh@linuxfoundation.org \
--cc=keescook@chromium.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=mchehab+samsung@kernel.org \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=robh@kernel.org \
--cc=tony.luck@intel.com \
--cc=vigneshr@ti.com \
/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).