linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Sergei Shtepa <sergei.shtepa@veeam.com>
Cc: axboe@kernel.dk, corbet@lwn.net, linux-block@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 06/21] block, blksnap: module management interface functions
Date: Thu, 15 Dec 2022 01:28:58 -0800	[thread overview]
Message-ID: <Y5ro2qv8pxGQ5bTN@infradead.org> (raw)
In-Reply-To: <20221209142331.26395-7-sergei.shtepa@veeam.com>

> +	ret = register_chrdev(0, THIS_MODULE->name, &ctrl_fops);

This crashed when blksnap is built into the kernel because THIS_MODULE
is NULL for that case.

But I think most of this boilerplate can be removed - just use a misc
device and use /proc/misc to find the major from userspace if anything
cares, but nothing should as the misc devices already create the
node through udev.  I.e.g:

diff --git a/drivers/block/blksnap/Makefile b/drivers/block/blksnap/Makefile
index b196b17f9d9d97..cbcac12cc61d59 100644
--- a/drivers/block/blksnap/Makefile
+++ b/drivers/block/blksnap/Makefile
@@ -12,7 +12,6 @@ blksnap-y := 		\
 	main.o		\
 	snapimage.o	\
 	snapshot.o	\
-	sysfs.o		\
 	tracker.o
 
 obj-$(CONFIG_BLK_SNAP)	 += blksnap.o
diff --git a/drivers/block/blksnap/ctrl.c b/drivers/block/blksnap/ctrl.c
index 990ffb004db3f9..d1a5e3a13a19be 100644
--- a/drivers/block/blksnap/ctrl.c
+++ b/drivers/block/blksnap/ctrl.c
@@ -5,6 +5,7 @@
 #include <linux/poll.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
+#include <linux/miscdevice.h>
 #include <uapi/linux/blksnap.h>
 #include "ctrl.h"
 #include "params.h"
@@ -16,16 +17,6 @@
 static_assert(sizeof(uuid_t) == sizeof(struct blk_snap_uuid),
 	"Invalid size of struct blk_snap_uuid or uuid_t.");
 
-static int blk_snap_major;
-
-static long ctrl_unlocked_ioctl(struct file *filp, unsigned int cmd,
-				unsigned long arg);
-
-static const struct file_operations ctrl_fops = {
-	.owner = THIS_MODULE,
-	.unlocked_ioctl = ctrl_unlocked_ioctl,
-};
-
 static const struct blk_snap_version version = {
 	.major = VERSION_MAJOR,
 	.minor = VERSION_MINOR,
@@ -33,34 +24,6 @@ static const struct blk_snap_version version = {
 	.build = VERSION_BUILD,
 };
 
-int get_blk_snap_major(void)
-{
-	return blk_snap_major;
-}
-
-int ctrl_init(void)
-{
-	int ret;
-
-	ret = register_chrdev(0, THIS_MODULE->name, &ctrl_fops);
-	if (ret < 0) {
-		pr_err("Failed to register a character device. errno=%d\n",
-		       abs(blk_snap_major));
-		return ret;
-	}
-
-	blk_snap_major = ret;
-	pr_info("Register control device [%d:0].\n", blk_snap_major);
-	return 0;
-}
-
-void ctrl_done(void)
-{
-	pr_info("Unregister control device\n");
-
-	unregister_chrdev(blk_snap_major, THIS_MODULE->name);
-}
-
 static int ioctl_version(unsigned long arg)
 {
 	if (copy_to_user((void *)arg, &version, sizeof(version))) {
@@ -408,3 +371,24 @@ static long ctrl_unlocked_ioctl(struct file *filp, unsigned int cmd,
 
 	return blk_snap_ioctl_table[nr](arg);
 }
+
+static const struct file_operations blksnap_ctrl_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= ctrl_unlocked_ioctl,
+};
+
+static struct miscdevice blksnap_ctrl_misc = {
+	.minor		= MISC_DYNAMIC_MINOR,
+	.name		= "blksnap-control",
+	.fops		= &blksnap_ctrl_fops,
+};
+
+int ctrl_init(void)
+{
+	return misc_register(&blksnap_ctrl_misc);
+}
+
+void ctrl_done(void)
+{
+	misc_deregister(&blksnap_ctrl_misc);
+}
diff --git a/drivers/block/blksnap/ctrl.h b/drivers/block/blksnap/ctrl.h
index ade3f1cf57e90c..88f4b05296e575 100644
--- a/drivers/block/blksnap/ctrl.h
+++ b/drivers/block/blksnap/ctrl.h
@@ -2,8 +2,6 @@
 #ifndef __BLK_SNAP_CTRL_H
 #define __BLK_SNAP_CTRL_H
 
-int get_blk_snap_major(void);
-
 int ctrl_init(void);
 void ctrl_done(void);
 #endif /* __BLK_SNAP_CTRL_H */
diff --git a/drivers/block/blksnap/main.c b/drivers/block/blksnap/main.c
index a7939efc6497b9..f8b6f5c406ed7c 100644
--- a/drivers/block/blksnap/main.c
+++ b/drivers/block/blksnap/main.c
@@ -6,7 +6,6 @@
 #include "version.h"
 #include "params.h"
 #include "ctrl.h"
-#include "sysfs.h"
 #include "snapimage.h"
 #include "snapshot.h"
 #include "tracker.h"
@@ -44,8 +43,6 @@ static int __init blk_snap_init(void)
 	result = ctrl_init();
 	if (result)
 		return result;
-
-	result = sysfs_initialize();
 	return result;
 }
 
@@ -53,7 +50,6 @@ static void __exit blk_snap_exit(void)
 {
 	pr_info("Unloading module\n");
 
-	sysfs_finalize();
 	ctrl_done();
 
 	diff_io_done();
diff --git a/drivers/block/blksnap/sysfs.c b/drivers/block/blksnap/sysfs.c
deleted file mode 100644
index 6f53c4217d6c7b..00000000000000
--- a/drivers/block/blksnap/sysfs.c
+++ /dev/null
@@ -1,80 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#define pr_fmt(fmt) KBUILD_MODNAME "-sysfs: " fmt
-
-#include <linux/module.h>
-#include <linux/blkdev.h>
-#include <linux/sysfs.h>
-#include <linux/device.h>
-#include <uapi/linux/blksnap.h>
-#include "sysfs.h"
-#include "ctrl.h"
-
-static ssize_t major_show(struct class *class, struct class_attribute *attr,
-			  char *buf)
-{
-	sprintf(buf, "%d", get_blk_snap_major());
-	return strlen(buf);
-}
-
-/* Declare class_attr_major */
-CLASS_ATTR_RO(major);
-
-static struct class *blk_snap_class;
-
-static struct device *blk_snap_device;
-
-int sysfs_initialize(void)
-{
-	struct device *dev;
-	int res;
-
-	blk_snap_class = class_create(THIS_MODULE, THIS_MODULE->name);
-	if (IS_ERR(blk_snap_class)) {
-		res = PTR_ERR(blk_snap_class);
-
-		pr_err("Bad class create. errno=%d\n", abs(res));
-		return res;
-	}
-
-	pr_info("Create 'major' sysfs attribute\n");
-	res = class_create_file(blk_snap_class, &class_attr_major);
-	if (res) {
-		pr_err("Failed to create 'major' sysfs file\n");
-
-		class_destroy(blk_snap_class);
-		blk_snap_class = NULL;
-		return res;
-	}
-
-	dev = device_create(blk_snap_class, NULL,
-			    MKDEV(get_blk_snap_major(), 0), NULL,
-			    THIS_MODULE->name);
-	if (IS_ERR(dev)) {
-		res = PTR_ERR(dev);
-		pr_err("Failed to create device, errno=%d\n", abs(res));
-
-		class_remove_file(blk_snap_class, &class_attr_major);
-		class_destroy(blk_snap_class);
-		blk_snap_class = NULL;
-		return res;
-	}
-
-	blk_snap_device = dev;
-	return res;
-}
-
-void sysfs_finalize(void)
-{
-	pr_info("Cleanup sysfs\n");
-
-	if (blk_snap_device) {
-		device_unregister(blk_snap_device);
-		blk_snap_device = NULL;
-	}
-
-	if (blk_snap_class != NULL) {
-		class_remove_file(blk_snap_class, &class_attr_major);
-		class_destroy(blk_snap_class);
-		blk_snap_class = NULL;
-	}
-}
diff --git a/drivers/block/blksnap/sysfs.h b/drivers/block/blksnap/sysfs.h
deleted file mode 100644
index 5fc200d3678964..00000000000000
--- a/drivers/block/blksnap/sysfs.h
+++ /dev/null
@@ -1,7 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __BLK_SNAP_SYSFS_H
-#define __BLK_SNAP_SYSFS_H
-
-int sysfs_initialize(void);
-void sysfs_finalize(void);
-#endif /* __BLK_SNAP_SYSFS_H */

  reply	other threads:[~2022-12-15  9:29 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-09 14:23 [PATCH v2 00/21] blksnap - block devices snapshots module Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 01/21] documentation, blkfilter: Block Device Filtering Mechanism Sergei Shtepa
2022-12-10  4:15   ` Bagas Sanjaya
2022-12-09 14:23 ` [PATCH v2 02/21] block, " Sergei Shtepa
2022-12-15  9:26   ` Christoph Hellwig
2022-12-15 10:46     ` Sergei Shtepa
2022-12-16  7:04       ` Christoph Hellwig
2023-01-31 23:58   ` Mike Snitzer
2023-02-01 11:09     ` Fabio Fantoni
2023-02-01 13:16     ` Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 03/21] documentation, capability: fix Generic Block Device Capability Sergei Shtepa
2022-12-13 12:13   ` Fabio Fantoni
2022-12-30 15:35     ` Fabio Fantoni
2022-12-09 14:23 ` [PATCH v2 04/21] documentation, blksnap: Block Devices Snapshots Module Sergei Shtepa
2022-12-10  3:50   ` Bagas Sanjaya
2022-12-09 14:23 ` [PATCH v2 05/21] block, blksnap: header file of the module interface Sergei Shtepa
2022-12-09 22:13   ` kernel test robot
2022-12-09 23:14   ` kernel test robot
2022-12-09 14:23 ` [PATCH v2 06/21] block, blksnap: module management interface functions Sergei Shtepa
2022-12-15  9:28   ` Christoph Hellwig [this message]
     [not found]   ` <CGME20230103153406eucas1p205c48bd767e6a86f6f1121db7eb5fc19@eucas1p2.samsung.com>
2023-01-03 15:26     ` Pankaj Raghav
2022-12-09 14:23 ` [PATCH v2 07/21] block, blksnap: init() and exit() functions Sergei Shtepa
2022-12-15  9:30   ` Christoph Hellwig
2022-12-09 14:23 ` [PATCH v2 08/21] block, blksnap: interaction with sysfs Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 09/21] block, blksnap: attaching and detaching the filter and handling I/O units Sergei Shtepa
2022-12-15 10:01   ` Christoph Hellwig
2022-12-09 14:23 ` [PATCH v2 10/21] block, blksnap: map of change block tracking Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 11/21] block, blksnap: minimum data storage unit of the original block device Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 12/21] block, blksnap: buffer in memory for the minimum data storage unit Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 13/21] block, blksnap: functions and structures for performing block I/O operations Sergei Shtepa
2022-12-15 10:06   ` Christoph Hellwig
2022-12-09 14:23 ` [PATCH v2 14/21] block, blksnap: storage for storing difference blocks Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 15/21] block, blksnap: event queue from the difference storage Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 16/21] block, blksnap: owner of information about overwritten blocks of the original block device Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 17/21] block, blksnap: snapshot image " Sergei Shtepa
2022-12-15  9:45   ` Christoph Hellwig
2022-12-09 14:23 ` [PATCH v2 18/21] block, blksnap: snapshot Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 19/21] block, blksnap: Kconfig and Makefile Sergei Shtepa
2022-12-09 14:23 ` [PATCH v2 20/21] block, blksnap: adds a blksnap to the kernel tree Sergei Shtepa
2022-12-09 21:53   ` kernel test robot
2022-12-09 14:23 ` [PATCH v2 21/21] block, blksnap: adds a maintainer for new files Sergei Shtepa
2022-12-10  3:23 ` [PATCH v2 00/21] blksnap - block devices snapshots module Bagas Sanjaya
2022-12-10 22:57   ` Sergei Shtepa
     [not found] ` <20230101071813.3329-1-hdanton@sina.com>
2023-01-02  9:44   ` [PATCH v2 17/21] block, blksnap: snapshot image block device Sergei Shtepa
     [not found] ` <20230101110542.3395-1-hdanton@sina.com>
2023-01-02  9:58   ` [PATCH v2 18/21] block, blksnap: snapshot Sergei Shtepa
2023-01-17 21:04 ` [PATCH v2 00/21] blksnap - block devices snapshots module Mike Snitzer
2023-01-18 10:51   ` Hannes Reinecke
2023-01-24 11:34   ` Sergei Shtepa
2023-01-31 20:47     ` Mike Snitzer
2023-02-01  2:32       ` Mason Giles

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=Y5ro2qv8pxGQ5bTN@infradead.org \
    --to=hch@infradead.org \
    --cc=axboe@kernel.dk \
    --cc=corbet@lwn.net \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sergei.shtepa@veeam.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).