On Wed, Mar 25, 2020 at 04:54:57PM +0800, WeiXiong Liao wrote: > pstore/blk is similar to pstore/ram, but dump log to block device > rather than persistent ram. > [...] > +int psblk_register_blkdev(unsigned int major, psblk_panic_write_op panic_write) > +{ > + struct block_device *bdev; > + struct psblk_device dev = {0}; > + struct bdev_info *binfo; > + int ret = -ENODEV; > + void *holder = blkdev; > + > + binfo = psblk_get_bdev_info(); > + if (IS_ERR(binfo)) > + return PTR_ERR(binfo); > + > + /* only allow driver matching the @blkdev */ > + if (!binfo->devt || MAJOR(binfo->devt) != major) { > + pr_debug("invalid major %u (expect %u)\n", > + major, MAJOR(binfo->devt)); > + return -ENODEV; > + } > + > + /* hold bdev exclusively */ > + bdev = psblk_get_bdev(holder); > + if (IS_ERR(bdev)) { > + pr_err("failed to open '%s'!\n", blkdev); > + return PTR_ERR(bdev); > + } > + > + /* psblk_bdev must be assigned before register to pstore/blk */ > + psblk_bdev = bdev; > + blkdev_panic_write = panic_write; > + > + dev.total_size = psblk_bdev_size(bdev); > + dev.panic_write = panic_write ? psblk_blk_panic_write : NULL; > + dev.read = psblk_generic_blk_read; > + dev.write = psblk_generic_blk_write; > + > + ret = psblk_register_do(&dev); > + if (ret) > + goto err_put_bdev; > + > + pr_info("using '%s'\n", blkdev); > + return 0; > + > +err_put_bdev: > + psblk_bdev = NULL; > + blkdev_panic_write = NULL; > + psblk_put_bdev(bdev, holder); > + return ret; > +} > +EXPORT_SYMBOL_GPL(psblk_register_blkdev); I've gotten this series refactored on top of current pstore, and I've been making various bikeshed changes to names, etc, and as I went to go start testing, I realized that nothing actually uses psblk_register_blkdev(). It seems like it should be possible to just start using this on any block device of the user's choosing. I assume the idea is to allow for drivers to register panic_write handlers, but even without that, it'd be nice to just be able to test this with something like /dev/loop0. What's your thinking on how this would happen? It seems like if pstore/blk uses pstore/zone, and mtdpstore uses pstore/blk, there should be a blkoops that uses pstore/blk too? I guess I need to learn a bit more about how block device probing works so pstore/blk can notice devices as they're brought online, etc. -- Kees Cook