linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-next 0/8] update for rnbd
@ 2020-11-26 10:47 Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 1/8] block/rnbd-clt: Make path parameter optional for map_device Jack Wang
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block; +Cc: axboe, hch, sagi, bvanassche, danil.kipnis

Hi Jens,

Please consider to include following changes to next merge window.

Bugfix:
- fix memleak when kobject_init_and_add fails.

features:
- rnbd-clt to support mapping two devices with same name from
different servers, and documentation
- rnbd-srv: force_close devices from one client and documentation.

misc:
- rnbd-clt: make path parameter optional
- rnbd-clt: dynamically alloc buffer to reduce memory footprint.

Thanks!
Jack Wang.

Gioh Kim (2):
  Documentation/ABI/rnbd-clt: fix typo in sysfs-class-rnbd-client
  Documentation/ABI/rnbd-clt: session name is appended to the device
    path

Guoqing Jiang (2):
  block/rnbd-clt: support mapping two devices with the same name from
    different servers
  block/rnbd: call kobject_put in the failure path

Jack Wang (1):
  Documentation/ABI/rnbd-srv: add document for force_close

Lutz Pogrell (1):
  block/rnbd-srv: close a mapped device from server side.

Md Haris Iqbal (2):
  block/rnbd-clt: Make path parameter optional for map_device
  block/rnbd-clt: Dynamically alloc buffer for pathname &
    blk_symlink_name

 .../ABI/testing/sysfs-class-rnbd-client       |  8 +--
 .../ABI/testing/sysfs-class-rnbd-server       |  8 +++
 drivers/block/rnbd/rnbd-clt-sysfs.c           | 21 ++++--
 drivers/block/rnbd/rnbd-clt.c                 | 33 +++++++---
 drivers/block/rnbd/rnbd-clt.h                 |  4 +-
 drivers/block/rnbd/rnbd-srv-sysfs.c           | 66 +++++++++++++++----
 drivers/block/rnbd/rnbd-srv.c                 | 19 +++++-
 drivers/block/rnbd/rnbd-srv.h                 |  4 +-
 8 files changed, 129 insertions(+), 34 deletions(-)

-- 
2.25.1


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

* [PATCH for-next 1/8] block/rnbd-clt: Make path parameter optional for map_device
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 2/8] block/rnbd-clt: support mapping two devices with the same name from different servers Jack Wang
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block; +Cc: axboe, hch, sagi, bvanassche, danil.kipnis, Md Haris Iqbal

From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>

During map_device if the given session exists, then the path parameter is
not used. In such a case, the path parameter is redundant.

This commit makes the path parameter optional for map_device. When the
path parameter is not given, if the session exists then that is used to
establish the rtrs connection.

If the session does not exist, and the path parameter is also missing,
then map_device fails.

Signed-off-by: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/block/rnbd/rnbd-clt-sysfs.c | 1 -
 drivers/block/rnbd/rnbd-clt.c       | 6 ++++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
index 4f4474eecadb..e7b41ec7cd6a 100644
--- a/drivers/block/rnbd/rnbd-clt-sysfs.c
+++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
@@ -37,7 +37,6 @@ enum {
 };
 
 static const unsigned int rnbd_opt_mandatory[] = {
-	RNBD_OPT_PATH,
 	RNBD_OPT_DEV_PATH,
 	RNBD_OPT_SESSNAME,
 };
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index 8b2411ccbda9..edefa0761a81 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -1193,6 +1193,12 @@ find_and_get_or_create_sess(const char *sessname,
 	else if (!first)
 		return sess;
 
+	if (!path_cnt) {
+		pr_err("Session %s not found, and path parameter not given", sessname);
+		err = -ENXIO;
+		goto put_sess;
+	}
+
 	rtrs_ops = (struct rtrs_clt_ops) {
 		.priv = sess,
 		.link_ev = rnbd_clt_link_ev,
-- 
2.25.1


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

* [PATCH for-next 2/8] block/rnbd-clt: support mapping two devices with the same name from different servers
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 1/8] block/rnbd-clt: Make path parameter optional for map_device Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 3/8] Documentation/ABI/rnbd-clt: fix typo in sysfs-class-rnbd-client Jack Wang
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block
  Cc: axboe, hch, sagi, bvanassche, danil.kipnis, Guoqing Jiang,
	Gioh Kim, Md Haris Iqbal

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

Previously, we can't map same device name from different sessions
due to the limitation of sysfs naming mechanism.

root@clt2:~# ls -l /sys/class/rnbd-client/ctl/devices/
total 0
lrwxrwxrwx 1 root 0 Sep  2 16:31 !dev!nullb1 -> ../../../block/rnbd0

We only use the device name in above, which caused device with
the same name can't be mapped from another server. To address
the issue, the sessname is appended to the node to differentiate
where the device comes from.

Also, we need to check if the pathname is existed in a specific
session instead of search it in global sess_list.

Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@cloud.ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/block/rnbd/rnbd-clt-sysfs.c |  4 ++++
 drivers/block/rnbd/rnbd-clt.c       | 13 ++++++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
index e7b41ec7cd6a..5d3c3c80dab4 100644
--- a/drivers/block/rnbd/rnbd-clt-sysfs.c
+++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
@@ -480,6 +480,10 @@ static int rnbd_clt_get_path_name(struct rnbd_clt_dev *dev, char *buf,
 	if (ret >= len)
 		return -ENAMETOOLONG;
 
+	ret = snprintf(buf, len, "%s@%s", buf, dev->sess->sessname);
+	if (ret >= len)
+		return -ENAMETOOLONG;
+
 	return 0;
 }
 
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index edefa0761a81..1bb495e50931 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -1410,13 +1410,16 @@ static struct rnbd_clt_dev *init_dev(struct rnbd_clt_session *sess,
 	return ERR_PTR(ret);
 }
 
-static bool __exists_dev(const char *pathname)
+static bool __exists_dev(const char *pathname, const char *sessname)
 {
 	struct rnbd_clt_session *sess;
 	struct rnbd_clt_dev *dev;
 	bool found = false;
 
 	list_for_each_entry(sess, &sess_list, list) {
+		if (sessname && strncmp(sess->sessname, sessname,
+					sizeof(sess->sessname)))
+			continue;
 		mutex_lock(&sess->lock);
 		list_for_each_entry(dev, &sess->devs_list, list) {
 			if (!strncmp(dev->pathname, pathname,
@@ -1433,12 +1436,12 @@ static bool __exists_dev(const char *pathname)
 	return found;
 }
 
-static bool exists_devpath(const char *pathname)
+static bool exists_devpath(const char *pathname, const char *sessname)
 {
 	bool found;
 
 	mutex_lock(&sess_lock);
-	found = __exists_dev(pathname);
+	found = __exists_dev(pathname, sessname);
 	mutex_unlock(&sess_lock);
 
 	return found;
@@ -1451,7 +1454,7 @@ static bool insert_dev_if_not_exists_devpath(const char *pathname,
 	bool found;
 
 	mutex_lock(&sess_lock);
-	found = __exists_dev(pathname);
+	found = __exists_dev(pathname, sess->sessname);
 	if (!found) {
 		mutex_lock(&sess->lock);
 		list_add_tail(&dev->list, &sess->devs_list);
@@ -1481,7 +1484,7 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
 	struct rnbd_clt_dev *dev;
 	int ret;
 
-	if (exists_devpath(pathname))
+	if (unlikely(exists_devpath(pathname, sessname)))
 		return ERR_PTR(-EEXIST);
 
 	sess = find_and_get_or_create_sess(sessname, paths, path_cnt, port_nr);
-- 
2.25.1


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

* [PATCH for-next 3/8] Documentation/ABI/rnbd-clt: fix typo in sysfs-class-rnbd-client
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 1/8] block/rnbd-clt: Make path parameter optional for map_device Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 2/8] block/rnbd-clt: support mapping two devices with the same name from different servers Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 4/8] Documentation/ABI/rnbd-clt: session name is appended to the device path Jack Wang
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block; +Cc: axboe, hch, sagi, bvanassche, danil.kipnis, Gioh Kim

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

/sys/block/rnbd<N> is created, not /sys/block/rnbd_client/rnbd<N>

Signed-off-by: Gioh Kim <gi-oh.kim@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 Documentation/ABI/testing/sysfs-class-rnbd-client | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-rnbd-client b/Documentation/ABI/testing/sysfs-class-rnbd-client
index 00c0286733d4..ca3267b81886 100644
--- a/Documentation/ABI/testing/sysfs-class-rnbd-client
+++ b/Documentation/ABI/testing/sysfs-class-rnbd-client
@@ -66,7 +66,7 @@ Description:	Expected format is the following::
 		    The rnbd_server prepends the <device_path> received from client
 		    with <dev_search_path> and tries to open the
 		    <dev_search_path>/<device_path> block device.  On success,
-		    a /dev/rnbd<N> device file, a /sys/block/rnbd_client/rnbd<N>/
+		    a /dev/rnbd<N> device file, a /sys/block/rnbd<N>/
 		    directory and an entry in /sys/class/rnbd-client/ctl/devices
 		    will be created.
 
-- 
2.25.1


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

* [PATCH for-next 4/8] Documentation/ABI/rnbd-clt: session name is appended to the device path
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
                   ` (2 preceding siblings ...)
  2020-11-26 10:47 ` [PATCH for-next 3/8] Documentation/ABI/rnbd-clt: fix typo in sysfs-class-rnbd-client Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 5/8] block/rnbd-srv: close a mapped device from server side Jack Wang
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block; +Cc: axboe, hch, sagi, bvanassche, danil.kipnis, Gioh Kim

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

When mapping a device,
/sys/devices/virtual/rnbd-client/ctl/devices/<device_id> was created.
But we found out that it had a problem when mapping the same file
on different servers. So we append the session name after the
device_id as below.
/sys/devices/virtual/rnbd-client/ctl/devices/<device_id>@<session_name>

Signed-off-by: Gioh Kim <gi-oh.kim@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 Documentation/ABI/testing/sysfs-class-rnbd-client | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-rnbd-client b/Documentation/ABI/testing/sysfs-class-rnbd-client
index ca3267b81886..2aa05b3e348e 100644
--- a/Documentation/ABI/testing/sysfs-class-rnbd-client
+++ b/Documentation/ABI/testing/sysfs-class-rnbd-client
@@ -95,12 +95,12 @@ Description:	Expected format is the following::
 		---------------------------------
 
 		After mapping, the device file can be found by:
-		o  The symlink /sys/class/rnbd-client/ctl/devices/<device_id>
+		o  The symlink /sys/class/rnbd-client/ctl/devices/<device_id>@<session_name>
 		points to /sys/block/<dev-name>. The last part of the symlink destination
 		is the same as the device name.  By extracting the last part of the
 		path the path to the device /dev/<dev-name> can be build.
 
-		* /dev/block/$(cat /sys/class/rnbd-client/ctl/devices/<device_id>/dev)
+		* /dev/block/$(cat /sys/class/rnbd-client/ctl/devices/<device_id>@<session_name>/dev)
 
 		How to find the <device_id> of the device is described on the next
 		section.
@@ -110,7 +110,7 @@ Date:		Feb 2020
 KernelVersion:	5.7
 Contact:	Jack Wang <jinpu.wang@cloud.ionos.com> Danil Kipnis <danil.kipnis@cloud.ionos.com>
 Description:	For each device mapped on the client a new symbolic link is created as
-		/sys/class/rnbd-client/ctl/devices/<device_id>, which points
+		/sys/class/rnbd-client/ctl/devices/<device_id>@<session_name>, which points
 		to the block device created by rnbd (/sys/block/rnbd<N>/).
 		The <device_id> of each device is created as follows:
 
-- 
2.25.1


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

* [PATCH for-next 5/8] block/rnbd-srv: close a mapped device from server side.
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
                   ` (3 preceding siblings ...)
  2020-11-26 10:47 ` [PATCH for-next 4/8] Documentation/ABI/rnbd-clt: session name is appended to the device path Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 6/8] Documentation/ABI/rnbd-srv: add document for force_close Jack Wang
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block
  Cc: axboe, hch, sagi, bvanassche, danil.kipnis, Lutz Pogrell, Gioh Kim

From: Lutz Pogrell <lutz.pogrell@cloud.ionos.com>

The forceful close of an exported device is required
for the use case, when the client side hangs, is crashed,
or is not accessible.

There have been cases observed, where only some of
the devices are to be cleaned up, but the session shall
remain.

When the device is to be exported to a different
client host, server side cleanup is required.

Signed-off-by: Lutz Pogrell <lutz.pogrell@cloud.ionos.com>
Reviewed-by: Gioh Kim <gi-oh.kim@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/block/rnbd/rnbd-srv-sysfs.c | 38 ++++++++++++++++++++++++++++-
 drivers/block/rnbd/rnbd-srv.c       | 19 +++++++++++++--
 drivers/block/rnbd/rnbd-srv.h       |  4 ++-
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
index 106775c074d1..08ffb492ebfa 100644
--- a/drivers/block/rnbd/rnbd-srv-sysfs.c
+++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
@@ -120,10 +120,46 @@ static ssize_t mapping_path_show(struct kobject *kobj,
 static struct kobj_attribute rnbd_srv_dev_session_mapping_path_attr =
 	__ATTR_RO(mapping_path);
 
+static ssize_t rnbd_srv_dev_session_force_close_show(struct kobject *kobj,
+					struct kobj_attribute *attr, char *page)
+{
+	return scnprintf(page, PAGE_SIZE, "Usage: echo 1 > %s\n",
+			 attr->attr.name);
+}
+
+static ssize_t rnbd_srv_dev_session_force_close_store(struct kobject *kobj,
+					struct kobj_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct rnbd_srv_sess_dev *sess_dev;
+
+	sess_dev = container_of(kobj, struct rnbd_srv_sess_dev, kobj);
+
+	if (!sysfs_streq(buf, "1")) {
+		rnbd_srv_err(sess_dev, "%s: invalid value: '%s'\n",
+			      attr->attr.name, buf);
+		return -EINVAL;
+	}
+
+	rnbd_srv_info(sess_dev, "force close requested\n");
+
+	/* first remove sysfs itself to avoid deadlock */
+	sysfs_remove_file_self(&sess_dev->kobj, &attr->attr);
+	rnbd_srv_sess_dev_force_close(sess_dev);
+
+	return count;
+}
+
+static struct kobj_attribute rnbd_srv_dev_session_force_close_attr =
+	__ATTR(force_close, 0644,
+	       rnbd_srv_dev_session_force_close_show,
+	       rnbd_srv_dev_session_force_close_store);
+
 static struct attribute *rnbd_srv_default_dev_sessions_attrs[] = {
 	&rnbd_srv_dev_session_access_mode_attr.attr,
 	&rnbd_srv_dev_session_ro_attr.attr,
 	&rnbd_srv_dev_session_mapping_path_attr.attr,
+	&rnbd_srv_dev_session_force_close_attr.attr,
 	NULL,
 };
 
@@ -145,7 +181,7 @@ static void rnbd_srv_sess_dev_release(struct kobject *kobj)
 	struct rnbd_srv_sess_dev *sess_dev;
 
 	sess_dev = container_of(kobj, struct rnbd_srv_sess_dev, kobj);
-	rnbd_destroy_sess_dev(sess_dev);
+	rnbd_destroy_sess_dev(sess_dev, sess_dev->keep_id);
 }
 
 static struct kobj_type rnbd_srv_sess_dev_ktype = {
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index e1bc8b4cd592..d1ee72ed8384 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -212,12 +212,20 @@ static void rnbd_put_srv_dev(struct rnbd_srv_dev *dev)
 	kref_put(&dev->kref, destroy_device_cb);
 }
 
-void rnbd_destroy_sess_dev(struct rnbd_srv_sess_dev *sess_dev)
+void rnbd_destroy_sess_dev(struct rnbd_srv_sess_dev *sess_dev, bool keep_id)
 {
 	DECLARE_COMPLETION_ONSTACK(dc);
 
-	xa_erase(&sess_dev->sess->index_idr, sess_dev->device_id);
+	if (keep_id)
+		/* free the resources for the id but don't  */
+		/* allow to re-use the id itself because it */
+		/* is still used by the client              */
+		xa_cmpxchg(&sess_dev->sess->index_idr, sess_dev->device_id,
+			   sess_dev, NULL, 0);
+	else
+		xa_erase(&sess_dev->sess->index_idr, sess_dev->device_id);
 	synchronize_rcu();
+
 	sess_dev->destroy_comp = &dc;
 	rnbd_put_sess_dev(sess_dev);
 	wait_for_completion(&dc); /* wait for inflights to drop to zero */
@@ -328,6 +336,13 @@ static int rnbd_srv_link_ev(struct rtrs_srv *rtrs,
 	}
 }
 
+void rnbd_srv_sess_dev_force_close(struct rnbd_srv_sess_dev *sess_dev)
+{
+	rnbd_srv_destroy_dev_session_sysfs(sess_dev);
+	sess_dev->keep_id = true;
+
+}
+
 static int process_msg_close(struct rtrs_srv *rtrs,
 			     struct rnbd_srv_session *srv_sess,
 			     void *data, size_t datalen, const void *usr,
diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
index 5a8544b5e74f..b157371c25ed 100644
--- a/drivers/block/rnbd/rnbd-srv.h
+++ b/drivers/block/rnbd/rnbd-srv.h
@@ -56,6 +56,7 @@ struct rnbd_srv_sess_dev {
 	struct rnbd_srv_dev		*dev;
 	struct kobject                  kobj;
 	u32                             device_id;
+	bool				keep_id;
 	fmode_t                         open_flags;
 	struct kref			kref;
 	struct completion               *destroy_comp;
@@ -63,6 +64,7 @@ struct rnbd_srv_sess_dev {
 	enum rnbd_access_mode		access_mode;
 };
 
+void rnbd_srv_sess_dev_force_close(struct rnbd_srv_sess_dev *sess_dev);
 /* rnbd-srv-sysfs.c */
 
 int rnbd_srv_create_dev_sysfs(struct rnbd_srv_dev *dev,
@@ -73,6 +75,6 @@ int rnbd_srv_create_dev_session_sysfs(struct rnbd_srv_sess_dev *sess_dev);
 void rnbd_srv_destroy_dev_session_sysfs(struct rnbd_srv_sess_dev *sess_dev);
 int rnbd_srv_create_sysfs_files(void);
 void rnbd_srv_destroy_sysfs_files(void);
-void rnbd_destroy_sess_dev(struct rnbd_srv_sess_dev *sess_dev);
+void rnbd_destroy_sess_dev(struct rnbd_srv_sess_dev *sess_dev, bool keep_id);
 
 #endif /* RNBD_SRV_H */
-- 
2.25.1


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

* [PATCH for-next 6/8] Documentation/ABI/rnbd-srv: add document for force_close
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
                   ` (4 preceding siblings ...)
  2020-11-26 10:47 ` [PATCH for-next 5/8] block/rnbd-srv: close a mapped device from server side Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 7/8] block/rnbd: call kobject_put in the failure path Jack Wang
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block; +Cc: axboe, hch, sagi, bvanassche, danil.kipnis

describe force_close of device

Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 Documentation/ABI/testing/sysfs-class-rnbd-server | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-rnbd-server b/Documentation/ABI/testing/sysfs-class-rnbd-server
index ba60a90c0e45..6c5996cd7cfb 100644
--- a/Documentation/ABI/testing/sysfs-class-rnbd-server
+++ b/Documentation/ABI/testing/sysfs-class-rnbd-server
@@ -48,3 +48,11 @@ Date:		Feb 2020
 KernelVersion:	5.7
 Contact:	Jack Wang <jinpu.wang@cloud.ionos.com> Danil Kipnis <danil.kipnis@cloud.ionos.com>
 Description:	Contains the device access mode: ro, rw or migration.
+
+What:		/sys/class/rnbd-server/ctl/devices/<device_name>/sessions/<session-name>/force_close
+Date:		Nov 2020
+KernelVersion:	5.10
+Contact:	Jack Wang <jinpu.wang@cloud.ionos.com> Danil Kipnis <danil.kipnis@cloud.ionos.com>
+Description:	Write "1" to the file to close the device on server side. Please
+		note that the client side device will not be closed, read or
+		write to the device will get -ENOTCONN.
-- 
2.25.1


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

* [PATCH for-next 7/8] block/rnbd: call kobject_put in the failure path
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
                   ` (5 preceding siblings ...)
  2020-11-26 10:47 ` [PATCH for-next 6/8] Documentation/ABI/rnbd-srv: add document for force_close Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-11-26 10:47 ` [PATCH for-next 8/8] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name Jack Wang
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block
  Cc: axboe, hch, sagi, bvanassche, danil.kipnis, Guoqing Jiang,
	Md Haris Iqbal

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

Per the comment of kobject_init_and_add, we need to cleanup the memory
by call kobject_put.

Also we need to call kobject_del for the other failure cases if the
kobject_init_and_add doesn't fail.

Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/block/rnbd/rnbd-clt-sysfs.c |  4 +++-
 drivers/block/rnbd/rnbd-srv-sysfs.c | 28 ++++++++++++++++------------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
index 5d3c3c80dab4..e3c3270b0cee 100644
--- a/drivers/block/rnbd/rnbd-clt-sysfs.c
+++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
@@ -450,9 +450,11 @@ static int rnbd_clt_add_dev_kobj(struct rnbd_clt_dev *dev)
 
 	ret = kobject_init_and_add(&dev->kobj, &rnbd_dev_ktype, gd_kobj, "%s",
 				   "rnbd");
-	if (ret)
+	if (ret) {
 		rnbd_clt_err(dev, "Failed to create device sysfs dir, err: %d\n",
 			      ret);
+		kobject_put(&dev->kobj);
+	}
 
 	return ret;
 }
diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
index 08ffb492ebfa..05ffe488ddc6 100644
--- a/drivers/block/rnbd/rnbd-srv-sysfs.c
+++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
@@ -47,13 +47,17 @@ int rnbd_srv_create_dev_sysfs(struct rnbd_srv_dev *dev,
 
 	ret = kobject_init_and_add(&dev->dev_kobj, &dev_ktype,
 				   rnbd_devs_kobj, dev_name);
-	if (ret)
+	if (ret) {
+		kobject_put(&dev->dev_kobj);
 		return ret;
+	}
 
 	dev->dev_sessions_kobj = kobject_create_and_add("sessions",
 							&dev->dev_kobj);
-	if (!dev->dev_sessions_kobj)
-		goto put_dev_kobj;
+	if (!dev->dev_sessions_kobj) {
+		ret = -ENOMEM;
+		goto free_dev_kobj;
+	}
 
 	bdev_kobj = &disk_to_dev(bdev->bd_disk)->kobj;
 	ret = sysfs_create_link(&dev->dev_kobj, bdev_kobj, "block_dev");
@@ -64,7 +68,8 @@ int rnbd_srv_create_dev_sysfs(struct rnbd_srv_dev *dev,
 
 put_sess_kobj:
 	kobject_put(dev->dev_sessions_kobj);
-put_dev_kobj:
+free_dev_kobj:
+	kobject_del(&dev->dev_kobj);
 	kobject_put(&dev->dev_kobj);
 	return ret;
 }
@@ -196,18 +201,17 @@ int rnbd_srv_create_dev_session_sysfs(struct rnbd_srv_sess_dev *sess_dev)
 	ret = kobject_init_and_add(&sess_dev->kobj, &rnbd_srv_sess_dev_ktype,
 				   sess_dev->dev->dev_sessions_kobj, "%s",
 				   sess_dev->sess->sessname);
-	if (ret)
+	if (ret) {
+		kobject_put(&sess_dev->kobj);
 		return ret;
+	}
 
 	ret = sysfs_create_group(&sess_dev->kobj,
 				 &rnbd_srv_default_dev_session_attr_group);
-	if (ret)
-		goto err;
-
-	return 0;
-
-err:
-	kobject_put(&sess_dev->kobj);
+	if (ret) {
+		kobject_del(&sess_dev->kobj);
+		kobject_put(&sess_dev->kobj);
+	}
 
 	return ret;
 }
-- 
2.25.1


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

* [PATCH for-next 8/8] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
                   ` (6 preceding siblings ...)
  2020-11-26 10:47 ` [PATCH for-next 7/8] block/rnbd: call kobject_put in the failure path Jack Wang
@ 2020-11-26 10:47 ` Jack Wang
  2020-12-04 10:15 ` [PATCH for-next 0/8] update for rnbd Jinpu Wang
  2020-12-04 16:41 ` Jens Axboe
  9 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2020-11-26 10:47 UTC (permalink / raw)
  To: linux-block
  Cc: axboe, hch, sagi, bvanassche, danil.kipnis, Md Haris Iqbal, Lutz Pogrell

From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>

For every rnbd_clt_dev, we alloc the pathname and blk_symlink_name
statically to NAME_MAX which is 255 bytes. In most of the cases we only
need less than 10 bytes, so 500 bytes per block device are wasted.

This commit dynamically allocates memory buffer for pathname and
blk_symlink_name.

Signed-off-by: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
Reviewed-by: Lutz Pogrell <lutz.pogrell@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/block/rnbd/rnbd-clt-sysfs.c | 12 ++++++++++--
 drivers/block/rnbd/rnbd-clt.c       | 14 +++++++++++---
 drivers/block/rnbd/rnbd-clt.h       |  4 ++--
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
index e3c3270b0cee..c3c96a567568 100644
--- a/drivers/block/rnbd/rnbd-clt-sysfs.c
+++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
@@ -434,6 +434,7 @@ void rnbd_clt_remove_dev_symlink(struct rnbd_clt_dev *dev)
 	 */
 	if (strlen(dev->blk_symlink_name) && try_module_get(THIS_MODULE)) {
 		sysfs_remove_link(rnbd_devs_kobj, dev->blk_symlink_name);
+		kfree(dev->blk_symlink_name);
 		module_put(THIS_MODULE);
 	}
 }
@@ -492,10 +493,17 @@ static int rnbd_clt_get_path_name(struct rnbd_clt_dev *dev, char *buf,
 static int rnbd_clt_add_dev_symlink(struct rnbd_clt_dev *dev)
 {
 	struct kobject *gd_kobj = &disk_to_dev(dev->gd)->kobj;
-	int ret;
+	int ret, len;
+
+	len = strlen(dev->pathname) + strlen(dev->sess->sessname) + 2;
+	dev->blk_symlink_name = kzalloc(len, GFP_KERNEL);
+	if (!dev->blk_symlink_name) {
+		rnbd_clt_err(dev, "Failed to allocate memory for blk_symlink_name\n");
+		goto out_err;
+	}
 
 	ret = rnbd_clt_get_path_name(dev, dev->blk_symlink_name,
-				      sizeof(dev->blk_symlink_name));
+				      len);
 	if (ret) {
 		rnbd_clt_err(dev, "Failed to get /sys/block symlink path, err: %d\n",
 			      ret);
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index 1bb495e50931..34bc6083b58d 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -59,6 +59,7 @@ static void rnbd_clt_put_dev(struct rnbd_clt_dev *dev)
 	ida_simple_remove(&index_ida, dev->clt_device_id);
 	mutex_unlock(&ida_lock);
 	kfree(dev->hw_queues);
+	kfree(dev->pathname);
 	rnbd_clt_put_sess(dev->sess);
 	mutex_destroy(&dev->lock);
 	kfree(dev);
@@ -1387,10 +1388,17 @@ static struct rnbd_clt_dev *init_dev(struct rnbd_clt_session *sess,
 		       pathname, sess->sessname, ret);
 		goto out_queues;
 	}
+
+	dev->pathname = kzalloc(strlen(pathname) + 1, GFP_KERNEL);
+	if (!dev->pathname) {
+		ret = -ENOMEM;
+		goto out_queues;
+	}
+	strlcpy(dev->pathname, pathname, strlen(pathname) + 1);
+
 	dev->clt_device_id	= ret;
 	dev->sess		= sess;
 	dev->access_mode	= access_mode;
-	strlcpy(dev->pathname, pathname, sizeof(dev->pathname));
 	mutex_init(&dev->lock);
 	refcount_set(&dev->refcount, 1);
 	dev->dev_state = DEV_STATE_INIT;
@@ -1422,8 +1430,8 @@ static bool __exists_dev(const char *pathname, const char *sessname)
 			continue;
 		mutex_lock(&sess->lock);
 		list_for_each_entry(dev, &sess->devs_list, list) {
-			if (!strncmp(dev->pathname, pathname,
-				     sizeof(dev->pathname))) {
+			if (strlen(dev->pathname) == strlen(pathname) &&
+			    !strcmp(dev->pathname, pathname)) {
 				found = true;
 				break;
 			}
diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
index ed33654aa486..b193d5904050 100644
--- a/drivers/block/rnbd/rnbd-clt.h
+++ b/drivers/block/rnbd/rnbd-clt.h
@@ -108,7 +108,7 @@ struct rnbd_clt_dev {
 	u32			clt_device_id;
 	struct mutex		lock;
 	enum rnbd_clt_dev_state	dev_state;
-	char			pathname[NAME_MAX];
+	char			*pathname;
 	enum rnbd_access_mode	access_mode;
 	bool			read_only;
 	bool			rotational;
@@ -126,7 +126,7 @@ struct rnbd_clt_dev {
 	struct list_head        list;
 	struct gendisk		*gd;
 	struct kobject		kobj;
-	char			blk_symlink_name[NAME_MAX];
+	char			*blk_symlink_name;
 	refcount_t		refcount;
 	struct work_struct	unmap_on_rmmod_work;
 };
-- 
2.25.1


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

* Re: [PATCH for-next 0/8] update for rnbd
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
                   ` (7 preceding siblings ...)
  2020-11-26 10:47 ` [PATCH for-next 8/8] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name Jack Wang
@ 2020-12-04 10:15 ` Jinpu Wang
  2020-12-04 16:41 ` Jens Axboe
  9 siblings, 0 replies; 11+ messages in thread
From: Jinpu Wang @ 2020-12-04 10:15 UTC (permalink / raw)
  To: linux-block
  Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Bart Van Assche,
	Danil Kipnis

On Thu, Nov 26, 2020 at 11:47 AM Jack Wang <jinpu.wang@cloud.ionos.com> wrote:
>
> Hi Jens,
>
> Please consider to include following changes to next merge window.

Hi Jens!

ping :)

Thanks!

>
> Bugfix:
> - fix memleak when kobject_init_and_add fails.
>
> features:
> - rnbd-clt to support mapping two devices with same name from
> different servers, and documentation
> - rnbd-srv: force_close devices from one client and documentation.
>
> misc:
> - rnbd-clt: make path parameter optional
> - rnbd-clt: dynamically alloc buffer to reduce memory footprint.
>
> Thanks!
> Jack Wang.
>
> Gioh Kim (2):
>   Documentation/ABI/rnbd-clt: fix typo in sysfs-class-rnbd-client
>   Documentation/ABI/rnbd-clt: session name is appended to the device
>     path
>
> Guoqing Jiang (2):
>   block/rnbd-clt: support mapping two devices with the same name from
>     different servers
>   block/rnbd: call kobject_put in the failure path
>
> Jack Wang (1):
>   Documentation/ABI/rnbd-srv: add document for force_close
>
> Lutz Pogrell (1):
>   block/rnbd-srv: close a mapped device from server side.
>
> Md Haris Iqbal (2):
>   block/rnbd-clt: Make path parameter optional for map_device
>   block/rnbd-clt: Dynamically alloc buffer for pathname &
>     blk_symlink_name
>
>  .../ABI/testing/sysfs-class-rnbd-client       |  8 +--
>  .../ABI/testing/sysfs-class-rnbd-server       |  8 +++
>  drivers/block/rnbd/rnbd-clt-sysfs.c           | 21 ++++--
>  drivers/block/rnbd/rnbd-clt.c                 | 33 +++++++---
>  drivers/block/rnbd/rnbd-clt.h                 |  4 +-
>  drivers/block/rnbd/rnbd-srv-sysfs.c           | 66 +++++++++++++++----
>  drivers/block/rnbd/rnbd-srv.c                 | 19 +++++-
>  drivers/block/rnbd/rnbd-srv.h                 |  4 +-
>  8 files changed, 129 insertions(+), 34 deletions(-)
>
> --
> 2.25.1
>

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

* Re: [PATCH for-next 0/8] update for rnbd
  2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
                   ` (8 preceding siblings ...)
  2020-12-04 10:15 ` [PATCH for-next 0/8] update for rnbd Jinpu Wang
@ 2020-12-04 16:41 ` Jens Axboe
  9 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2020-12-04 16:41 UTC (permalink / raw)
  To: Jack Wang, linux-block; +Cc: hch, sagi, bvanassche, danil.kipnis

On 11/26/20 3:47 AM, Jack Wang wrote:
> Hi Jens,
> 
> Please consider to include following changes to next merge window.
> 
> Bugfix:
> - fix memleak when kobject_init_and_add fails.
> 
> features:
> - rnbd-clt to support mapping two devices with same name from
> different servers, and documentation
> - rnbd-srv: force_close devices from one client and documentation.
> 
> misc:
> - rnbd-clt: make path parameter optional
> - rnbd-clt: dynamically alloc buffer to reduce memory footprint.

Applied, thanks.

-- 
Jens Axboe


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

end of thread, other threads:[~2020-12-04 16:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 10:47 [PATCH for-next 0/8] update for rnbd Jack Wang
2020-11-26 10:47 ` [PATCH for-next 1/8] block/rnbd-clt: Make path parameter optional for map_device Jack Wang
2020-11-26 10:47 ` [PATCH for-next 2/8] block/rnbd-clt: support mapping two devices with the same name from different servers Jack Wang
2020-11-26 10:47 ` [PATCH for-next 3/8] Documentation/ABI/rnbd-clt: fix typo in sysfs-class-rnbd-client Jack Wang
2020-11-26 10:47 ` [PATCH for-next 4/8] Documentation/ABI/rnbd-clt: session name is appended to the device path Jack Wang
2020-11-26 10:47 ` [PATCH for-next 5/8] block/rnbd-srv: close a mapped device from server side Jack Wang
2020-11-26 10:47 ` [PATCH for-next 6/8] Documentation/ABI/rnbd-srv: add document for force_close Jack Wang
2020-11-26 10:47 ` [PATCH for-next 7/8] block/rnbd: call kobject_put in the failure path Jack Wang
2020-11-26 10:47 ` [PATCH for-next 8/8] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name Jack Wang
2020-12-04 10:15 ` [PATCH for-next 0/8] update for rnbd Jinpu Wang
2020-12-04 16:41 ` Jens Axboe

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).