In the subject and through-out: s/recorders/pstore front-ends/ 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 > > 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@xxxxxxxxxxxxxxxxx> > --- > 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 > +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 > + > 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. > #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 > -- Kees Cook