Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
* simplify pstore-blk
@ 2020-10-16 13:20 Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 1/9] pstore/zone: cap the maximum device size Christoph Hellwig
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

Hi all,

this series cleans up and massively simplifies the pstore-blk code,
please take a look.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] pstore/zone: cap the maximum device size
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 2/9] pstore/blk: update the command line example Christoph Hellwig
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

Introduce an abritrary 128MiB cap to avoid malloc failures when using
a larger block device.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/pstore/zone.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c
index 3ce89216670c9b..5266ccbec007f3 100644
--- a/fs/pstore/zone.c
+++ b/fs/pstore/zone.c
@@ -1299,6 +1299,10 @@ int register_pstore_zone(struct pstore_zone_info *info)
 		pr_warn("total_size must be >= 4096\n");
 		return -EINVAL;
 	}
+	if (info->total_size > SZ_128M) {
+		pr_warn("capping size to 128MiB\n");
+		info->total_size = SZ_128M;
+	}
 
 	if (!info->kmsg_size && !info->pmsg_size && !info->console_size &&
 	    !info->ftrace_size) {
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/9] pstore/blk: update the command line example
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 1/9] pstore/zone: cap the maximum device size Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 3/9] pstore/blk: remove {un,}register_pstore_blk Christoph Hellwig
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

Use the human readable device name instead of the device number, and
add the required best_effort parameter.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/admin-guide/pstore-blk.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/pstore-blk.rst b/Documentation/admin-guide/pstore-blk.rst
index 296d5027787ac2..d9ec8b0572d3b2 100644
--- a/Documentation/admin-guide/pstore-blk.rst
+++ b/Documentation/admin-guide/pstore-blk.rst
@@ -35,7 +35,7 @@ module parameters have priority over Kconfig.
 
 Here is an example for module parameters::
 
-        pstore_blk.blkdev=179:7 pstore_blk.kmsg_size=64
+        pstore_blk.blkdev=/dev/mmcblk0p7 pstore_blk.kmsg_size=64 best_effort=y
 
 The detail of each configurations may be of interest to you.
 
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/9] pstore/blk: remove {un,}register_pstore_blk
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 1/9] pstore/zone: cap the maximum device size Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 2/9] pstore/blk: update the command line example Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 4/9] pstore/blk: remove __unregister_pstore_blk Christoph Hellwig
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

This interface is entirely unused, so remove them and various bits of
unreachable code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/admin-guide/pstore-blk.rst |  8 +--
 fs/pstore/blk.c                          | 79 ++----------------------
 include/linux/pstore_blk.h               | 42 -------------
 3 files changed, 7 insertions(+), 122 deletions(-)

diff --git a/Documentation/admin-guide/pstore-blk.rst b/Documentation/admin-guide/pstore-blk.rst
index d9ec8b0572d3b2..84477621384d85 100644
--- a/Documentation/admin-guide/pstore-blk.rst
+++ b/Documentation/admin-guide/pstore-blk.rst
@@ -151,13 +151,7 @@ otherwise KMSG_DUMP_MAX.
 Configurations for driver
 -------------------------
 
-Only a block device driver cares about these configurations. A block device
-driver uses ``register_pstore_blk`` to register to pstore/blk.
-
-.. kernel-doc:: fs/pstore/blk.c
-   :identifiers: register_pstore_blk
-
-A non-block device driver uses ``register_pstore_device`` with
+A device driver uses ``register_pstore_device`` with
 ``struct pstore_device_info`` to register to pstore/blk.
 
 .. kernel-doc:: fs/pstore/blk.c
diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index fcd5563dde063c..7f8368e94b3604 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -90,7 +90,6 @@ MODULE_PARM_DESC(blkdev, "block device for pstore storage");
 static DEFINE_MUTEX(pstore_blk_lock);
 static struct block_device *psblk_bdev;
 static struct pstore_zone_info *pstore_zone_info;
-static pstore_blk_panic_write_op blkdev_panic_write;
 
 struct bdev_info {
 	dev_t devt;
@@ -341,24 +340,7 @@ static ssize_t psblk_generic_blk_write(const char *buf, size_t bytes,
 	return ret;
 }
 
-static ssize_t psblk_blk_panic_write(const char *buf, size_t size,
-		loff_t off)
-{
-	int ret;
-
-	if (!blkdev_panic_write)
-		return -EOPNOTSUPP;
-
-	/* size and off must align to SECTOR_SIZE for block device */
-	ret = blkdev_panic_write(buf, off >> SECTOR_SHIFT,
-			size >> SECTOR_SHIFT);
-	/* try next zone */
-	if (ret == -ENOMSG)
-		return ret;
-	return ret ? -EIO : size;
-}
-
-static int __register_pstore_blk(struct pstore_blk_info *info)
+static int __register_pstore_blk(void)
 {
 	char bdev_name[BDEVNAME_SIZE];
 	struct block_device *bdev;
@@ -378,68 +360,34 @@ static int __register_pstore_blk(struct pstore_blk_info *info)
 	}
 
 	/* only allow driver matching the @blkdev */
-	if (!binfo.devt || (!best_effort &&
-			    MAJOR(binfo.devt) != info->major)) {
-		pr_debug("invalid major %u (expect %u)\n",
-				info->major, MAJOR(binfo.devt));
+	if (!binfo.devt) {
+		pr_debug("no major\n");
 		ret = -ENODEV;
 		goto err_put_bdev;
 	}
 
 	/* psblk_bdev must be assigned before register to pstore/blk */
 	psblk_bdev = bdev;
-	blkdev_panic_write = info->panic_write;
-
-	/* Copy back block device details. */
-	info->devt = binfo.devt;
-	info->nr_sects = binfo.nr_sects;
-	info->start_sect = binfo.start_sect;
 
 	memset(&dev, 0, sizeof(dev));
-	dev.total_size = info->nr_sects << SECTOR_SHIFT;
-	dev.flags = info->flags;
+	dev.total_size = binfo.nr_sects << SECTOR_SHIFT;
 	dev.read = psblk_generic_blk_read;
 	dev.write = psblk_generic_blk_write;
-	dev.erase = NULL;
-	dev.panic_write = info->panic_write ? psblk_blk_panic_write : NULL;
 
 	ret = __register_pstore_device(&dev);
 	if (ret)
 		goto err_put_bdev;
 
 	bdevname(bdev, bdev_name);
-	pr_info("attached %s%s\n", bdev_name,
-		info->panic_write ? "" : " (no dedicated panic_write!)");
+	pr_info("attached %s (no dedicated panic_write!)\n", bdev_name);
 	return 0;
 
 err_put_bdev:
 	psblk_bdev = NULL;
-	blkdev_panic_write = NULL;
 	psblk_put_bdev(bdev, holder);
 	return ret;
 }
 
-/**
- * register_pstore_blk() - register block device to pstore/blk
- *
- * @info: details on the desired block device interface
- *
- * Return:
- * * 0		- OK
- * * Others	- something error.
- */
-int register_pstore_blk(struct pstore_blk_info *info)
-{
-	int ret;
-
-	mutex_lock(&pstore_blk_lock);
-	ret = __register_pstore_blk(info);
-	mutex_unlock(&pstore_blk_lock);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(register_pstore_blk);
-
 static void __unregister_pstore_blk(unsigned int major)
 {
 	struct pstore_device_info dev = { .read = psblk_generic_blk_read };
@@ -449,24 +397,10 @@ static void __unregister_pstore_blk(unsigned int major)
 	if (psblk_bdev && MAJOR(psblk_bdev->bd_dev) == major) {
 		__unregister_pstore_device(&dev);
 		psblk_put_bdev(psblk_bdev, holder);
-		blkdev_panic_write = NULL;
 		psblk_bdev = NULL;
 	}
 }
 
-/**
- * unregister_pstore_blk() - unregister block device from pstore/blk
- *
- * @major: the major device number of device
- */
-void unregister_pstore_blk(unsigned int major)
-{
-	mutex_lock(&pstore_blk_lock);
-	__unregister_pstore_blk(major);
-	mutex_unlock(&pstore_blk_lock);
-}
-EXPORT_SYMBOL_GPL(unregister_pstore_blk);
-
 /* get information of pstore/blk */
 int pstore_blk_get_config(struct pstore_blk_config *info)
 {
@@ -483,12 +417,11 @@ EXPORT_SYMBOL_GPL(pstore_blk_get_config);
 
 static int __init pstore_blk_init(void)
 {
-	struct pstore_blk_info info = { };
 	int ret = 0;
 
 	mutex_lock(&pstore_blk_lock);
 	if (!pstore_zone_info && best_effort && blkdev[0])
-		ret = __register_pstore_blk(&info);
+		ret = __register_pstore_blk();
 	mutex_unlock(&pstore_blk_lock);
 
 	return ret;
diff --git a/include/linux/pstore_blk.h b/include/linux/pstore_blk.h
index 61e914522b0193..99564f93d77488 100644
--- a/include/linux/pstore_blk.h
+++ b/include/linux/pstore_blk.h
@@ -7,48 +7,6 @@
 #include <linux/pstore.h>
 #include <linux/pstore_zone.h>
 
-/**
- * typedef pstore_blk_panic_write_op - panic write operation to block device
- *
- * @buf: the data to write
- * @start_sect: start sector to block device
- * @sects: sectors count on buf
- *
- * Return: On success, zero should be returned. Others excluding -ENOMSG
- * mean error. -ENOMSG means to try next zone.
- *
- * Panic write to block device must be aligned to SECTOR_SIZE.
- */
-typedef int (*pstore_blk_panic_write_op)(const char *buf, sector_t start_sect,
-		sector_t sects);
-
-/**
- * struct pstore_blk_info - pstore/blk registration details
- *
- * @major:	Which major device number to support with pstore/blk
- * @flags:	The supported PSTORE_FLAGS_* from linux/pstore.h.
- * @panic_write:The write operation only used for the panic case.
- *		This can be NULL, but is recommended to avoid losing
- *		crash data if the kernel's IO path or work queues are
- *		broken during a panic.
- * @devt:	The dev_t that pstore/blk has attached to.
- * @nr_sects:	Number of sectors on @devt.
- * @start_sect:	Starting sector on @devt.
- */
-struct pstore_blk_info {
-	unsigned int major;
-	unsigned int flags;
-	pstore_blk_panic_write_op panic_write;
-
-	/* Filled in by pstore/blk after registration. */
-	dev_t devt;
-	sector_t nr_sects;
-	sector_t start_sect;
-};
-
-int  register_pstore_blk(struct pstore_blk_info *info);
-void unregister_pstore_blk(unsigned int major);
-
 /**
  * struct pstore_device_info - back-end pstore/blk driver structure.
  *
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/9] pstore/blk: remove __unregister_pstore_blk
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-10-16 13:20 ` [PATCH 3/9] pstore/blk: remove {un,}register_pstore_blk Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 5/9] pstore/blk: simplify the block device open / close path Christoph Hellwig
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

Fold __unregister_pstore_blk into its only caller, and merge the
two identical calls to __unregister_pstore_device that exist in the
caller now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/pstore/blk.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index 7f8368e94b3604..3a2214ecf942ae 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -388,19 +388,6 @@ static int __register_pstore_blk(void)
 	return ret;
 }
 
-static void __unregister_pstore_blk(unsigned int major)
-{
-	struct pstore_device_info dev = { .read = psblk_generic_blk_read };
-	void *holder = blkdev;
-
-	lockdep_assert_held(&pstore_blk_lock);
-	if (psblk_bdev && MAJOR(psblk_bdev->bd_dev) == major) {
-		__unregister_pstore_device(&dev);
-		psblk_put_bdev(psblk_bdev, holder);
-		psblk_bdev = NULL;
-	}
-}
-
 /* get information of pstore/blk */
 int pstore_blk_get_config(struct pstore_blk_config *info)
 {
@@ -430,16 +417,14 @@ late_initcall(pstore_blk_init);
 
 static void __exit pstore_blk_exit(void)
 {
+	struct pstore_device_info dev = { };
+
 	mutex_lock(&pstore_blk_lock);
+	if (pstore_zone_info)
+		dev.read = pstore_zone_info->read;
+	__unregister_pstore_device(&dev);
 	if (psblk_bdev)
-		__unregister_pstore_blk(MAJOR(psblk_bdev->bd_dev));
-	else {
-		struct pstore_device_info dev = { };
-
-		if (pstore_zone_info)
-			dev.read = pstore_zone_info->read;
-		__unregister_pstore_device(&dev);
-	}
+		psblk_put_bdev(psblk_bdev, blkdev);
 	mutex_unlock(&pstore_blk_lock);
 }
 module_exit(pstore_blk_exit);
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/9] pstore/blk: simplify the block device open / close path
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-10-16 13:20 ` [PATCH 4/9] pstore/blk: remove __unregister_pstore_blk Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 6/9] pstore/zone: split struct pstore_zone_info Christoph Hellwig
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

Remove the pointless psblk_get_bdev and psblk_put_bdev helper,
and don't bother holding pstore_blk_lock over the block device
open / close interactions given that they only happen first thing
during module init and last thing during module exit.  Also don't
bother with unregistering a zone info that did not come from the
actually block backed code, as users like mtd have to unregister
it themselves already.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/pstore/blk.c | 156 ++++++++++++------------------------------------
 1 file changed, 38 insertions(+), 118 deletions(-)

diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index 3a2214ecf942ae..a8aa56cba96d59 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -91,12 +91,6 @@ static DEFINE_MUTEX(pstore_blk_lock);
 static struct block_device *psblk_bdev;
 static struct pstore_zone_info *pstore_zone_info;
 
-struct bdev_info {
-	dev_t devt;
-	sector_t nr_sects;
-	sector_t start_sect;
-};
-
 #define check_size(name, alignsize) ({				\
 	long _##name_ = (name);					\
 	_##name_ = _##name_ <= 0 ? 0 : (_##name_ * 1024);	\
@@ -205,75 +199,6 @@ void unregister_pstore_device(struct pstore_device_info *dev)
 }
 EXPORT_SYMBOL_GPL(unregister_pstore_device);
 
-/**
- * psblk_get_bdev() - open block device
- *
- * @holder:	Exclusive holder identifier
- * @info:	Information about bdev to fill in
- *
- * Return: pointer to block device on success and others on error.
- *
- * On success, the returned block_device has reference count of one.
- */
-static struct block_device *psblk_get_bdev(void *holder,
-					   struct bdev_info *info)
-{
-	struct block_device *bdev = ERR_PTR(-ENODEV);
-	fmode_t mode = FMODE_READ | FMODE_WRITE;
-	sector_t nr_sects;
-
-	lockdep_assert_held(&pstore_blk_lock);
-
-	if (pstore_zone_info)
-		return ERR_PTR(-EBUSY);
-
-	if (!blkdev[0])
-		return ERR_PTR(-ENODEV);
-
-	if (holder)
-		mode |= FMODE_EXCL;
-	bdev = blkdev_get_by_path(blkdev, mode, holder);
-	if (IS_ERR(bdev)) {
-		dev_t devt;
-
-		devt = name_to_dev_t(blkdev);
-		if (devt == 0)
-			return ERR_PTR(-ENODEV);
-		bdev = blkdev_get_by_dev(devt, mode, holder);
-		if (IS_ERR(bdev))
-			return bdev;
-	}
-
-	nr_sects = part_nr_sects_read(bdev->bd_part);
-	if (!nr_sects) {
-		pr_err("not enough space for '%s'\n", blkdev);
-		blkdev_put(bdev, mode);
-		return ERR_PTR(-ENOSPC);
-	}
-
-	if (info) {
-		info->devt = bdev->bd_dev;
-		info->nr_sects = nr_sects;
-		info->start_sect = get_start_sect(bdev);
-	}
-
-	return bdev;
-}
-
-static void psblk_put_bdev(struct block_device *bdev, void *holder)
-{
-	fmode_t mode = FMODE_READ | FMODE_WRITE;
-
-	lockdep_assert_held(&pstore_blk_lock);
-
-	if (!bdev)
-		return;
-
-	if (holder)
-		mode |= FMODE_EXCL;
-	blkdev_put(bdev, mode);
-}
-
 static ssize_t psblk_generic_blk_read(char *buf, size_t bytes, loff_t pos)
 {
 	struct block_device *bdev = psblk_bdev;
@@ -340,29 +265,39 @@ static ssize_t psblk_generic_blk_write(const char *buf, size_t bytes,
 	return ret;
 }
 
-static int __register_pstore_blk(void)
+static int __init pstore_blk_init(void)
 {
 	char bdev_name[BDEVNAME_SIZE];
 	struct block_device *bdev;
 	struct pstore_device_info dev;
-	struct bdev_info binfo;
-	void *holder = blkdev;
 	int ret = -ENODEV;
-
-	lockdep_assert_held(&pstore_blk_lock);
+	fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
+	sector_t nr_sects;
+	
+	if (!best_effort || !blkdev[0])
+		return 0;
 
 	/* hold bdev exclusively */
-	memset(&binfo, 0, sizeof(binfo));
-	bdev = psblk_get_bdev(holder, &binfo);
+	bdev = blkdev_get_by_path(blkdev, mode, blkdev);
 	if (IS_ERR(bdev)) {
-		pr_err("failed to open '%s'!\n", blkdev);
-		return PTR_ERR(bdev);
+		dev_t devt;
+
+		devt = name_to_dev_t(blkdev);
+		if (devt == 0) {
+			pr_err("failed to open '%s'!\n", blkdev);
+			return -ENODEV;
+		}
+		bdev = blkdev_get_by_dev(devt, mode, blkdev);
+		if (IS_ERR(bdev)) {
+			pr_err("failed to open '%s'!\n", blkdev);
+			return PTR_ERR(bdev);
+		}
 	}
 
-	/* only allow driver matching the @blkdev */
-	if (!binfo.devt) {
-		pr_debug("no major\n");
-		ret = -ENODEV;
+	nr_sects = part_nr_sects_read(bdev->bd_part);
+	if (!nr_sects) {
+		pr_err("not enough space for '%s'\n", blkdev);
+		ret = -ENOSPC;
 		goto err_put_bdev;
 	}
 
@@ -370,11 +305,11 @@ static int __register_pstore_blk(void)
 	psblk_bdev = bdev;
 
 	memset(&dev, 0, sizeof(dev));
-	dev.total_size = binfo.nr_sects << SECTOR_SHIFT;
+	dev.total_size = nr_sects << SECTOR_SHIFT;
 	dev.read = psblk_generic_blk_read;
 	dev.write = psblk_generic_blk_write;
 
-	ret = __register_pstore_device(&dev);
+	ret = register_pstore_device(&dev);
 	if (ret)
 		goto err_put_bdev;
 
@@ -383,10 +318,22 @@ static int __register_pstore_blk(void)
 	return 0;
 
 err_put_bdev:
+	blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
 	psblk_bdev = NULL;
-	psblk_put_bdev(bdev, holder);
 	return ret;
 }
+late_initcall(pstore_blk_init);
+
+static void __exit pstore_blk_exit(void)
+{
+	struct pstore_device_info dev = { .read = psblk_generic_blk_read };
+
+	if (!psblk_bdev)
+		return;
+	unregister_pstore_device(&dev);
+	blkdev_put(psblk_bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
+}
+module_exit(pstore_blk_exit);
 
 /* get information of pstore/blk */
 int pstore_blk_get_config(struct pstore_blk_config *info)
@@ -402,33 +349,6 @@ int pstore_blk_get_config(struct pstore_blk_config *info)
 }
 EXPORT_SYMBOL_GPL(pstore_blk_get_config);
 
-static int __init pstore_blk_init(void)
-{
-	int ret = 0;
-
-	mutex_lock(&pstore_blk_lock);
-	if (!pstore_zone_info && best_effort && blkdev[0])
-		ret = __register_pstore_blk();
-	mutex_unlock(&pstore_blk_lock);
-
-	return ret;
-}
-late_initcall(pstore_blk_init);
-
-static void __exit pstore_blk_exit(void)
-{
-	struct pstore_device_info dev = { };
-
-	mutex_lock(&pstore_blk_lock);
-	if (pstore_zone_info)
-		dev.read = pstore_zone_info->read;
-	__unregister_pstore_device(&dev);
-	if (psblk_bdev)
-		psblk_put_bdev(psblk_bdev, blkdev);
-	mutex_unlock(&pstore_blk_lock);
-}
-module_exit(pstore_blk_exit);
-
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("WeiXiong Liao <liaoweixiong@allwinnertech.com>");
 MODULE_AUTHOR("Kees Cook <keescook@chromium.org>");
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 6/9] pstore/zone: split struct pstore_zone_info
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
                   ` (4 preceding siblings ...)
  2020-10-16 13:20 ` [PATCH 5/9] pstore/blk: simplify the block device open / close path Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 7/9] pstore/blk: remove struct pstore_device_info Christoph Hellwig
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

Split out a new pstore_zone_ops structure for static function pointers
plus the name. Also remove the unused owner field entirely.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/mtd/mtdpstore.c     | 13 +++++++----
 fs/pstore/blk.c             | 23 ++++++++++---------
 fs/pstore/zone.c            | 46 +++++++++++++++++++------------------
 include/linux/pstore_blk.h  | 23 ++-----------------
 include/linux/pstore_zone.h | 41 ++++++++++++++++++---------------
 5 files changed, 69 insertions(+), 77 deletions(-)

diff --git a/drivers/mtd/mtdpstore.c b/drivers/mtd/mtdpstore.c
index a3ae8778f6a9b9..232ba5c39c2a55 100644
--- a/drivers/mtd/mtdpstore.c
+++ b/drivers/mtd/mtdpstore.c
@@ -378,6 +378,14 @@ static ssize_t mtdpstore_panic_write(const char *buf, size_t size, loff_t off)
 	return retlen;
 }
 
+static const struct pstore_zone_ops mtdpstore_ops = {
+	.name		= KBUILD_MODNAME,
+	.read		= mtdpstore_read,
+	.write		= mtdpstore_write,
+	.erase		= mtdpstore_erase,
+	.panic_write	= mtdpstore_panic_write,
+};
+
 static void mtdpstore_notify_add(struct mtd_info *mtd)
 {
 	int ret;
@@ -426,10 +434,7 @@ static void mtdpstore_notify_add(struct mtd_info *mtd)
 	cxt->dev.total_size = mtd->size;
 	/* just support dmesg right now */
 	cxt->dev.flags = PSTORE_FLAGS_DMESG;
-	cxt->dev.read = mtdpstore_read;
-	cxt->dev.write = mtdpstore_write;
-	cxt->dev.erase = mtdpstore_erase;
-	cxt->dev.panic_write = mtdpstore_panic_write;
+	cxt->dev.ops = &mtdpstore_ops;
 
 	ret = register_pstore_device(&cxt->dev);
 	if (ret) {
diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index a8aa56cba96d59..f7c7f325e42c71 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -108,7 +108,8 @@ static int __register_pstore_device(struct pstore_device_info *dev)
 
 	lockdep_assert_held(&pstore_blk_lock);
 
-	if (!dev || !dev->total_size || !dev->read || !dev->write)
+	if (!dev || !dev->total_size || !dev->ops ||
+	    !dev->ops->read || !dev->ops->write)
 		return -EINVAL;
 
 	/* someone already registered before */
@@ -141,12 +142,7 @@ static int __register_pstore_device(struct pstore_device_info *dev)
 
 	pstore_zone_info->total_size = dev->total_size;
 	pstore_zone_info->max_reason = max_reason;
-	pstore_zone_info->read = dev->read;
-	pstore_zone_info->write = dev->write;
-	pstore_zone_info->erase = dev->erase;
-	pstore_zone_info->panic_write = dev->panic_write;
-	pstore_zone_info->name = KBUILD_MODNAME;
-	pstore_zone_info->owner = THIS_MODULE;
+	pstore_zone_info->ops = dev->ops;
 
 	ret = register_pstore_zone(pstore_zone_info);
 	if (ret) {
@@ -179,7 +175,7 @@ EXPORT_SYMBOL_GPL(register_pstore_device);
 static void __unregister_pstore_device(struct pstore_device_info *dev)
 {
 	lockdep_assert_held(&pstore_blk_lock);
-	if (pstore_zone_info && pstore_zone_info->read == dev->read) {
+	if (pstore_zone_info && pstore_zone_info->ops == dev->ops) {
 		unregister_pstore_zone(pstore_zone_info);
 		kfree(pstore_zone_info);
 		pstore_zone_info = NULL;
@@ -265,6 +261,12 @@ static ssize_t psblk_generic_blk_write(const char *buf, size_t bytes,
 	return ret;
 }
 
+static const struct pstore_zone_ops pstore_blk_zone_ops = {
+	.name		= KBUILD_MODNAME,
+	.read		= psblk_generic_blk_read,
+	.write		= psblk_generic_blk_write,
+};
+
 static int __init pstore_blk_init(void)
 {
 	char bdev_name[BDEVNAME_SIZE];
@@ -305,9 +307,8 @@ static int __init pstore_blk_init(void)
 	psblk_bdev = bdev;
 
 	memset(&dev, 0, sizeof(dev));
+	dev.ops = &pstore_blk_zone_ops;
 	dev.total_size = nr_sects << SECTOR_SHIFT;
-	dev.read = psblk_generic_blk_read;
-	dev.write = psblk_generic_blk_write;
 
 	ret = register_pstore_device(&dev);
 	if (ret)
@@ -326,7 +327,7 @@ late_initcall(pstore_blk_init);
 
 static void __exit pstore_blk_exit(void)
 {
-	struct pstore_device_info dev = { .read = psblk_generic_blk_read };
+	struct pstore_device_info dev = { .ops = &pstore_blk_zone_ops };
 
 	if (!psblk_bdev)
 		return;
diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c
index 5266ccbec007f3..111d26bb2a8f56 100644
--- a/fs/pstore/zone.c
+++ b/fs/pstore/zone.c
@@ -218,7 +218,7 @@ static int psz_zone_write(struct pstore_zone *zone,
 	if (!is_on_panic() && !atomic_read(&pstore_zone_cxt.recovered))
 		goto dirty;
 
-	writeop = is_on_panic() ? info->panic_write : info->write;
+	writeop = is_on_panic() ? info->ops->panic_write : info->ops->write;
 	if (!writeop)
 		goto dirty;
 
@@ -337,7 +337,7 @@ static int psz_kmsg_recover_data(struct psz_context *cxt)
 	unsigned long i;
 	ssize_t rcnt;
 
-	if (!info->read)
+	if (!info->ops->read)
 		return -EINVAL;
 
 	for (i = 0; i < cxt->kmsg_max_cnt; i++) {
@@ -360,8 +360,8 @@ static int psz_kmsg_recover_data(struct psz_context *cxt)
 		if (!zone->should_recover)
 			continue;
 		buf = zone->buffer;
-		rcnt = info->read((char *)buf, zone->buffer_size + sizeof(*buf),
-				zone->off);
+		rcnt = info->ops->read((char *)buf, zone->buffer_size +
+				sizeof(*buf), zone->off);
 		if (rcnt != zone->buffer_size + sizeof(*buf))
 			return (int)rcnt < 0 ? (int)rcnt : -EIO;
 	}
@@ -383,7 +383,7 @@ static int psz_kmsg_recover_meta(struct psz_context *cxt)
 	 */
 	char buffer_header[sizeof(*buf) + sizeof(*hdr)] = {0};
 
-	if (!info->read)
+	if (!info->ops->read)
 		return -EINVAL;
 
 	len = sizeof(*buf) + sizeof(*hdr);
@@ -393,13 +393,14 @@ static int psz_kmsg_recover_meta(struct psz_context *cxt)
 		if (unlikely(!zone))
 			return -EINVAL;
 
-		rcnt = info->read((char *)buf, len, zone->off);
+		rcnt = info->ops->read((char *)buf, len, zone->off);
 		if (rcnt == -ENOMSG) {
 			pr_debug("%s with id %lu may be broken, skip\n",
 					zone->name, i);
 			continue;
 		} else if (rcnt != len) {
-			pr_err("read %s with id %lu failed\n", zone->name, i);
+			pr_err("read %s with id %lu failed\n", zone->name,
+					i);
 			return (int)rcnt < 0 ? (int)rcnt : -EIO;
 		}
 
@@ -495,11 +496,11 @@ static int psz_recover_zone(struct psz_context *cxt, struct pstore_zone *zone)
 		return 0;
 	}
 
-	if (unlikely(!info->read))
+	if (unlikely(!info->ops->read))
 		return -EINVAL;
 
 	len = sizeof(struct psz_buffer);
-	rcnt = info->read((char *)&tmpbuf, len, zone->off);
+	rcnt = info->ops->read((char *)&tmpbuf, len, zone->off);
 	if (rcnt != len) {
 		pr_debug("read zone %s failed\n", zone->name);
 		return (int)rcnt < 0 ? (int)rcnt : -EIO;
@@ -541,7 +542,7 @@ static int psz_recover_zone(struct psz_context *cxt, struct pstore_zone *zone)
 	off = zone->off + sizeof(*oldbuf);
 
 	/* get part of data */
-	rcnt = info->read(buf, len - start, off + start);
+	rcnt = info->ops->read(buf, len - start, off + start);
 	if (rcnt != len - start) {
 		pr_err("read zone %s failed\n", zone->name);
 		ret = (int)rcnt < 0 ? (int)rcnt : -EIO;
@@ -549,7 +550,7 @@ static int psz_recover_zone(struct psz_context *cxt, struct pstore_zone *zone)
 	}
 
 	/* get the rest of data */
-	rcnt = info->read(buf + len - start, start, off);
+	rcnt = info->ops->read(buf + len - start, start, off);
 	if (rcnt != start) {
 		pr_err("read zone %s failed\n", zone->name);
 		ret = (int)rcnt < 0 ? (int)rcnt : -EIO;
@@ -671,8 +672,8 @@ static inline int psz_kmsg_erase(struct psz_context *cxt,
 
 	size = buffer_datalen(zone) + sizeof(*zone->buffer);
 	atomic_set(&zone->buffer->datalen, 0);
-	if (cxt->pstore_zone_info->erase)
-		return cxt->pstore_zone_info->erase(size, zone->off);
+	if (cxt->pstore_zone_info->ops->erase)
+		return cxt->pstore_zone_info->ops->erase(size, zone->off);
 	else
 		return psz_zone_write(zone, FLUSH_META, NULL, 0, 0);
 }
@@ -1185,8 +1186,9 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type,
 
 	*off += size;
 
-	pr_debug("pszone %s: off 0x%llx, %zu header, %zu data\n", zone->name,
-			zone->off, sizeof(*zone->buffer), zone->buffer_size);
+	pr_debug("pszone %s: off 0x%llx, %zu header, %zu data\n",
+			zone->name, zone->off, sizeof(*zone->buffer),
+			zone->buffer_size);
 	return zone;
 }
 
@@ -1310,7 +1312,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
 		return -EINVAL;
 	}
 
-	if (!info->name || !info->name[0])
+	if (!info->ops->name || !info->ops->name[0])
 		return -EINVAL;
 
 #define check_size(name, size) {					\
@@ -1338,7 +1340,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	 * if no @read, pstore may mount failed.
 	 * if no @write, pstore do not support to remove record file.
 	 */
-	if (!info->read || !info->write) {
+	if (!info->ops || !info->ops->read || !info->ops->write) {
 		pr_err("no valid general read/write interface\n");
 		return -EINVAL;
 	}
@@ -1346,13 +1348,13 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	mutex_lock(&cxt->pstore_zone_info_lock);
 	if (cxt->pstore_zone_info) {
 		pr_warn("'%s' already loaded: ignoring '%s'\n",
-				cxt->pstore_zone_info->name, info->name);
+				cxt->pstore_zone_info->ops->name, info->ops->name);
 		mutex_unlock(&cxt->pstore_zone_info_lock);
 		return -EBUSY;
 	}
 	cxt->pstore_zone_info = info;
 
-	pr_debug("register %s with properties:\n", info->name);
+	pr_debug("register %s with properties:\n", info->ops->name);
 	pr_debug("\ttotal size : %ld Bytes\n", info->total_size);
 	pr_debug("\tkmsg size : %ld Bytes\n", info->kmsg_size);
 	pr_debug("\tpmsg size : %ld Bytes\n", info->pmsg_size);
@@ -1376,14 +1378,14 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	}
 	cxt->pstore.data = cxt;
 
-	pr_info("registered %s as backend for", info->name);
+	pr_info("registered %s as backend for", info->ops->name);
 	cxt->pstore.max_reason = info->max_reason;
-	cxt->pstore.name = info->name;
+	cxt->pstore.name = info->ops->name;
 	if (info->kmsg_size) {
 		cxt->pstore.flags |= PSTORE_FLAGS_DMESG;
 		pr_cont(" kmsg(%s",
 			kmsg_dump_reason_str(cxt->pstore.max_reason));
-		if (cxt->pstore_zone_info->panic_write)
+		if (cxt->pstore_zone_info->ops->panic_write)
 			pr_cont(",panic_write");
 		pr_cont(")");
 	}
diff --git a/include/linux/pstore_blk.h b/include/linux/pstore_blk.h
index 99564f93d77488..095a44ce5e122c 100644
--- a/include/linux/pstore_blk.h
+++ b/include/linux/pstore_blk.h
@@ -15,31 +15,12 @@
  * @flags:	Refer to macro starting with PSTORE_FLAGS defined in
  *		linux/pstore.h. It means what front-ends this device support.
  *		Zero means all backends for compatible.
- * @read:	The general read operation. Both of the function parameters
- *		@size and @offset are relative value to bock device (not the
- *		whole disk).
- *		On success, the number of bytes should be returned, others
- *		means error.
- * @write:	The same as @read, but the following error number:
- *		-EBUSY means try to write again later.
- *		-ENOMSG means to try next zone.
- * @erase:	The general erase operation for device with special removing
- *		job. Both of the function parameters @size and @offset are
- *		relative value to storage.
- *		Return 0 on success and others on failure.
- * @panic_write:The write operation only used for panic case. It's optional
- *		if you do not care panic log. The parameters are relative
- *		value to storage.
- *		On success, the number of bytes should be returned, others
- *		excluding -ENOMSG mean error. -ENOMSG means to try next zone.
+ * @ops:	operations to access the device.
  */
 struct pstore_device_info {
 	unsigned long total_size;
 	unsigned int flags;
-	pstore_zone_read_op read;
-	pstore_zone_write_op write;
-	pstore_zone_erase_op erase;
-	pstore_zone_write_op panic_write;
+	const struct pstore_zone_ops *ops;
 };
 
 int  register_pstore_device(struct pstore_device_info *dev);
diff --git a/include/linux/pstore_zone.h b/include/linux/pstore_zone.h
index 1e35eaa33e5e0d..7bff124c96d8b1 100644
--- a/include/linux/pstore_zone.h
+++ b/include/linux/pstore_zone.h
@@ -5,22 +5,10 @@
 
 #include <linux/types.h>
 
-typedef ssize_t (*pstore_zone_read_op)(char *, size_t, loff_t);
-typedef ssize_t (*pstore_zone_write_op)(const char *, size_t, loff_t);
-typedef ssize_t (*pstore_zone_erase_op)(size_t, loff_t);
 /**
- * struct pstore_zone_info - pstore/zone back-end driver structure
+ * struct pstore_zone_ops - pstore/zone ops structure
  *
- * @owner:	Module which is responsible for this back-end driver.
  * @name:	Name of the back-end driver.
- * @total_size: The total size in bytes pstore/zone can use. It must be greater
- *		than 4096 and be multiple of 4096.
- * @kmsg_size:	The size of oops/panic zone. Zero means disabled, otherwise,
- *		it must be multiple of SECTOR_SIZE(512 Bytes).
- * @max_reason: Maximum kmsg dump reason to store.
- * @pmsg_size:	The size of pmsg zone which is the same as @kmsg_size.
- * @console_size:The size of console zone which is the same as @kmsg_size.
- * @ftrace_size:The size of ftrace zone which is the same as @kmsg_size.
  * @read:	The general read operation. Both of the function parameters
  *		@size and @offset are relative value to storage.
  *		On success, the number of bytes should be returned, others
@@ -38,20 +26,35 @@ typedef ssize_t (*pstore_zone_erase_op)(size_t, loff_t);
  *		On success, the number of bytes should be returned, others
  *		excluding -ENOMSG mean error. -ENOMSG means to try next zone.
  */
-struct pstore_zone_info {
-	struct module *owner;
+struct pstore_zone_ops {
 	const char *name;
+	ssize_t (*read)(char *buf, size_t count, loff_t pos);
+	ssize_t (*write)(const char *buf, size_t bytes, loff_t pos);
+	ssize_t (*erase)(size_t byes, loff_t pos);
+	ssize_t (*panic_write)(const char *buf, size_t bytes, loff_t pos);
+};
 
+/**
+ * struct pstore_zone_info - pstore/zone back-end driver structure
+ *
+ * @ops:	Operations to access the zone.
+ * @total_size: The total size in bytes pstore/zone can use. It must be greater
+ *		than 4096 and be multiple of 4096.
+ * @kmsg_size:	The size of oops/panic zone. Zero means disabled, otherwise,
+ *		it must be multiple of SECTOR_SIZE(512 Bytes).
+ * @max_reason: Maximum kmsg dump reason to store.
+ * @pmsg_size:	The size of pmsg zone which is the same as @kmsg_size.
+ * @console_size:The size of console zone which is the same as @kmsg_size.
+ * @ftrace_size:The size of ftrace zone which is the same as @kmsg_size.
+ */
+struct pstore_zone_info {
+	const struct pstore_zone_ops *ops;
 	unsigned long total_size;
 	unsigned long kmsg_size;
 	int max_reason;
 	unsigned long pmsg_size;
 	unsigned long console_size;
 	unsigned long ftrace_size;
-	pstore_zone_read_op read;
-	pstore_zone_write_op write;
-	pstore_zone_erase_op erase;
-	pstore_zone_write_op panic_write;
 };
 
 extern int register_pstore_zone(struct pstore_zone_info *info);
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 7/9] pstore/blk: remove struct pstore_device_info
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
                   ` (5 preceding siblings ...)
  2020-10-16 13:20 ` [PATCH 6/9] pstore/zone: split struct pstore_zone_info Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 8/9] pstore/blk: use the normal block device I/O path Christoph Hellwig
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

The total_size and flags are only needed at registrations time, so just
pass them to register_pstore_device directly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/mtd/mtdpstore.c    | 10 ++--
 fs/pstore/blk.c            | 98 ++++++++++++++++----------------------
 include/linux/pstore_blk.h | 21 ++------
 3 files changed, 47 insertions(+), 82 deletions(-)

diff --git a/drivers/mtd/mtdpstore.c b/drivers/mtd/mtdpstore.c
index 232ba5c39c2a55..88d0305ca27336 100644
--- a/drivers/mtd/mtdpstore.c
+++ b/drivers/mtd/mtdpstore.c
@@ -12,7 +12,6 @@
 static struct mtdpstore_context {
 	int index;
 	struct pstore_blk_config info;
-	struct pstore_device_info dev;
 	struct mtd_info *mtd;
 	unsigned long *rmmap;		/* removed bit map */
 	unsigned long *usedmap;		/* used bit map */
@@ -431,12 +430,9 @@ static void mtdpstore_notify_add(struct mtd_info *mtd)
 	longcnt = BITS_TO_LONGS(div_u64(mtd->size, mtd->erasesize));
 	cxt->badmap = kcalloc(longcnt, sizeof(long), GFP_KERNEL);
 
-	cxt->dev.total_size = mtd->size;
 	/* just support dmesg right now */
-	cxt->dev.flags = PSTORE_FLAGS_DMESG;
-	cxt->dev.ops = &mtdpstore_ops;
-
-	ret = register_pstore_device(&cxt->dev);
+	ret = register_pstore_device(&mtdpstore_ops, mtd->size,
+				     PSTORE_FLAGS_DMESG);
 	if (ret) {
 		dev_err(&mtd->dev, "mtd%d register to psblk failed\n",
 				mtd->index);
@@ -531,7 +527,7 @@ static void mtdpstore_notify_remove(struct mtd_info *mtd)
 
 	mtdpstore_flush_removed(cxt);
 
-	unregister_pstore_device(&cxt->dev);
+	unregister_pstore_device(&mtdpstore_ops);
 	kfree(cxt->badmap);
 	kfree(cxt->usedmap);
 	kfree(cxt->rmmap);
diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index f7c7f325e42c71..0430b190a1df2a 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -102,27 +102,40 @@ static struct pstore_zone_info *pstore_zone_info;
 	_##name_;						\
 })
 
-static int __register_pstore_device(struct pstore_device_info *dev)
+/**
+ * register_pstore_device() - register device to pstore/blk
+ *
+ * @ops:	operations to access the device.
+ * @total_size: The total size in bytes pstore/blk can use. It must be greater
+ *		than 4096 and be multiple of 4096.
+ * @flags:	Refer to macro starting with PSTORE_FLAGS defined in
+ *		linux/pstore.h. It means what front-ends this device support.
+ *		Zero means all backends for compatible.
+ */
+int register_pstore_device(const struct pstore_zone_ops *ops,
+		unsigned long total_size, unsigned int flags)
 {
 	int ret;
 
-	lockdep_assert_held(&pstore_blk_lock);
-
-	if (!dev || !dev->total_size || !dev->ops ||
-	    !dev->ops->read || !dev->ops->write)
+	if (!ops || !ops->read || !ops->write || !total_size)
 		return -EINVAL;
 
 	/* someone already registered before */
-	if (pstore_zone_info)
-		return -EBUSY;
+	mutex_lock(&pstore_blk_lock);
+	if (pstore_zone_info) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
 
 	pstore_zone_info = kzalloc(sizeof(struct pstore_zone_info), GFP_KERNEL);
-	if (!pstore_zone_info)
-		return -ENOMEM;
+	if (!pstore_zone_info) {
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
 
 	/* zero means not limit on which backends to attempt to store. */
-	if (!dev->flags)
-		dev->flags = UINT_MAX;
+	if (!flags)
+		flags = UINT_MAX;
 
 #define verify_size(name, alignsize, enabled) {				\
 		long _##name_;						\
@@ -134,63 +147,40 @@ static int __register_pstore_device(struct pstore_device_info *dev)
 		pstore_zone_info->name = _##name_;			\
 	}
 
-	verify_size(kmsg_size, 4096, dev->flags & PSTORE_FLAGS_DMESG);
-	verify_size(pmsg_size, 4096, dev->flags & PSTORE_FLAGS_PMSG);
-	verify_size(console_size, 4096, dev->flags & PSTORE_FLAGS_CONSOLE);
-	verify_size(ftrace_size, 4096, dev->flags & PSTORE_FLAGS_FTRACE);
+	verify_size(kmsg_size, 4096, flags & PSTORE_FLAGS_DMESG);
+	verify_size(pmsg_size, 4096, flags & PSTORE_FLAGS_PMSG);
+	verify_size(console_size, 4096, flags & PSTORE_FLAGS_CONSOLE);
+	verify_size(ftrace_size, 4096, flags & PSTORE_FLAGS_FTRACE);
 #undef verify_size
 
-	pstore_zone_info->total_size = dev->total_size;
+	pstore_zone_info->total_size = total_size;
 	pstore_zone_info->max_reason = max_reason;
-	pstore_zone_info->ops = dev->ops;
+	pstore_zone_info->ops = ops;
 
 	ret = register_pstore_zone(pstore_zone_info);
 	if (ret) {
 		kfree(pstore_zone_info);
 		pstore_zone_info = NULL;
 	}
+out_unlock:
+	mutex_unlock(&pstore_blk_lock);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(register_pstore_device);
+
 /**
- * register_pstore_device() - register non-block device to pstore/blk
- *
- * @dev: non-block device information
+ * unregister_pstore_device() - unregister a device from pstore/blk
  *
- * Return:
- * * 0		- OK
- * * Others	- something error.
+ * @ops: device operations
  */
-int register_pstore_device(struct pstore_device_info *dev)
+void unregister_pstore_device(const struct pstore_zone_ops *ops)
 {
-	int ret;
-
 	mutex_lock(&pstore_blk_lock);
-	ret = __register_pstore_device(dev);
-	mutex_unlock(&pstore_blk_lock);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(register_pstore_device);
-
-static void __unregister_pstore_device(struct pstore_device_info *dev)
-{
-	lockdep_assert_held(&pstore_blk_lock);
-	if (pstore_zone_info && pstore_zone_info->ops == dev->ops) {
+	if (pstore_zone_info && pstore_zone_info->ops == ops) {
 		unregister_pstore_zone(pstore_zone_info);
 		kfree(pstore_zone_info);
 		pstore_zone_info = NULL;
 	}
-}
-
-/**
- * unregister_pstore_device() - unregister non-block device from pstore/blk
- *
- * @dev: non-block device information
- */
-void unregister_pstore_device(struct pstore_device_info *dev)
-{
-	mutex_lock(&pstore_blk_lock);
-	__unregister_pstore_device(dev);
 	mutex_unlock(&pstore_blk_lock);
 }
 EXPORT_SYMBOL_GPL(unregister_pstore_device);
@@ -271,7 +261,6 @@ static int __init pstore_blk_init(void)
 {
 	char bdev_name[BDEVNAME_SIZE];
 	struct block_device *bdev;
-	struct pstore_device_info dev;
 	int ret = -ENODEV;
 	fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
 	sector_t nr_sects;
@@ -306,11 +295,8 @@ static int __init pstore_blk_init(void)
 	/* psblk_bdev must be assigned before register to pstore/blk */
 	psblk_bdev = bdev;
 
-	memset(&dev, 0, sizeof(dev));
-	dev.ops = &pstore_blk_zone_ops;
-	dev.total_size = nr_sects << SECTOR_SHIFT;
-
-	ret = register_pstore_device(&dev);
+	ret = register_pstore_device(&pstore_blk_zone_ops,
+			nr_sects << SECTOR_SHIFT, 0);
 	if (ret)
 		goto err_put_bdev;
 
@@ -327,11 +313,9 @@ late_initcall(pstore_blk_init);
 
 static void __exit pstore_blk_exit(void)
 {
-	struct pstore_device_info dev = { .ops = &pstore_blk_zone_ops };
-
 	if (!psblk_bdev)
 		return;
-	unregister_pstore_device(&dev);
+	unregister_pstore_device(&pstore_blk_zone_ops);
 	blkdev_put(psblk_bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
 }
 module_exit(pstore_blk_exit);
diff --git a/include/linux/pstore_blk.h b/include/linux/pstore_blk.h
index 095a44ce5e122c..0abd412a6cb3e3 100644
--- a/include/linux/pstore_blk.h
+++ b/include/linux/pstore_blk.h
@@ -7,24 +7,9 @@
 #include <linux/pstore.h>
 #include <linux/pstore_zone.h>
 
-/**
- * struct pstore_device_info - back-end pstore/blk driver structure.
- *
- * @total_size: The total size in bytes pstore/blk can use. It must be greater
- *		than 4096 and be multiple of 4096.
- * @flags:	Refer to macro starting with PSTORE_FLAGS defined in
- *		linux/pstore.h. It means what front-ends this device support.
- *		Zero means all backends for compatible.
- * @ops:	operations to access the device.
- */
-struct pstore_device_info {
-	unsigned long total_size;
-	unsigned int flags;
-	const struct pstore_zone_ops *ops;
-};
-
-int  register_pstore_device(struct pstore_device_info *dev);
-void unregister_pstore_device(struct pstore_device_info *dev);
+int register_pstore_device(const struct pstore_zone_ops *ops,
+		unsigned long total_size, unsigned int flags);
+void unregister_pstore_device(const struct pstore_zone_ops *ops);
 
 /**
  * struct pstore_blk_config - the pstore_blk backend configuration
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 8/9] pstore/blk: use the normal block device I/O path
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
                   ` (6 preceding siblings ...)
  2020-10-16 13:20 ` [PATCH 7/9] pstore/blk: remove struct pstore_device_info Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 13:20 ` [PATCH 9/9] pstore/blk: don't depend on CONFIG_BLOCK Christoph Hellwig
  2020-10-16 22:54 ` simplify pstore-blk Kees Cook
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

Stop poking into block layer internals and just open the block device
file an use kernel_read and kernel_write on it.  Note that this means
the transformation from name_to_dev_t can't be used anymore, and proper
block device file names must be used.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/pstore/blk.c            | 152 +++++++++++++------------------------
 include/linux/pstore_blk.h |   2 +
 init/main.c                |   4 +
 3 files changed, 57 insertions(+), 101 deletions(-)

diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index 0430b190a1df2a..bd4eadfc9bd795 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -8,15 +8,13 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include "../../block/blk.h"
 #include <linux/blkdev.h>
 #include <linux/string.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
 #include <linux/pstore_blk.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/init_syscalls.h>
 #include <linux/mount.h>
-#include <linux/uio.h>
 
 static long kmsg_size = CONFIG_PSTORE_BLK_KMSG_SIZE;
 module_param(kmsg_size, long, 0400);
@@ -88,7 +86,6 @@ MODULE_PARM_DESC(blkdev, "block device for pstore storage");
  * during the register/unregister functions.
  */
 static DEFINE_MUTEX(pstore_blk_lock);
-static struct block_device *psblk_bdev;
 static struct pstore_zone_info *pstore_zone_info;
 
 #define check_size(name, alignsize) ({				\
@@ -185,70 +182,20 @@ void unregister_pstore_device(const struct pstore_zone_ops *ops)
 }
 EXPORT_SYMBOL_GPL(unregister_pstore_device);
 
+static struct file *psblk_file;
+
 static ssize_t psblk_generic_blk_read(char *buf, size_t bytes, loff_t pos)
 {
-	struct block_device *bdev = psblk_bdev;
-	struct file file;
-	struct kiocb kiocb;
-	struct iov_iter iter;
-	struct kvec iov = {.iov_base = buf, .iov_len = bytes};
-
-	if (!bdev)
-		return -ENODEV;
-
-	memset(&file, 0, sizeof(struct file));
-	file.f_mapping = bdev->bd_inode->i_mapping;
-	file.f_flags = O_DSYNC | __O_SYNC | O_NOATIME;
-	file.f_inode = bdev->bd_inode;
-	file_ra_state_init(&file.f_ra, file.f_mapping);
-
-	init_sync_kiocb(&kiocb, &file);
-	kiocb.ki_pos = pos;
-	iov_iter_kvec(&iter, READ, &iov, 1, bytes);
-
-	return generic_file_read_iter(&kiocb, &iter);
+	return kernel_read(psblk_file, buf, bytes, &pos);
 }
 
 static ssize_t psblk_generic_blk_write(const char *buf, size_t bytes,
 		loff_t pos)
 {
-	struct block_device *bdev = psblk_bdev;
-	struct iov_iter iter;
-	struct kiocb kiocb;
-	struct file file;
-	ssize_t ret;
-	struct kvec iov = {.iov_base = (void *)buf, .iov_len = bytes};
-
-	if (!bdev)
-		return -ENODEV;
-
 	/* Console/Ftrace backend may handle buffer until flush dirty zones */
 	if (in_interrupt() || irqs_disabled())
 		return -EBUSY;
-
-	memset(&file, 0, sizeof(struct file));
-	file.f_mapping = bdev->bd_inode->i_mapping;
-	file.f_flags = O_DSYNC | __O_SYNC | O_NOATIME;
-	file.f_inode = bdev->bd_inode;
-
-	init_sync_kiocb(&kiocb, &file);
-	kiocb.ki_pos = pos;
-	iov_iter_kvec(&iter, WRITE, &iov, 1, bytes);
-
-	inode_lock(bdev->bd_inode);
-	ret = generic_write_checks(&kiocb, &iter);
-	if (ret > 0)
-		ret = generic_perform_write(&file, &iter, pos);
-	inode_unlock(bdev->bd_inode);
-
-	if (likely(ret > 0)) {
-		const struct file_operations f_op = {.fsync = blkdev_fsync};
-
-		file.f_op = &f_op;
-		kiocb.ki_pos += ret;
-		ret = generic_write_sync(&kiocb, ret);
-	}
-	return ret;
+	return kernel_write(psblk_file, buf, bytes, &pos);
 }
 
 static const struct pstore_zone_ops pstore_blk_zone_ops = {
@@ -257,68 +204,71 @@ static const struct pstore_zone_ops pstore_blk_zone_ops = {
 	.write		= psblk_generic_blk_write,
 };
 
-static int __init pstore_blk_init(void)
+static int __init __pstore_blk_init(const char *name)
 {
-	char bdev_name[BDEVNAME_SIZE];
-	struct block_device *bdev;
-	int ret = -ENODEV;
-	fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
-	sector_t nr_sects;
-	
+	int ret = -EINVAL;
+
 	if (!best_effort || !blkdev[0])
 		return 0;
 
-	/* hold bdev exclusively */
-	bdev = blkdev_get_by_path(blkdev, mode, blkdev);
-	if (IS_ERR(bdev)) {
-		dev_t devt;
-
-		devt = name_to_dev_t(blkdev);
-		if (devt == 0) {
-			pr_err("failed to open '%s'!\n", blkdev);
-			return -ENODEV;
-		}
-		bdev = blkdev_get_by_dev(devt, mode, blkdev);
-		if (IS_ERR(bdev)) {
-			pr_err("failed to open '%s'!\n", blkdev);
-			return PTR_ERR(bdev);
-		}
+	psblk_file = filp_open(name, O_RDWR | O_DSYNC | O_NOATIME | O_EXCL, 0);
+	if (IS_ERR(psblk_file)) {
+		ret = PTR_ERR(psblk_file);
+		pr_err("failed to open '%s': %d!\n", name, ret);
+		goto out;
 	}
-
-	nr_sects = part_nr_sects_read(bdev->bd_part);
-	if (!nr_sects) {
-		pr_err("not enough space for '%s'\n", blkdev);
-		ret = -ENOSPC;
-		goto err_put_bdev;
+	if (!S_ISBLK(file_inode(psblk_file)->i_mode)) {
+		pr_err("'%s' is not block device!\n", blkdev);
+		goto out_fput;
 	}
 
-	/* psblk_bdev must be assigned before register to pstore/blk */
-	psblk_bdev = bdev;
-
 	ret = register_pstore_device(&pstore_blk_zone_ops,
-			nr_sects << SECTOR_SHIFT, 0);
+			file_inode(psblk_file)->i_bdev->bd_inode->i_size, 0);
 	if (ret)
-		goto err_put_bdev;
+		goto out_fput;
 
-	bdevname(bdev, bdev_name);
-	pr_info("attached %s (no dedicated panic_write!)\n", bdev_name);
+	pr_info("using device '%s'\n", blkdev);
 	return 0;
-
-err_put_bdev:
-	blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
-	psblk_bdev = NULL;
+out_fput:
+	fput(psblk_file);
+out:
+	psblk_file = NULL;
 	return ret;
 }
+
+#ifdef MODULE
+static int __init pstore_blk_init(void)
+{
+	return __pstore_blk_init(blkdev);
+}
 late_initcall(pstore_blk_init);
 
 static void __exit pstore_blk_exit(void)
 {
-	if (!psblk_bdev)
+	if (!psblk_file)
 		return;
 	unregister_pstore_device(&pstore_blk_zone_ops);
-	blkdev_put(psblk_bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
+	fput(psblk_file);
 }
 module_exit(pstore_blk_exit);
+#else /* MODULE */
+/*
+ * During early boot the real root file system hasn't been mounted yet,
+ * and not device nodes are present yet.  Use the same scheme to find
+ * the device that we use for mounting the root file system.
+ */
+void __init pstore_blk_early_init(void)
+{
+	const char devname[] = "/dev/pstore-blk";
+	dev_t dev = name_to_dev_t(blkdev);
+
+	if (!dev)
+		return;
+	init_unlink(devname);
+	init_mknod(devname, S_IFBLK | 0600, new_encode_dev(dev));
+	__pstore_blk_init(devname);
+}
+#endif /* MODULE */
 
 /* get information of pstore/blk */
 int pstore_blk_get_config(struct pstore_blk_config *info)
diff --git a/include/linux/pstore_blk.h b/include/linux/pstore_blk.h
index 0abd412a6cb3e3..7c06b9d6740e2a 100644
--- a/include/linux/pstore_blk.h
+++ b/include/linux/pstore_blk.h
@@ -39,4 +39,6 @@ struct pstore_blk_config {
  */
 int pstore_blk_get_config(struct pstore_blk_config *info);
 
+void __init pstore_blk_early_init(void);
+
 #endif
diff --git a/init/main.c b/init/main.c
index 1af84337cb18d5..058cce64f70390 100644
--- a/init/main.c
+++ b/init/main.c
@@ -98,6 +98,7 @@
 #include <linux/mem_encrypt.h>
 #include <linux/kcsan.h>
 #include <linux/init_syscalls.h>
+#include <linux/pstore_blk.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
@@ -1524,6 +1525,9 @@ static noinline void __init kernel_init_freeable(void)
 		prepare_namespace();
 	}
 
+	if (IS_BUILTIN(CONFIG_PSTORE_BLK))
+		pstore_blk_early_init();
+
 	/*
 	 * Ok, we have completed the initial bootup, and
 	 * we're essentially up and running. Get rid of the
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 9/9] pstore/blk: don't depend on CONFIG_BLOCK
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
                   ` (7 preceding siblings ...)
  2020-10-16 13:20 ` [PATCH 8/9] pstore/blk: use the normal block device I/O path Christoph Hellwig
@ 2020-10-16 13:20 ` Christoph Hellwig
  2020-10-16 22:54 ` simplify pstore-blk Kees Cook
  9 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-10-16 13:20 UTC (permalink / raw)
  To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, WeiXiong Liao
  Cc: linux-mtd, linux-kernel

pstore-blk contains of two different layers:

 a) a tiny layer of sugar coating ontop of pstore-zone.  This part has
    no dependencies on the block layer, and can be used e.g. by mtd
 b) an implementation of a default fallback pstore zone backend for
    block devices

Add an ifdef for the latter so that pstore-blk itself does not have to
depend on CONFIG_BLOCK.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/pstore/Kconfig | 2 +-
 fs/pstore/blk.c   | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index e16a49ebfe546d..6eadb538316e52 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -164,7 +164,7 @@ config PSTORE_ZONE
 config PSTORE_BLK
 	tristate "Log panic/oops to a block device"
 	depends on PSTORE
-	depends on BLOCK
+	depends on BLOCK || !BLOCK
 	select PSTORE_ZONE
 	default n
 	help
diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index bd4eadfc9bd795..d3d25edb943cdd 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -182,6 +182,7 @@ void unregister_pstore_device(const struct pstore_zone_ops *ops)
 }
 EXPORT_SYMBOL_GPL(unregister_pstore_device);
 
+#ifdef CONFIG_BLOCK
 static struct file *psblk_file;
 
 static ssize_t psblk_generic_blk_read(char *buf, size_t bytes, loff_t pos)
@@ -269,6 +270,7 @@ void __init pstore_blk_early_init(void)
 	__pstore_blk_init(devname);
 }
 #endif /* MODULE */
+#endif /* CONFIG_BLOCK */
 
 /* get information of pstore/blk */
 int pstore_blk_get_config(struct pstore_blk_config *info)
-- 
2.28.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: simplify pstore-blk
  2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
                   ` (8 preceding siblings ...)
  2020-10-16 13:20 ` [PATCH 9/9] pstore/blk: don't depend on CONFIG_BLOCK Christoph Hellwig
@ 2020-10-16 22:54 ` Kees Cook
  9 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2020-10-16 22:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Tony Luck, Anton Vorontsov, linux-kernel, WeiXiong Liao,
	linux-mtd, Colin Cross

On Fri, Oct 16, 2020 at 03:20:38PM +0200, Christoph Hellwig wrote:
> this series cleans up and massively simplifies the pstore-blk code,
> please take a look.

Cool! Thanks for doing this work. I have a few things I'd like to see
done differently, and while I'm not a huge fan of the general reduction
in utility, I can live with it as long as it doesn't make other things
worse. :) I'll get this reviewed with specific feedback soon, but I'm
about to be EOW. ;)

-- 
Kees Cook

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16 13:20 simplify pstore-blk Christoph Hellwig
2020-10-16 13:20 ` [PATCH 1/9] pstore/zone: cap the maximum device size Christoph Hellwig
2020-10-16 13:20 ` [PATCH 2/9] pstore/blk: update the command line example Christoph Hellwig
2020-10-16 13:20 ` [PATCH 3/9] pstore/blk: remove {un,}register_pstore_blk Christoph Hellwig
2020-10-16 13:20 ` [PATCH 4/9] pstore/blk: remove __unregister_pstore_blk Christoph Hellwig
2020-10-16 13:20 ` [PATCH 5/9] pstore/blk: simplify the block device open / close path Christoph Hellwig
2020-10-16 13:20 ` [PATCH 6/9] pstore/zone: split struct pstore_zone_info Christoph Hellwig
2020-10-16 13:20 ` [PATCH 7/9] pstore/blk: remove struct pstore_device_info Christoph Hellwig
2020-10-16 13:20 ` [PATCH 8/9] pstore/blk: use the normal block device I/O path Christoph Hellwig
2020-10-16 13:20 ` [PATCH 9/9] pstore/blk: don't depend on CONFIG_BLOCK Christoph Hellwig
2020-10-16 22:54 ` simplify pstore-blk Kees Cook

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git