* [PATCH 00/10] misc patches for rnbd
@ 2023-05-23 7:53 Guoqing Jiang
2023-05-23 7:53 ` [PATCH 01/10] block/rnbd: kill rnbd_flags_supported Guoqing Jiang
` (9 more replies)
0 siblings, 10 replies; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Hi,
This series mostly do cleanup and other trival things, pls review.
Thanks,
Guoqing
Guoqing Jiang (10):
block/rnbd: kill rnbd_flags_supported
block/rnbd-srv: remove unused header
block/rnbd: introduce rnbd_access_modes
block/rnbd-srv: no need to check sess_dev
block/rnbd-srv: defer the allocation of rnbd_io_private
block/rnbd-srv: rename one member in rnbd_srv_dev
block/rnbd-srv: init ret with 0 instead of -EPERM
block/rnbd-srv: init err earlier in rnbd_srv_init_module
block/rnbd-srv: make process_msg_sess_info returns void
block/rnbd: change device's name
drivers/block/rnbd/Makefile | 6 ++--
drivers/block/rnbd/rnbd-clt-sysfs.c | 6 ++--
drivers/block/rnbd/rnbd-common.c | 23 ------------
drivers/block/rnbd/rnbd-proto.h | 31 +++++-----------
drivers/block/rnbd/rnbd-srv-sysfs.c | 5 ++-
drivers/block/rnbd/rnbd-srv.c | 56 ++++++++++++-----------------
drivers/block/rnbd/rnbd-srv.h | 2 +-
7 files changed, 40 insertions(+), 89 deletions(-)
delete mode 100644 drivers/block/rnbd/rnbd-common.c
--
2.35.3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 01/10] block/rnbd: kill rnbd_flags_supported
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:19 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 02/10] block/rnbd-srv: remove unused header Guoqing Jiang
` (8 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
This routine is not called since added. Then the two flags
(RNBD_OP_LAST and RNBD_F_ALL) can be removed too after kill
rnbd_flags_supported.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-proto.h | 22 ----------------------
1 file changed, 22 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-proto.h b/drivers/block/rnbd/rnbd-proto.h
index da1d0542d7e2..84fd69844b7d 100644
--- a/drivers/block/rnbd/rnbd-proto.h
+++ b/drivers/block/rnbd/rnbd-proto.h
@@ -185,7 +185,6 @@ struct rnbd_msg_io {
enum rnbd_io_flags {
/* Operations */
-
RNBD_OP_READ = 0,
RNBD_OP_WRITE = 1,
RNBD_OP_FLUSH = 2,
@@ -193,15 +192,9 @@ enum rnbd_io_flags {
RNBD_OP_SECURE_ERASE = 4,
RNBD_OP_WRITE_SAME = 5,
- RNBD_OP_LAST,
-
/* Flags */
-
RNBD_F_SYNC = 1<<(RNBD_OP_BITS + 0),
RNBD_F_FUA = 1<<(RNBD_OP_BITS + 1),
-
- RNBD_F_ALL = (RNBD_F_SYNC | RNBD_F_FUA)
-
};
static inline u32 rnbd_op(u32 flags)
@@ -214,21 +207,6 @@ static inline u32 rnbd_flags(u32 flags)
return flags & ~RNBD_OP_MASK;
}
-static inline bool rnbd_flags_supported(u32 flags)
-{
- u32 op;
-
- op = rnbd_op(flags);
- flags = rnbd_flags(flags);
-
- if (op >= RNBD_OP_LAST)
- return false;
- if (flags & ~RNBD_F_ALL)
- return false;
-
- return true;
-}
-
static inline blk_opf_t rnbd_to_bio_flags(u32 rnbd_opf)
{
blk_opf_t bio_opf;
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 02/10] block/rnbd-srv: remove unused header
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
2023-05-23 7:53 ` [PATCH 01/10] block/rnbd: kill rnbd_flags_supported Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:20 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 03/10] block/rnbd: introduce rnbd_access_modes Guoqing Jiang
` (7 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
No need to include it since none of macros in limits.h are
used by rnbd-srv.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-srv-sysfs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
index d5d9267e1fa5..9fe7d9e0ab63 100644
--- a/drivers/block/rnbd/rnbd-srv-sysfs.c
+++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
@@ -9,7 +9,6 @@
#undef pr_fmt
#define pr_fmt(fmt) KBUILD_MODNAME " L" __stringify(__LINE__) ": " fmt
-#include <uapi/linux/limits.h>
#include <linux/kobject.h>
#include <linux/sysfs.h>
#include <linux/stat.h>
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 03/10] block/rnbd: introduce rnbd_access_modes
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
2023-05-23 7:53 ` [PATCH 01/10] block/rnbd: kill rnbd_flags_supported Guoqing Jiang
2023-05-23 7:53 ` [PATCH 02/10] block/rnbd-srv: remove unused header Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:23 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 04/10] block/rnbd-srv: no need to check sess_dev Guoqing Jiang
` (6 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Add one new array (marked with __maybe_unused to prevent gcc warning about
"defined but not used" with W=1), then we can remove rnbd_access_mode_str
and rnbd-common.c accordingly.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/Makefile | 6 ++----
drivers/block/rnbd/rnbd-clt-sysfs.c | 4 ++--
drivers/block/rnbd/rnbd-common.c | 23 -----------------------
drivers/block/rnbd/rnbd-proto.h | 9 +++++++++
drivers/block/rnbd/rnbd-srv-sysfs.c | 2 +-
drivers/block/rnbd/rnbd-srv.c | 4 ++--
6 files changed, 16 insertions(+), 32 deletions(-)
delete mode 100644 drivers/block/rnbd/rnbd-common.c
diff --git a/drivers/block/rnbd/Makefile b/drivers/block/rnbd/Makefile
index 40b31630822c..208e5f865497 100644
--- a/drivers/block/rnbd/Makefile
+++ b/drivers/block/rnbd/Makefile
@@ -3,13 +3,11 @@
ccflags-y := -I$(srctree)/drivers/infiniband/ulp/rtrs
rnbd-client-y := rnbd-clt.o \
- rnbd-clt-sysfs.o \
- rnbd-common.o
+ rnbd-clt-sysfs.o
CFLAGS_rnbd-srv-trace.o = -I$(src)
-rnbd-server-y := rnbd-common.o \
- rnbd-srv.o \
+rnbd-server-y := rnbd-srv.o \
rnbd-srv-sysfs.o \
rnbd-srv-trace.o
diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
index 8c6087949794..a0b49a0c0bdd 100644
--- a/drivers/block/rnbd/rnbd-clt-sysfs.c
+++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
@@ -278,7 +278,7 @@ static ssize_t access_mode_show(struct kobject *kobj,
dev = container_of(kobj, struct rnbd_clt_dev, kobj);
- return sysfs_emit(page, "%s\n", rnbd_access_mode_str(dev->access_mode));
+ return sysfs_emit(page, "%s\n", rnbd_access_modes[dev->access_mode].str);
}
static struct kobj_attribute rnbd_clt_access_mode =
@@ -596,7 +596,7 @@ static ssize_t rnbd_clt_map_device_store(struct kobject *kobj,
pr_info("Mapping device %s on session %s, (access_mode: %s, nr_poll_queues: %d)\n",
pathname, sessname,
- rnbd_access_mode_str(access_mode),
+ rnbd_access_modes[access_mode].str,
nr_poll_queues);
dev = rnbd_clt_map_device(sessname, paths, path_cnt, port_nr, pathname,
diff --git a/drivers/block/rnbd/rnbd-common.c b/drivers/block/rnbd/rnbd-common.c
deleted file mode 100644
index 596c3f732403..000000000000
--- a/drivers/block/rnbd/rnbd-common.c
+++ /dev/null
@@ -1,23 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * RDMA Network Block Driver
- *
- * Copyright (c) 2014 - 2018 ProfitBricks GmbH. All rights reserved.
- * Copyright (c) 2018 - 2019 1&1 IONOS Cloud GmbH. All rights reserved.
- * Copyright (c) 2019 - 2020 1&1 IONOS SE. All rights reserved.
- */
-#include "rnbd-proto.h"
-
-const char *rnbd_access_mode_str(enum rnbd_access_mode mode)
-{
- switch (mode) {
- case RNBD_ACCESS_RO:
- return "ro";
- case RNBD_ACCESS_RW:
- return "rw";
- case RNBD_ACCESS_MIGRATION:
- return "migration";
- default:
- return "unknown";
- }
-}
diff --git a/drivers/block/rnbd/rnbd-proto.h b/drivers/block/rnbd/rnbd-proto.h
index 84fd69844b7d..185e24eaf2bf 100644
--- a/drivers/block/rnbd/rnbd-proto.h
+++ b/drivers/block/rnbd/rnbd-proto.h
@@ -61,6 +61,15 @@ enum rnbd_access_mode {
RNBD_ACCESS_MIGRATION,
};
+static const __maybe_unused struct {
+ int mode;
+ const char *str;
+} rnbd_access_modes[] = {
+ [RNBD_ACCESS_RO] = {RNBD_ACCESS_RO, "ro"},
+ [RNBD_ACCESS_RW] = {RNBD_ACCESS_RW, "rw"},
+ [RNBD_ACCESS_MIGRATION] = {RNBD_ACCESS_MIGRATION, "migration"},
+};
+
/**
* struct rnbd_msg_sess_info - initial session info from client to server
* @hdr: message header
diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
index 9fe7d9e0ab63..4962826e9639 100644
--- a/drivers/block/rnbd/rnbd-srv-sysfs.c
+++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
@@ -103,7 +103,7 @@ static ssize_t access_mode_show(struct kobject *kobj,
sess_dev = container_of(kobj, struct rnbd_srv_sess_dev, kobj);
return sysfs_emit(page, "%s\n",
- rnbd_access_mode_str(sess_dev->access_mode));
+ rnbd_access_modes[sess_dev->access_mode].str);
}
static struct kobj_attribute rnbd_srv_dev_session_access_mode_attr =
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 2cfed2e58d64..e9ef6bd4b50c 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -483,7 +483,7 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
pr_err("Mapping device '%s' for session %s with RW permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
srv_dev->id, srv_sess->sessname,
srv_dev->open_write_cnt,
- rnbd_access_mode_str(access_mode));
+ rnbd_access_modes[access_mode].str);
}
break;
case RNBD_ACCESS_MIGRATION:
@@ -494,7 +494,7 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
pr_err("Mapping device '%s' for session %s with migration permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
srv_dev->id, srv_sess->sessname,
srv_dev->open_write_cnt,
- rnbd_access_mode_str(access_mode));
+ rnbd_access_modes[access_mode].str);
}
break;
default:
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 04/10] block/rnbd-srv: no need to check sess_dev
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
` (2 preceding siblings ...)
2023-05-23 7:53 ` [PATCH 03/10] block/rnbd: introduce rnbd_access_modes Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:27 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 05/10] block/rnbd-srv: defer the allocation of rnbd_io_private Guoqing Jiang
` (5 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Check ret is enough since if sess_dev is NULL which also
implies ret should be 0.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-srv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index e9ef6bd4b50c..c4122e65b36a 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -96,7 +96,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session *srv_sess)
ret = kref_get_unless_zero(&sess_dev->kref);
rcu_read_unlock();
- if (!sess_dev || !ret)
+ if (!ret)
return ERR_PTR(-ENXIO);
return sess_dev;
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 05/10] block/rnbd-srv: defer the allocation of rnbd_io_private
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
` (3 preceding siblings ...)
2023-05-23 7:53 ` [PATCH 04/10] block/rnbd-srv: no need to check sess_dev Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:29 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 06/10] block/rnbd-srv: rename one member in rnbd_srv_dev Guoqing Jiang
` (4 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Only allocate priv after session is available.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-srv.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index c4122e65b36a..b4c880759a52 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -128,20 +128,17 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
trace_process_rdma(srv_sess, msg, id, datalen, usrlen);
- priv = kmalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
dev_id = le32_to_cpu(msg->device_id);
-
sess_dev = rnbd_get_sess_dev(dev_id, srv_sess);
if (IS_ERR(sess_dev)) {
pr_err_ratelimited("Got I/O request on session %s for unknown device id %d\n",
srv_sess->sessname, dev_id);
- err = -ENOTCONN;
- goto err;
+ return -ENOTCONN;
}
+ priv = kmalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
priv->sess_dev = sess_dev;
priv->id = id;
@@ -169,7 +166,6 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
bio_put:
bio_put(bio);
rnbd_put_sess_dev(sess_dev);
-err:
kfree(priv);
return err;
}
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 06/10] block/rnbd-srv: rename one member in rnbd_srv_dev
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
` (4 preceding siblings ...)
2023-05-23 7:53 ` [PATCH 05/10] block/rnbd-srv: defer the allocation of rnbd_io_private Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:30 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 07/10] block/rnbd-srv: init ret with 0 instead of -EPERM Guoqing Jiang
` (3 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
It actually represents the name of rnbd_srv_dev.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-srv.c | 14 +++++++-------
drivers/block/rnbd/rnbd-srv.h | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index b4c880759a52..e51eb4b7f6e6 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -176,7 +176,7 @@ static void destroy_device(struct kref *kref)
WARN_ONCE(!list_empty(&dev->sess_dev_list),
"Device %s is being destroyed but still in use!\n",
- dev->id);
+ dev->name);
spin_lock(&dev_lock);
list_del(&dev->list);
@@ -427,7 +427,7 @@ static struct rnbd_srv_dev *rnbd_srv_init_srv_dev(struct block_device *bdev)
if (!dev)
return ERR_PTR(-ENOMEM);
- snprintf(dev->id, sizeof(dev->id), "%pg", bdev);
+ snprintf(dev->name, sizeof(dev->name), "%pg", bdev);
kref_init(&dev->kref);
INIT_LIST_HEAD(&dev->sess_dev_list);
mutex_init(&dev->lock);
@@ -442,7 +442,7 @@ rnbd_srv_find_or_add_srv_dev(struct rnbd_srv_dev *new_dev)
spin_lock(&dev_lock);
list_for_each_entry(dev, &dev_list, list) {
- if (!strncmp(dev->id, new_dev->id, sizeof(dev->id))) {
+ if (!strncmp(dev->name, new_dev->name, sizeof(dev->name))) {
if (!kref_get_unless_zero(&dev->kref))
/*
* We lost the race, device is almost dead.
@@ -477,7 +477,7 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
ret = 0;
} else {
pr_err("Mapping device '%s' for session %s with RW permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
- srv_dev->id, srv_sess->sessname,
+ srv_dev->name, srv_sess->sessname,
srv_dev->open_write_cnt,
rnbd_access_modes[access_mode].str);
}
@@ -488,14 +488,14 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
ret = 0;
} else {
pr_err("Mapping device '%s' for session %s with migration permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
- srv_dev->id, srv_sess->sessname,
+ srv_dev->name, srv_sess->sessname,
srv_dev->open_write_cnt,
rnbd_access_modes[access_mode].str);
}
break;
default:
pr_err("Received mapping request for device '%s' on session %s with invalid access mode: %d\n",
- srv_dev->id, srv_sess->sessname, access_mode);
+ srv_dev->name, srv_sess->sessname, access_mode);
ret = -EINVAL;
}
@@ -770,7 +770,7 @@ static int process_msg_open(struct rnbd_srv_session *srv_sess,
list_add(&srv_sess_dev->dev_list, &srv_dev->sess_dev_list);
mutex_unlock(&srv_dev->lock);
- rnbd_srv_info(srv_sess_dev, "Opened device '%s'\n", srv_dev->id);
+ rnbd_srv_info(srv_sess_dev, "Opened device '%s'\n", srv_dev->name);
kfree(full_path);
diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
index f5962fd31d62..6b5e5ade18ae 100644
--- a/drivers/block/rnbd/rnbd-srv.h
+++ b/drivers/block/rnbd/rnbd-srv.h
@@ -35,7 +35,7 @@ struct rnbd_srv_dev {
struct kobject dev_kobj;
struct kobject *dev_sessions_kobj;
struct kref kref;
- char id[NAME_MAX];
+ char name[NAME_MAX];
/* List of rnbd_srv_sess_dev structs */
struct list_head sess_dev_list;
struct mutex lock;
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 07/10] block/rnbd-srv: init ret with 0 instead of -EPERM
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
` (5 preceding siblings ...)
2023-05-23 7:53 ` [PATCH 06/10] block/rnbd-srv: rename one member in rnbd_srv_dev Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:30 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 08/10] block/rnbd-srv: init err earlier in rnbd_srv_init_module Guoqing Jiang
` (2 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Let's always set errno after pr_err which is consistent with
default case.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-srv.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index e51eb4b7f6e6..102831c302fc 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -463,34 +463,33 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
struct rnbd_srv_session *srv_sess,
enum rnbd_access_mode access_mode)
{
- int ret = -EPERM;
+ int ret = 0;
mutex_lock(&srv_dev->lock);
switch (access_mode) {
case RNBD_ACCESS_RO:
- ret = 0;
break;
case RNBD_ACCESS_RW:
if (srv_dev->open_write_cnt == 0) {
srv_dev->open_write_cnt++;
- ret = 0;
} else {
pr_err("Mapping device '%s' for session %s with RW permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
srv_dev->name, srv_sess->sessname,
srv_dev->open_write_cnt,
rnbd_access_modes[access_mode].str);
+ ret = -EPERM;
}
break;
case RNBD_ACCESS_MIGRATION:
if (srv_dev->open_write_cnt < 2) {
srv_dev->open_write_cnt++;
- ret = 0;
} else {
pr_err("Mapping device '%s' for session %s with migration permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
srv_dev->name, srv_sess->sessname,
srv_dev->open_write_cnt,
rnbd_access_modes[access_mode].str);
+ ret = -EPERM;
}
break;
default:
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 08/10] block/rnbd-srv: init err earlier in rnbd_srv_init_module
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
` (6 preceding siblings ...)
2023-05-23 7:53 ` [PATCH 07/10] block/rnbd-srv: init ret with 0 instead of -EPERM Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:31 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 09/10] block/rnbd-srv: make process_msg_sess_info returns void Guoqing Jiang
2023-05-23 7:53 ` [PATCH 10/10] block/rnbd: change device's name Guoqing Jiang
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
With this, we can remove several lines of code.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-srv.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 102831c302fc..1fdf3366135a 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -803,7 +803,7 @@ static struct rtrs_srv_ctx *rtrs_ctx;
static struct rtrs_srv_ops rtrs_ops;
static int __init rnbd_srv_init_module(void)
{
- int err;
+ int err = 0;
BUILD_BUG_ON(sizeof(struct rnbd_msg_hdr) != 4);
BUILD_BUG_ON(sizeof(struct rnbd_msg_sess_info) != 36);
@@ -817,19 +817,17 @@ static int __init rnbd_srv_init_module(void)
};
rtrs_ctx = rtrs_srv_open(&rtrs_ops, port_nr);
if (IS_ERR(rtrs_ctx)) {
- err = PTR_ERR(rtrs_ctx);
pr_err("rtrs_srv_open(), err: %d\n", err);
- return err;
+ return PTR_ERR(rtrs_ctx);
}
err = rnbd_srv_create_sysfs_files();
if (err) {
pr_err("rnbd_srv_create_sysfs_files(), err: %d\n", err);
rtrs_srv_close(rtrs_ctx);
- return err;
}
- return 0;
+ return err;
}
static void __exit rnbd_srv_cleanup_module(void)
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 09/10] block/rnbd-srv: make process_msg_sess_info returns void
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
` (7 preceding siblings ...)
2023-05-23 7:53 ` [PATCH 08/10] block/rnbd-srv: init err earlier in rnbd_srv_init_module Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:32 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 10/10] block/rnbd: change device's name Guoqing Jiang
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Change the return type to void given it always returns 0.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-srv.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 1fdf3366135a..d678ffa50c5c 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -352,7 +352,7 @@ static int process_msg_open(struct rnbd_srv_session *srv_sess,
const void *msg, size_t len,
void *data, size_t datalen);
-static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
+static void process_msg_sess_info(struct rnbd_srv_session *srv_sess,
const void *msg, size_t len,
void *data, size_t datalen);
@@ -380,8 +380,7 @@ static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
ret = process_msg_open(srv_sess, usr, usrlen, data, datalen);
break;
case RNBD_MSG_SESS_INFO:
- ret = process_msg_sess_info(srv_sess, usr, usrlen, data,
- datalen);
+ process_msg_sess_info(srv_sess, usr, usrlen, data, datalen);
break;
default:
pr_warn("Received unexpected message type %d from session %s\n",
@@ -626,7 +625,7 @@ static char *rnbd_srv_get_full_path(struct rnbd_srv_session *srv_sess,
return full_path;
}
-static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
+static void process_msg_sess_info(struct rnbd_srv_session *srv_sess,
const void *msg, size_t len,
void *data, size_t datalen)
{
@@ -639,8 +638,6 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
rsp->hdr.type = cpu_to_le16(RNBD_MSG_SESS_INFO_RSP);
rsp->ver = srv_sess->ver;
-
- return 0;
}
/**
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 10/10] block/rnbd: change device's name
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
` (8 preceding siblings ...)
2023-05-23 7:53 ` [PATCH 09/10] block/rnbd-srv: make process_msg_sess_info returns void Guoqing Jiang
@ 2023-05-23 7:53 ` Guoqing Jiang
2023-05-23 9:18 ` Jinpu Wang
9 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 7:53 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Both rnbd-srv and rnbd-clt set it with 'clt', which is not
clear, let's change them to 'clt' and 'srv' accordingly.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-clt-sysfs.c | 2 +-
drivers/block/rnbd/rnbd-srv-sysfs.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
index a0b49a0c0bdd..f6e2b075d2d5 100644
--- a/drivers/block/rnbd/rnbd-clt-sysfs.c
+++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
@@ -652,7 +652,7 @@ int rnbd_clt_create_sysfs_files(void)
rnbd_dev = device_create_with_groups(rnbd_dev_class, NULL,
MKDEV(0, 0), NULL,
- default_attr_groups, "ctl");
+ default_attr_groups, "clt");
if (IS_ERR(rnbd_dev)) {
err = PTR_ERR(rnbd_dev);
goto cls_destroy;
diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
index 4962826e9639..f17a4085dfbb 100644
--- a/drivers/block/rnbd/rnbd-srv-sysfs.c
+++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
@@ -219,7 +219,7 @@ int rnbd_srv_create_sysfs_files(void)
return PTR_ERR(rnbd_dev_class);
rnbd_dev = device_create(rnbd_dev_class, NULL,
- MKDEV(0, 0), NULL, "ctl");
+ MKDEV(0, 0), NULL, "srv");
if (IS_ERR(rnbd_dev)) {
err = PTR_ERR(rnbd_dev);
goto cls_destroy;
--
2.35.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 10/10] block/rnbd: change device's name
2023-05-23 7:53 ` [PATCH 10/10] block/rnbd: change device's name Guoqing Jiang
@ 2023-05-23 9:18 ` Jinpu Wang
2023-05-23 9:20 ` Guoqing Jiang
0 siblings, 1 reply; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:18 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Both rnbd-srv and rnbd-clt set it with 'clt', which is not
> clear, let's change them to 'clt' and 'srv' accordingly.
The "ctl" means "control" here, it contains some writable knobs..
And this change will break user space tools, so NAK for this one,
others looks good, will reply separately.
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
NAK, please drop it.
Thx!
> ---
> drivers/block/rnbd/rnbd-clt-sysfs.c | 2 +-
> drivers/block/rnbd/rnbd-srv-sysfs.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
> index a0b49a0c0bdd..f6e2b075d2d5 100644
> --- a/drivers/block/rnbd/rnbd-clt-sysfs.c
> +++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
> @@ -652,7 +652,7 @@ int rnbd_clt_create_sysfs_files(void)
>
> rnbd_dev = device_create_with_groups(rnbd_dev_class, NULL,
> MKDEV(0, 0), NULL,
> - default_attr_groups, "ctl");
> + default_attr_groups, "clt");
> if (IS_ERR(rnbd_dev)) {
> err = PTR_ERR(rnbd_dev);
> goto cls_destroy;
> diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
> index 4962826e9639..f17a4085dfbb 100644
> --- a/drivers/block/rnbd/rnbd-srv-sysfs.c
> +++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
> @@ -219,7 +219,7 @@ int rnbd_srv_create_sysfs_files(void)
> return PTR_ERR(rnbd_dev_class);
>
> rnbd_dev = device_create(rnbd_dev_class, NULL,
> - MKDEV(0, 0), NULL, "ctl");
> + MKDEV(0, 0), NULL, "srv");
> if (IS_ERR(rnbd_dev)) {
> err = PTR_ERR(rnbd_dev);
> goto cls_destroy;
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 01/10] block/rnbd: kill rnbd_flags_supported
2023-05-23 7:53 ` [PATCH 01/10] block/rnbd: kill rnbd_flags_supported Guoqing Jiang
@ 2023-05-23 9:19 ` Jinpu Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:19 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> This routine is not called since added. Then the two flags
> (RNBD_OP_LAST and RNBD_F_ALL) can be removed too after kill
> rnbd_flags_supported.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-proto.h | 22 ----------------------
> 1 file changed, 22 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-proto.h b/drivers/block/rnbd/rnbd-proto.h
> index da1d0542d7e2..84fd69844b7d 100644
> --- a/drivers/block/rnbd/rnbd-proto.h
> +++ b/drivers/block/rnbd/rnbd-proto.h
> @@ -185,7 +185,6 @@ struct rnbd_msg_io {
> enum rnbd_io_flags {
>
> /* Operations */
> -
> RNBD_OP_READ = 0,
> RNBD_OP_WRITE = 1,
> RNBD_OP_FLUSH = 2,
> @@ -193,15 +192,9 @@ enum rnbd_io_flags {
> RNBD_OP_SECURE_ERASE = 4,
> RNBD_OP_WRITE_SAME = 5,
>
> - RNBD_OP_LAST,
> -
> /* Flags */
> -
> RNBD_F_SYNC = 1<<(RNBD_OP_BITS + 0),
> RNBD_F_FUA = 1<<(RNBD_OP_BITS + 1),
> -
> - RNBD_F_ALL = (RNBD_F_SYNC | RNBD_F_FUA)
> -
> };
>
> static inline u32 rnbd_op(u32 flags)
> @@ -214,21 +207,6 @@ static inline u32 rnbd_flags(u32 flags)
> return flags & ~RNBD_OP_MASK;
> }
>
> -static inline bool rnbd_flags_supported(u32 flags)
> -{
> - u32 op;
> -
> - op = rnbd_op(flags);
> - flags = rnbd_flags(flags);
> -
> - if (op >= RNBD_OP_LAST)
> - return false;
> - if (flags & ~RNBD_F_ALL)
> - return false;
> -
> - return true;
> -}
> -
> static inline blk_opf_t rnbd_to_bio_flags(u32 rnbd_opf)
> {
> blk_opf_t bio_opf;
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/10] block/rnbd-srv: remove unused header
2023-05-23 7:53 ` [PATCH 02/10] block/rnbd-srv: remove unused header Guoqing Jiang
@ 2023-05-23 9:20 ` Jinpu Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:20 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> No need to include it since none of macros in limits.h are
> used by rnbd-srv.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-srv-sysfs.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
> index d5d9267e1fa5..9fe7d9e0ab63 100644
> --- a/drivers/block/rnbd/rnbd-srv-sysfs.c
> +++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
> @@ -9,7 +9,6 @@
> #undef pr_fmt
> #define pr_fmt(fmt) KBUILD_MODNAME " L" __stringify(__LINE__) ": " fmt
>
> -#include <uapi/linux/limits.h>
> #include <linux/kobject.h>
> #include <linux/sysfs.h>
> #include <linux/stat.h>
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 10/10] block/rnbd: change device's name
2023-05-23 9:18 ` Jinpu Wang
@ 2023-05-23 9:20 ` Guoqing Jiang
0 siblings, 0 replies; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-23 9:20 UTC (permalink / raw)
To: Jinpu Wang; +Cc: haris.iqbal, axboe, linux-block
On 5/23/23 17:18, Jinpu Wang wrote:
> On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Both rnbd-srv and rnbd-clt set it with 'clt', which is not
>> clear, let's change them to 'clt' and 'srv' accordingly.
> The "ctl" means "control" here, it contains some writable knobs..
>
> And this change will break user space tools, so NAK for this one,
> others looks good, will reply separately.
>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> NAK, please drop it.
Sure, thanks for explanation!
Guoqing
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 03/10] block/rnbd: introduce rnbd_access_modes
2023-05-23 7:53 ` [PATCH 03/10] block/rnbd: introduce rnbd_access_modes Guoqing Jiang
@ 2023-05-23 9:23 ` Jinpu Wang
2023-05-24 1:51 ` Guoqing Jiang
0 siblings, 1 reply; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:23 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Add one new array (marked with __maybe_unused to prevent gcc warning about
> "defined but not used" with W=1), then we can remove rnbd_access_mode_str
> and rnbd-common.c accordingly.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
> drivers/block/rnbd/Makefile | 6 ++----
> drivers/block/rnbd/rnbd-clt-sysfs.c | 4 ++--
> drivers/block/rnbd/rnbd-common.c | 23 -----------------------
> drivers/block/rnbd/rnbd-proto.h | 9 +++++++++
> drivers/block/rnbd/rnbd-srv-sysfs.c | 2 +-
> drivers/block/rnbd/rnbd-srv.c | 4 ++--
> 6 files changed, 16 insertions(+), 32 deletions(-)
> delete mode 100644 drivers/block/rnbd/rnbd-common.c
>
> diff --git a/drivers/block/rnbd/Makefile b/drivers/block/rnbd/Makefile
> index 40b31630822c..208e5f865497 100644
> --- a/drivers/block/rnbd/Makefile
> +++ b/drivers/block/rnbd/Makefile
> @@ -3,13 +3,11 @@
> ccflags-y := -I$(srctree)/drivers/infiniband/ulp/rtrs
>
> rnbd-client-y := rnbd-clt.o \
> - rnbd-clt-sysfs.o \
> - rnbd-common.o
> + rnbd-clt-sysfs.o
>
> CFLAGS_rnbd-srv-trace.o = -I$(src)
>
> -rnbd-server-y := rnbd-common.o \
> - rnbd-srv.o \
> +rnbd-server-y := rnbd-srv.o \
> rnbd-srv-sysfs.o \
> rnbd-srv-trace.o
>
> diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
> index 8c6087949794..a0b49a0c0bdd 100644
> --- a/drivers/block/rnbd/rnbd-clt-sysfs.c
> +++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
> @@ -278,7 +278,7 @@ static ssize_t access_mode_show(struct kobject *kobj,
>
> dev = container_of(kobj, struct rnbd_clt_dev, kobj);
>
> - return sysfs_emit(page, "%s\n", rnbd_access_mode_str(dev->access_mode));
> + return sysfs_emit(page, "%s\n", rnbd_access_modes[dev->access_mode].str);
> }
>
> static struct kobj_attribute rnbd_clt_access_mode =
> @@ -596,7 +596,7 @@ static ssize_t rnbd_clt_map_device_store(struct kobject *kobj,
>
> pr_info("Mapping device %s on session %s, (access_mode: %s, nr_poll_queues: %d)\n",
> pathname, sessname,
> - rnbd_access_mode_str(access_mode),
> + rnbd_access_modes[access_mode].str,
> nr_poll_queues);
>
> dev = rnbd_clt_map_device(sessname, paths, path_cnt, port_nr, pathname,
> diff --git a/drivers/block/rnbd/rnbd-common.c b/drivers/block/rnbd/rnbd-common.c
> deleted file mode 100644
> index 596c3f732403..000000000000
> --- a/drivers/block/rnbd/rnbd-common.c
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * RDMA Network Block Driver
> - *
> - * Copyright (c) 2014 - 2018 ProfitBricks GmbH. All rights reserved.
> - * Copyright (c) 2018 - 2019 1&1 IONOS Cloud GmbH. All rights reserved.
> - * Copyright (c) 2019 - 2020 1&1 IONOS SE. All rights reserved.
> - */
> -#include "rnbd-proto.h"
> -
> -const char *rnbd_access_mode_str(enum rnbd_access_mode mode)
> -{
> - switch (mode) {
> - case RNBD_ACCESS_RO:
> - return "ro";
> - case RNBD_ACCESS_RW:
> - return "rw";
> - case RNBD_ACCESS_MIGRATION:
> - return "migration";
> - default:
> - return "unknown";
> - }
> -}
> diff --git a/drivers/block/rnbd/rnbd-proto.h b/drivers/block/rnbd/rnbd-proto.h
> index 84fd69844b7d..185e24eaf2bf 100644
> --- a/drivers/block/rnbd/rnbd-proto.h
> +++ b/drivers/block/rnbd/rnbd-proto.h
> @@ -61,6 +61,15 @@ enum rnbd_access_mode {
> RNBD_ACCESS_MIGRATION,
> };
>
> +static const __maybe_unused struct {
> + int mode;
why not enum rnbd_access_mode?
> + const char *str;
> +} rnbd_access_modes[] = {
> + [RNBD_ACCESS_RO] = {RNBD_ACCESS_RO, "ro"},
> + [RNBD_ACCESS_RW] = {RNBD_ACCESS_RW, "rw"},
> + [RNBD_ACCESS_MIGRATION] = {RNBD_ACCESS_MIGRATION, "migration"},
> +};
> +
> /**
> * struct rnbd_msg_sess_info - initial session info from client to server
> * @hdr: message header
> diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c
> index 9fe7d9e0ab63..4962826e9639 100644
> --- a/drivers/block/rnbd/rnbd-srv-sysfs.c
> +++ b/drivers/block/rnbd/rnbd-srv-sysfs.c
> @@ -103,7 +103,7 @@ static ssize_t access_mode_show(struct kobject *kobj,
> sess_dev = container_of(kobj, struct rnbd_srv_sess_dev, kobj);
>
> return sysfs_emit(page, "%s\n",
> - rnbd_access_mode_str(sess_dev->access_mode));
> + rnbd_access_modes[sess_dev->access_mode].str);
> }
>
> static struct kobj_attribute rnbd_srv_dev_session_access_mode_attr =
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 2cfed2e58d64..e9ef6bd4b50c 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -483,7 +483,7 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
> pr_err("Mapping device '%s' for session %s with RW permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
> srv_dev->id, srv_sess->sessname,
> srv_dev->open_write_cnt,
> - rnbd_access_mode_str(access_mode));
> + rnbd_access_modes[access_mode].str);
> }
> break;
> case RNBD_ACCESS_MIGRATION:
> @@ -494,7 +494,7 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
> pr_err("Mapping device '%s' for session %s with migration permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
> srv_dev->id, srv_sess->sessname,
> srv_dev->open_write_cnt,
> - rnbd_access_mode_str(access_mode));
> + rnbd_access_modes[access_mode].str);
> }
> break;
> default:
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 04/10] block/rnbd-srv: no need to check sess_dev
2023-05-23 7:53 ` [PATCH 04/10] block/rnbd-srv: no need to check sess_dev Guoqing Jiang
@ 2023-05-23 9:27 ` Jinpu Wang
2023-05-24 1:35 ` Guoqing Jiang
0 siblings, 1 reply; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:27 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Check ret is enough since if sess_dev is NULL which also
> implies ret should be 0.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
> drivers/block/rnbd/rnbd-srv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index e9ef6bd4b50c..c4122e65b36a 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -96,7 +96,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session *srv_sess)
> ret = kref_get_unless_zero(&sess_dev->kref);
> rcu_read_unlock();
>
> - if (!sess_dev || !ret)
> + if (!ret)
> return ERR_PTR(-ENXIO);
>
> return sess_dev;
> --
> 2.35.3
This looks wrong, if you drop the check for !sess_dev, you have to
check it later with IS_ERR_OR_NULL when call rnbd_get_sess_dev
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/10] block/rnbd-srv: defer the allocation of rnbd_io_private
2023-05-23 7:53 ` [PATCH 05/10] block/rnbd-srv: defer the allocation of rnbd_io_private Guoqing Jiang
@ 2023-05-23 9:29 ` Jinpu Wang
2023-05-24 1:35 ` Guoqing Jiang
0 siblings, 1 reply; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:29 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Only allocate priv after session is available.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
> drivers/block/rnbd/rnbd-srv.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index c4122e65b36a..b4c880759a52 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -128,20 +128,17 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
>
> trace_process_rdma(srv_sess, msg, id, datalen, usrlen);
>
> - priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> -
> dev_id = le32_to_cpu(msg->device_id);
> -
> sess_dev = rnbd_get_sess_dev(dev_id, srv_sess);
> if (IS_ERR(sess_dev)) {
> pr_err_ratelimited("Got I/O request on session %s for unknown device id %d\n",
> srv_sess->sessname, dev_id);
> - err = -ENOTCONN;
> - goto err;
> + return -ENOTCONN;
> }
>
> + priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
before return you have to rnbd_put_sess_dev!
it seems not much benefit with the change.
> priv->sess_dev = sess_dev;
> priv->id = id;
>
> @@ -169,7 +166,6 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
> bio_put:
> bio_put(bio);
> rnbd_put_sess_dev(sess_dev);
> -err:
> kfree(priv);
> return err;
> }
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 06/10] block/rnbd-srv: rename one member in rnbd_srv_dev
2023-05-23 7:53 ` [PATCH 06/10] block/rnbd-srv: rename one member in rnbd_srv_dev Guoqing Jiang
@ 2023-05-23 9:30 ` Jinpu Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:30 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> It actually represents the name of rnbd_srv_dev.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-srv.c | 14 +++++++-------
> drivers/block/rnbd/rnbd-srv.h | 2 +-
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index b4c880759a52..e51eb4b7f6e6 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -176,7 +176,7 @@ static void destroy_device(struct kref *kref)
>
> WARN_ONCE(!list_empty(&dev->sess_dev_list),
> "Device %s is being destroyed but still in use!\n",
> - dev->id);
> + dev->name);
>
> spin_lock(&dev_lock);
> list_del(&dev->list);
> @@ -427,7 +427,7 @@ static struct rnbd_srv_dev *rnbd_srv_init_srv_dev(struct block_device *bdev)
> if (!dev)
> return ERR_PTR(-ENOMEM);
>
> - snprintf(dev->id, sizeof(dev->id), "%pg", bdev);
> + snprintf(dev->name, sizeof(dev->name), "%pg", bdev);
> kref_init(&dev->kref);
> INIT_LIST_HEAD(&dev->sess_dev_list);
> mutex_init(&dev->lock);
> @@ -442,7 +442,7 @@ rnbd_srv_find_or_add_srv_dev(struct rnbd_srv_dev *new_dev)
>
> spin_lock(&dev_lock);
> list_for_each_entry(dev, &dev_list, list) {
> - if (!strncmp(dev->id, new_dev->id, sizeof(dev->id))) {
> + if (!strncmp(dev->name, new_dev->name, sizeof(dev->name))) {
> if (!kref_get_unless_zero(&dev->kref))
> /*
> * We lost the race, device is almost dead.
> @@ -477,7 +477,7 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
> ret = 0;
> } else {
> pr_err("Mapping device '%s' for session %s with RW permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
> - srv_dev->id, srv_sess->sessname,
> + srv_dev->name, srv_sess->sessname,
> srv_dev->open_write_cnt,
> rnbd_access_modes[access_mode].str);
> }
> @@ -488,14 +488,14 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
> ret = 0;
> } else {
> pr_err("Mapping device '%s' for session %s with migration permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
> - srv_dev->id, srv_sess->sessname,
> + srv_dev->name, srv_sess->sessname,
> srv_dev->open_write_cnt,
> rnbd_access_modes[access_mode].str);
> }
> break;
> default:
> pr_err("Received mapping request for device '%s' on session %s with invalid access mode: %d\n",
> - srv_dev->id, srv_sess->sessname, access_mode);
> + srv_dev->name, srv_sess->sessname, access_mode);
> ret = -EINVAL;
> }
>
> @@ -770,7 +770,7 @@ static int process_msg_open(struct rnbd_srv_session *srv_sess,
> list_add(&srv_sess_dev->dev_list, &srv_dev->sess_dev_list);
> mutex_unlock(&srv_dev->lock);
>
> - rnbd_srv_info(srv_sess_dev, "Opened device '%s'\n", srv_dev->id);
> + rnbd_srv_info(srv_sess_dev, "Opened device '%s'\n", srv_dev->name);
>
> kfree(full_path);
>
> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> index f5962fd31d62..6b5e5ade18ae 100644
> --- a/drivers/block/rnbd/rnbd-srv.h
> +++ b/drivers/block/rnbd/rnbd-srv.h
> @@ -35,7 +35,7 @@ struct rnbd_srv_dev {
> struct kobject dev_kobj;
> struct kobject *dev_sessions_kobj;
> struct kref kref;
> - char id[NAME_MAX];
> + char name[NAME_MAX];
> /* List of rnbd_srv_sess_dev structs */
> struct list_head sess_dev_list;
> struct mutex lock;
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 07/10] block/rnbd-srv: init ret with 0 instead of -EPERM
2023-05-23 7:53 ` [PATCH 07/10] block/rnbd-srv: init ret with 0 instead of -EPERM Guoqing Jiang
@ 2023-05-23 9:30 ` Jinpu Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:30 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Let's always set errno after pr_err which is consistent with
> default case.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-srv.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index e51eb4b7f6e6..102831c302fc 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -463,34 +463,33 @@ static int rnbd_srv_check_update_open_perm(struct rnbd_srv_dev *srv_dev,
> struct rnbd_srv_session *srv_sess,
> enum rnbd_access_mode access_mode)
> {
> - int ret = -EPERM;
> + int ret = 0;
>
> mutex_lock(&srv_dev->lock);
>
> switch (access_mode) {
> case RNBD_ACCESS_RO:
> - ret = 0;
> break;
> case RNBD_ACCESS_RW:
> if (srv_dev->open_write_cnt == 0) {
> srv_dev->open_write_cnt++;
> - ret = 0;
> } else {
> pr_err("Mapping device '%s' for session %s with RW permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
> srv_dev->name, srv_sess->sessname,
> srv_dev->open_write_cnt,
> rnbd_access_modes[access_mode].str);
> + ret = -EPERM;
> }
> break;
> case RNBD_ACCESS_MIGRATION:
> if (srv_dev->open_write_cnt < 2) {
> srv_dev->open_write_cnt++;
> - ret = 0;
> } else {
> pr_err("Mapping device '%s' for session %s with migration permissions failed. Device already opened as 'RW' by %d client(s), access mode %s.\n",
> srv_dev->name, srv_sess->sessname,
> srv_dev->open_write_cnt,
> rnbd_access_modes[access_mode].str);
> + ret = -EPERM;
> }
> break;
> default:
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 08/10] block/rnbd-srv: init err earlier in rnbd_srv_init_module
2023-05-23 7:53 ` [PATCH 08/10] block/rnbd-srv: init err earlier in rnbd_srv_init_module Guoqing Jiang
@ 2023-05-23 9:31 ` Jinpu Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:31 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> With this, we can remove several lines of code.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-srv.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 102831c302fc..1fdf3366135a 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -803,7 +803,7 @@ static struct rtrs_srv_ctx *rtrs_ctx;
> static struct rtrs_srv_ops rtrs_ops;
> static int __init rnbd_srv_init_module(void)
> {
> - int err;
> + int err = 0;
>
> BUILD_BUG_ON(sizeof(struct rnbd_msg_hdr) != 4);
> BUILD_BUG_ON(sizeof(struct rnbd_msg_sess_info) != 36);
> @@ -817,19 +817,17 @@ static int __init rnbd_srv_init_module(void)
> };
> rtrs_ctx = rtrs_srv_open(&rtrs_ops, port_nr);
> if (IS_ERR(rtrs_ctx)) {
> - err = PTR_ERR(rtrs_ctx);
> pr_err("rtrs_srv_open(), err: %d\n", err);
> - return err;
> + return PTR_ERR(rtrs_ctx);
> }
>
> err = rnbd_srv_create_sysfs_files();
> if (err) {
> pr_err("rnbd_srv_create_sysfs_files(), err: %d\n", err);
> rtrs_srv_close(rtrs_ctx);
> - return err;
> }
>
> - return 0;
> + return err;
> }
>
> static void __exit rnbd_srv_cleanup_module(void)
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/10] block/rnbd-srv: make process_msg_sess_info returns void
2023-05-23 7:53 ` [PATCH 09/10] block/rnbd-srv: make process_msg_sess_info returns void Guoqing Jiang
@ 2023-05-23 9:32 ` Jinpu Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jinpu Wang @ 2023-05-23 9:32 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Change the return type to void given it always returns 0.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-srv.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 1fdf3366135a..d678ffa50c5c 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -352,7 +352,7 @@ static int process_msg_open(struct rnbd_srv_session *srv_sess,
> const void *msg, size_t len,
> void *data, size_t datalen);
>
> -static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
> +static void process_msg_sess_info(struct rnbd_srv_session *srv_sess,
> const void *msg, size_t len,
> void *data, size_t datalen);
>
> @@ -380,8 +380,7 @@ static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
> ret = process_msg_open(srv_sess, usr, usrlen, data, datalen);
> break;
> case RNBD_MSG_SESS_INFO:
> - ret = process_msg_sess_info(srv_sess, usr, usrlen, data,
> - datalen);
> + process_msg_sess_info(srv_sess, usr, usrlen, data, datalen);
> break;
> default:
> pr_warn("Received unexpected message type %d from session %s\n",
> @@ -626,7 +625,7 @@ static char *rnbd_srv_get_full_path(struct rnbd_srv_session *srv_sess,
> return full_path;
> }
>
> -static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
> +static void process_msg_sess_info(struct rnbd_srv_session *srv_sess,
> const void *msg, size_t len,
> void *data, size_t datalen)
> {
> @@ -639,8 +638,6 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
>
> rsp->hdr.type = cpu_to_le16(RNBD_MSG_SESS_INFO_RSP);
> rsp->ver = srv_sess->ver;
> -
> - return 0;
> }
>
> /**
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 04/10] block/rnbd-srv: no need to check sess_dev
2023-05-23 9:27 ` Jinpu Wang
@ 2023-05-24 1:35 ` Guoqing Jiang
2023-05-24 6:32 ` Jinpu Wang
0 siblings, 1 reply; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-24 1:35 UTC (permalink / raw)
To: Jinpu Wang; +Cc: haris.iqbal, axboe, linux-block
On 5/23/23 17:27, Jinpu Wang wrote:
> On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Check ret is enough since if sess_dev is NULL which also
>> implies ret should be 0.
>>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>> ---
>> drivers/block/rnbd/rnbd-srv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
>> index e9ef6bd4b50c..c4122e65b36a 100644
>> --- a/drivers/block/rnbd/rnbd-srv.c
>> +++ b/drivers/block/rnbd/rnbd-srv.c
>> @@ -96,7 +96,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session *srv_sess)
>> ret = kref_get_unless_zero(&sess_dev->kref);
>> rcu_read_unlock();
>>
>> - if (!sess_dev || !ret)
>> + if (!ret)
>> return ERR_PTR(-ENXIO);
>>
>> return sess_dev;
>> --
>> 2.35.3
> This looks wrong, if you drop the check for !sess_dev, you have to
> check it later with IS_ERR_OR_NULL when call rnbd_get_sess_dev
How can it return NULL? Pls correct me if I am wrong.
1. If sess_dev is NULL then ret is 0, so it just returns ERR_PTR(-ENXIO).
2. If sess_dev is not NULL, we still rely on the checking of ret.
But I can drop it as well if you think it is obscure.
Thanks,
Guoqing
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/10] block/rnbd-srv: defer the allocation of rnbd_io_private
2023-05-23 9:29 ` Jinpu Wang
@ 2023-05-24 1:35 ` Guoqing Jiang
0 siblings, 0 replies; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-24 1:35 UTC (permalink / raw)
To: Jinpu Wang; +Cc: haris.iqbal, axboe, linux-block
On 5/23/23 17:29, Jinpu Wang wrote:
> On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Only allocate priv after session is available.
>>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>> ---
>> drivers/block/rnbd/rnbd-srv.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
>> index c4122e65b36a..b4c880759a52 100644
>> --- a/drivers/block/rnbd/rnbd-srv.c
>> +++ b/drivers/block/rnbd/rnbd-srv.c
>> @@ -128,20 +128,17 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
>>
>> trace_process_rdma(srv_sess, msg, id, datalen, usrlen);
>>
>> - priv = kmalloc(sizeof(*priv), GFP_KERNEL);
>> - if (!priv)
>> - return -ENOMEM;
>> -
>> dev_id = le32_to_cpu(msg->device_id);
>> -
>> sess_dev = rnbd_get_sess_dev(dev_id, srv_sess);
>> if (IS_ERR(sess_dev)) {
>> pr_err_ratelimited("Got I/O request on session %s for unknown device id %d\n",
>> srv_sess->sessname, dev_id);
>> - err = -ENOTCONN;
>> - goto err;
>> + return -ENOTCONN;
>> }
>>
>> + priv = kmalloc(sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
> before return you have to rnbd_put_sess_dev!
> it seems not much benefit with the change.
You are right, thanks for the review.
Guoqing
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 03/10] block/rnbd: introduce rnbd_access_modes
2023-05-23 9:23 ` Jinpu Wang
@ 2023-05-24 1:51 ` Guoqing Jiang
0 siblings, 0 replies; 26+ messages in thread
From: Guoqing Jiang @ 2023-05-24 1:51 UTC (permalink / raw)
To: Jinpu Wang; +Cc: haris.iqbal, axboe, linux-block
On 5/23/23 17:23, Jinpu Wang wrote:
>> + int mode;
> why not enum rnbd_access_mode?
Will change it in next version, thanks for the review.
Guoqing
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 04/10] block/rnbd-srv: no need to check sess_dev
2023-05-24 1:35 ` Guoqing Jiang
@ 2023-05-24 6:32 ` Jinpu Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jinpu Wang @ 2023-05-24 6:32 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Wed, May 24, 2023 at 3:35 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
>
>
> On 5/23/23 17:27, Jinpu Wang wrote:
> > On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >> Check ret is enough since if sess_dev is NULL which also
> >> implies ret should be 0.
> >>
> >> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> >> ---
> >> drivers/block/rnbd/rnbd-srv.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> >> index e9ef6bd4b50c..c4122e65b36a 100644
> >> --- a/drivers/block/rnbd/rnbd-srv.c
> >> +++ b/drivers/block/rnbd/rnbd-srv.c
> >> @@ -96,7 +96,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session *srv_sess)
> >> ret = kref_get_unless_zero(&sess_dev->kref);
> >> rcu_read_unlock();
> >>
> >> - if (!sess_dev || !ret)
> >> + if (!ret)
> >> return ERR_PTR(-ENXIO);
> >>
> >> return sess_dev;
> >> --
> >> 2.35.3
> > This looks wrong, if you drop the check for !sess_dev, you have to
> > check it later with IS_ERR_OR_NULL when call rnbd_get_sess_dev
>
> How can it return NULL? Pls correct me if I am wrong.
>
> 1. If sess_dev is NULL then ret is 0, so it just returns ERR_PTR(-ENXIO).
> 2. If sess_dev is not NULL, we still rely on the checking of ret.
Ah, you are right.
Acked-by: Jack Wang <jinpu.wang@ionos.com>
thx!
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2023-05-24 6:32 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-23 7:53 [PATCH 00/10] misc patches for rnbd Guoqing Jiang
2023-05-23 7:53 ` [PATCH 01/10] block/rnbd: kill rnbd_flags_supported Guoqing Jiang
2023-05-23 9:19 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 02/10] block/rnbd-srv: remove unused header Guoqing Jiang
2023-05-23 9:20 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 03/10] block/rnbd: introduce rnbd_access_modes Guoqing Jiang
2023-05-23 9:23 ` Jinpu Wang
2023-05-24 1:51 ` Guoqing Jiang
2023-05-23 7:53 ` [PATCH 04/10] block/rnbd-srv: no need to check sess_dev Guoqing Jiang
2023-05-23 9:27 ` Jinpu Wang
2023-05-24 1:35 ` Guoqing Jiang
2023-05-24 6:32 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 05/10] block/rnbd-srv: defer the allocation of rnbd_io_private Guoqing Jiang
2023-05-23 9:29 ` Jinpu Wang
2023-05-24 1:35 ` Guoqing Jiang
2023-05-23 7:53 ` [PATCH 06/10] block/rnbd-srv: rename one member in rnbd_srv_dev Guoqing Jiang
2023-05-23 9:30 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 07/10] block/rnbd-srv: init ret with 0 instead of -EPERM Guoqing Jiang
2023-05-23 9:30 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 08/10] block/rnbd-srv: init err earlier in rnbd_srv_init_module Guoqing Jiang
2023-05-23 9:31 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 09/10] block/rnbd-srv: make process_msg_sess_info returns void Guoqing Jiang
2023-05-23 9:32 ` Jinpu Wang
2023-05-23 7:53 ` [PATCH 10/10] block/rnbd: change device's name Guoqing Jiang
2023-05-23 9:18 ` Jinpu Wang
2023-05-23 9:20 ` Guoqing Jiang
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).