* [RFC PATCH 1/6] rnbd-clt: open code send_msg_open in rnbd_clt_map_device
2022-06-20 3:49 [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Guoqing Jiang
@ 2022-06-20 3:49 ` Guoqing Jiang
2022-06-21 15:11 ` Jinpu Wang
2022-06-20 3:49 ` [RFC PATCH 2/6] rnbd-clt: don't free rsp in msg_open_conf for map scenario Guoqing Jiang
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-20 3:49 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Let's open code it in rnbd_clt_map_device, then we can use information
from rsp to setup gendisk and request_queue in next commits. After that,
we can remove some members (wc, fua and max_hw_sectors etc) from struct
rnbd_clt_dev.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-clt.c | 44 +++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index 409c76b81aed..0396532da742 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -1562,7 +1562,14 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
{
struct rnbd_clt_session *sess;
struct rnbd_clt_dev *dev;
- int ret;
+ int ret, errno;
+ struct rnbd_msg_open_rsp *rsp;
+ struct rnbd_msg_open msg;
+ struct rnbd_iu *iu;
+ struct kvec vec = {
+ .iov_base = &msg,
+ .iov_len = sizeof(msg)
+ };
if (exists_devpath(pathname, sessname))
return ERR_PTR(-EEXIST);
@@ -1582,13 +1589,46 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
ret = -EEXIST;
goto put_dev;
}
- ret = send_msg_open(dev, RTRS_PERMIT_WAIT);
+
+ rsp = kzalloc(sizeof(*rsp), GFP_KERNEL);
+ if (!rsp) {
+ ret = -ENOMEM;
+ goto del_dev;
+ }
+
+ iu = rnbd_get_iu(sess, RTRS_ADMIN_CON, RTRS_PERMIT_WAIT);
+ if (!iu) {
+ ret = -ENOMEM;
+ kfree(rsp);
+ goto del_dev;
+ }
+ iu->buf = rsp;
+ iu->dev = dev;
+ sg_init_one(iu->sgt.sgl, rsp, sizeof(*rsp));
+
+ msg.hdr.type = cpu_to_le16(RNBD_MSG_OPEN);
+ msg.access_mode = dev->access_mode;
+ strscpy(msg.dev_name, dev->pathname, sizeof(msg.dev_name));
+
+ WARN_ON(!rnbd_clt_get_dev(dev));
+ ret = send_usr_msg(sess->rtrs, READ, iu,
+ &vec, sizeof(*rsp), iu->sgt.sgl, 1,
+ msg_open_conf, &errno, RTRS_PERMIT_WAIT);
+ if (ret) {
+ rnbd_clt_put_dev(dev);
+ rnbd_put_iu(sess, iu);
+ kfree(rsp);
+ } else {
+ ret = errno;
+ }
+ rnbd_put_iu(sess, iu);
if (ret) {
rnbd_clt_err(dev,
"map_device: failed, can't open remote device, err: %d\n",
ret);
goto del_dev;
}
+
mutex_lock(&dev->lock);
pr_debug("Opened remote device: session=%s, path='%s'\n",
sess->sessname, pathname);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC PATCH 1/6] rnbd-clt: open code send_msg_open in rnbd_clt_map_device
2022-06-20 3:49 ` [RFC PATCH 1/6] rnbd-clt: open code send_msg_open in rnbd_clt_map_device Guoqing Jiang
@ 2022-06-21 15:11 ` Jinpu Wang
2022-06-22 11:14 ` Guoqing Jiang
0 siblings, 1 reply; 17+ messages in thread
From: Jinpu Wang @ 2022-06-21 15:11 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Mon, Jun 20, 2022 at 5:49 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Let's open code it in rnbd_clt_map_device, then we can use information
> from rsp to setup gendisk and request_queue in next commits. After that,
> we can remove some members (wc, fua and max_hw_sectors etc) from struct
> rnbd_clt_dev.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
> drivers/block/rnbd/rnbd-clt.c | 44 +++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
> index 409c76b81aed..0396532da742 100644
> --- a/drivers/block/rnbd/rnbd-clt.c
> +++ b/drivers/block/rnbd/rnbd-clt.c
> @@ -1562,7 +1562,14 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
> {
> struct rnbd_clt_session *sess;
> struct rnbd_clt_dev *dev;
> - int ret;
> + int ret, errno;
> + struct rnbd_msg_open_rsp *rsp;
> + struct rnbd_msg_open msg;
> + struct rnbd_iu *iu;
> + struct kvec vec = {
> + .iov_base = &msg,
> + .iov_len = sizeof(msg)
> + };
>
> if (exists_devpath(pathname, sessname))
> return ERR_PTR(-EEXIST);
> @@ -1582,13 +1589,46 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
> ret = -EEXIST;
> goto put_dev;
> }
> - ret = send_msg_open(dev, RTRS_PERMIT_WAIT);
> +
> + rsp = kzalloc(sizeof(*rsp), GFP_KERNEL);
> + if (!rsp) {
> + ret = -ENOMEM;
> + goto del_dev;
> + }
> +
> + iu = rnbd_get_iu(sess, RTRS_ADMIN_CON, RTRS_PERMIT_WAIT);
> + if (!iu) {
> + ret = -ENOMEM;
> + kfree(rsp);
> + goto del_dev;
> + }
> + iu->buf = rsp;
> + iu->dev = dev;
> + sg_init_one(iu->sgt.sgl, rsp, sizeof(*rsp));
> +
> + msg.hdr.type = cpu_to_le16(RNBD_MSG_OPEN);
> + msg.access_mode = dev->access_mode;
> + strscpy(msg.dev_name, dev->pathname, sizeof(msg.dev_name));
> +
> + WARN_ON(!rnbd_clt_get_dev(dev));
> + ret = send_usr_msg(sess->rtrs, READ, iu,
> + &vec, sizeof(*rsp), iu->sgt.sgl, 1,
> + msg_open_conf, &errno, RTRS_PERMIT_WAIT);
> + if (ret) {
> + rnbd_clt_put_dev(dev);
> + rnbd_put_iu(sess, iu);
> + kfree(rsp);
> + } else {
> + ret = errno;
> + }
> + rnbd_put_iu(sess, iu);
> if (ret) {
> rnbd_clt_err(dev,
> "map_device: failed, can't open remote device, err: %d\n",
> ret);
> goto del_dev;
> }
> +
looks ok, except this new line seems not needed.
Thx!
> mutex_lock(&dev->lock);
> pr_debug("Opened remote device: session=%s, path='%s'\n",
> sess->sessname, pathname);
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFC PATCH 1/6] rnbd-clt: open code send_msg_open in rnbd_clt_map_device
2022-06-21 15:11 ` Jinpu Wang
@ 2022-06-22 11:14 ` Guoqing Jiang
0 siblings, 0 replies; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-22 11:14 UTC (permalink / raw)
To: Jinpu Wang; +Cc: haris.iqbal, axboe, linux-block
On 6/21/22 11:11 PM, Jinpu Wang wrote:
> On Mon, Jun 20, 2022 at 5:49 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Let's open code it in rnbd_clt_map_device, then we can use information
>> from rsp to setup gendisk and request_queue in next commits. After that,
>> we can remove some members (wc, fua and max_hw_sectors etc) from struct
>> rnbd_clt_dev.
>>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>> ---
>> drivers/block/rnbd/rnbd-clt.c | 44 +++++++++++++++++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
>> index 409c76b81aed..0396532da742 100644
>> --- a/drivers/block/rnbd/rnbd-clt.c
>> +++ b/drivers/block/rnbd/rnbd-clt.c
>> @@ -1562,7 +1562,14 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
>> {
>> struct rnbd_clt_session *sess;
>> struct rnbd_clt_dev *dev;
>> - int ret;
>> + int ret, errno;
>> + struct rnbd_msg_open_rsp *rsp;
>> + struct rnbd_msg_open msg;
>> + struct rnbd_iu *iu;
>> + struct kvec vec = {
>> + .iov_base = &msg,
>> + .iov_len = sizeof(msg)
>> + };
>>
>> if (exists_devpath(pathname, sessname))
>> return ERR_PTR(-EEXIST);
>> @@ -1582,13 +1589,46 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
>> ret = -EEXIST;
>> goto put_dev;
>> }
>> - ret = send_msg_open(dev, RTRS_PERMIT_WAIT);
>> +
>> + rsp = kzalloc(sizeof(*rsp), GFP_KERNEL);
>> + if (!rsp) {
>> + ret = -ENOMEM;
>> + goto del_dev;
>> + }
>> +
>> + iu = rnbd_get_iu(sess, RTRS_ADMIN_CON, RTRS_PERMIT_WAIT);
>> + if (!iu) {
>> + ret = -ENOMEM;
>> + kfree(rsp);
>> + goto del_dev;
>> + }
>> + iu->buf = rsp;
>> + iu->dev = dev;
>> + sg_init_one(iu->sgt.sgl, rsp, sizeof(*rsp));
>> +
>> + msg.hdr.type = cpu_to_le16(RNBD_MSG_OPEN);
>> + msg.access_mode = dev->access_mode;
>> + strscpy(msg.dev_name, dev->pathname, sizeof(msg.dev_name));
>> +
>> + WARN_ON(!rnbd_clt_get_dev(dev));
>> + ret = send_usr_msg(sess->rtrs, READ, iu,
>> + &vec, sizeof(*rsp), iu->sgt.sgl, 1,
>> + msg_open_conf, &errno, RTRS_PERMIT_WAIT);
>> + if (ret) {
>> + rnbd_clt_put_dev(dev);
>> + rnbd_put_iu(sess, iu);
>> + kfree(rsp);
>> + } else {
>> + ret = errno;
>> + }
>> + rnbd_put_iu(sess, iu);
>> if (ret) {
>> rnbd_clt_err(dev,
>> "map_device: failed, can't open remote device, err: %d\n",
>> ret);
>> goto del_dev;
>> }
>> +
> looks ok, except this new line seems not needed.
Will remove it in next version.
Thanks,
Guoqing
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH 2/6] rnbd-clt: don't free rsp in msg_open_conf for map scenario
2022-06-20 3:49 [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Guoqing Jiang
2022-06-20 3:49 ` [RFC PATCH 1/6] rnbd-clt: open code send_msg_open in rnbd_clt_map_device Guoqing Jiang
@ 2022-06-20 3:49 ` Guoqing Jiang
2022-06-22 10:57 ` Jinpu Wang
2022-06-20 3:49 ` [RFC PATCH 3/6] rnbd-clt: kill read_only from struct rnbd_clt_dev Guoqing Jiang
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-20 3:49 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
For map scenario, rsp is freed in two places:
1. msg_open_conf frees rsp if rtrs_clt_request returns 0.
2. Otherwise, rsp is freed by the call sites of rtrs_clt_request.
Now, We'd like to control full lifecycle of rsp in rnbd_clt_map_device,
with that, it is feasible to pass rsp to rnbd_client_setup_device in
next commit.
For 1, it is possible to free rsp from the caller of send_usr_msg
because of the synchronization of iu->comp.wait. And we put iu later
in rnbd_clt_map_device to ensure order of release rsp and iu.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-clt.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index 0396532da742..6c4970878d23 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -507,6 +507,11 @@ static void msg_open_conf(struct work_struct *work)
struct rnbd_msg_open_rsp *rsp = iu->buf;
struct rnbd_clt_dev *dev = iu->dev;
int errno = iu->errno;
+ bool from_map = false;
+
+ /* INIT state is only triggered from rnbd_clt_map_device */
+ if (dev->dev_state == DEV_STATE_INIT)
+ from_map = true;
if (errno) {
rnbd_clt_err(dev,
@@ -523,7 +528,9 @@ static void msg_open_conf(struct work_struct *work)
send_msg_close(dev, device_id, RTRS_PERMIT_NOWAIT);
}
}
- kfree(rsp);
+ /* We free rsp in rnbd_clt_map_device for map scenario */
+ if (!from_map)
+ kfree(rsp);
wake_up_iu_comp(iu, errno);
rnbd_put_iu(dev->sess, iu);
rnbd_clt_put_dev(dev);
@@ -1617,16 +1624,14 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
if (ret) {
rnbd_clt_put_dev(dev);
rnbd_put_iu(sess, iu);
- kfree(rsp);
} else {
ret = errno;
}
- rnbd_put_iu(sess, iu);
if (ret) {
rnbd_clt_err(dev,
"map_device: failed, can't open remote device, err: %d\n",
ret);
- goto del_dev;
+ goto put_iu;
}
mutex_lock(&dev->lock);
@@ -1651,12 +1656,17 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
dev->max_hw_sectors, dev->wc, dev->fua);
mutex_unlock(&dev->lock);
+ kfree(rsp);
+ rnbd_put_iu(sess, iu);
rnbd_clt_put_sess(sess);
return dev;
send_close:
send_msg_close(dev, dev->device_id, RTRS_PERMIT_WAIT);
+put_iu:
+ kfree(rsp);
+ rnbd_put_iu(sess, iu);
del_dev:
delete_dev(dev);
put_dev:
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC PATCH 2/6] rnbd-clt: don't free rsp in msg_open_conf for map scenario
2022-06-20 3:49 ` [RFC PATCH 2/6] rnbd-clt: don't free rsp in msg_open_conf for map scenario Guoqing Jiang
@ 2022-06-22 10:57 ` Jinpu Wang
0 siblings, 0 replies; 17+ messages in thread
From: Jinpu Wang @ 2022-06-22 10:57 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> For map scenario, rsp is freed in two places:
>
> 1. msg_open_conf frees rsp if rtrs_clt_request returns 0.
>
> 2. Otherwise, rsp is freed by the call sites of rtrs_clt_request.
>
> Now, We'd like to control full lifecycle of rsp in rnbd_clt_map_device,
> with that, it is feasible to pass rsp to rnbd_client_setup_device in
> next commit.
>
> For 1, it is possible to free rsp from the caller of send_usr_msg
> because of the synchronization of iu->comp.wait. And we put iu later
> in rnbd_clt_map_device to ensure order of release rsp and iu.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
> drivers/block/rnbd/rnbd-clt.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
> index 0396532da742..6c4970878d23 100644
> --- a/drivers/block/rnbd/rnbd-clt.c
> +++ b/drivers/block/rnbd/rnbd-clt.c
> @@ -507,6 +507,11 @@ static void msg_open_conf(struct work_struct *work)
> struct rnbd_msg_open_rsp *rsp = iu->buf;
> struct rnbd_clt_dev *dev = iu->dev;
> int errno = iu->errno;
> + bool from_map = false;
> +
> + /* INIT state is only triggered from rnbd_clt_map_device */
> + if (dev->dev_state == DEV_STATE_INIT)
> + from_map = true;
>
> if (errno) {
> rnbd_clt_err(dev,
> @@ -523,7 +528,9 @@ static void msg_open_conf(struct work_struct *work)
> send_msg_close(dev, device_id, RTRS_PERMIT_NOWAIT);
> }
> }
> - kfree(rsp);
> + /* We free rsp in rnbd_clt_map_device for map scenario */
> + if (!from_map)
> + kfree(rsp);
> wake_up_iu_comp(iu, errno);
> rnbd_put_iu(dev->sess, iu);
> rnbd_clt_put_dev(dev);
> @@ -1617,16 +1624,14 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
> if (ret) {
> rnbd_clt_put_dev(dev);
> rnbd_put_iu(sess, iu);
> - kfree(rsp);
> } else {
> ret = errno;
> }
> - rnbd_put_iu(sess, iu);
> if (ret) {
> rnbd_clt_err(dev,
> "map_device: failed, can't open remote device, err: %d\n",
> ret);
> - goto del_dev;
> + goto put_iu;
> }
>
> mutex_lock(&dev->lock);
> @@ -1651,12 +1656,17 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
> dev->max_hw_sectors, dev->wc, dev->fua);
>
> mutex_unlock(&dev->lock);
> + kfree(rsp);
> + rnbd_put_iu(sess, iu);
> rnbd_clt_put_sess(sess);
>
> return dev;
>
> send_close:
> send_msg_close(dev, dev->device_id, RTRS_PERMIT_WAIT);
> +put_iu:
> + kfree(rsp);
> + rnbd_put_iu(sess, iu);
> del_dev:
> delete_dev(dev);
> put_dev:
> --
> 2.34.1
>
lgtm.
Acked-by: Jack Wang <jinpu.wang@ionos.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH 3/6] rnbd-clt: kill read_only from struct rnbd_clt_dev
2022-06-20 3:49 [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Guoqing Jiang
2022-06-20 3:49 ` [RFC PATCH 1/6] rnbd-clt: open code send_msg_open in rnbd_clt_map_device Guoqing Jiang
2022-06-20 3:49 ` [RFC PATCH 2/6] rnbd-clt: don't free rsp in msg_open_conf for map scenario Guoqing Jiang
@ 2022-06-20 3:49 ` Guoqing Jiang
2022-06-22 10:57 ` Jinpu Wang
2022-06-20 3:49 ` [RFC PATCH 4/6] rnbd-clt: reduce the size of " Guoqing Jiang
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-20 3:49 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
The member is not needed since we can call get_disk_ro to achieve the
same goal.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-clt.c | 8 ++------
drivers/block/rnbd/rnbd-clt.h | 1 -
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index 6c4970878d23..0bade2680eb9 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -949,7 +949,7 @@ static int rnbd_client_open(struct block_device *block_device, fmode_t mode)
{
struct rnbd_clt_dev *dev = block_device->bd_disk->private_data;
- if (dev->read_only && (mode & FMODE_WRITE))
+ if (get_disk_ro(dev->gd) && (mode & FMODE_WRITE))
return -EPERM;
if (dev->dev_state == DEV_STATE_UNMAPPED ||
@@ -1402,12 +1402,8 @@ static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
set_capacity(dev->gd, dev->nsectors);
- if (dev->access_mode == RNBD_ACCESS_RO) {
- dev->read_only = true;
+ if (dev->access_mode == RNBD_ACCESS_RO)
set_disk_ro(dev->gd, true);
- } else {
- dev->read_only = false;
- }
/*
* Network device does not need rotational
diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
index 2e2e8c4a85c1..26fb91d800e3 100644
--- a/drivers/block/rnbd/rnbd-clt.h
+++ b/drivers/block/rnbd/rnbd-clt.h
@@ -117,7 +117,6 @@ struct rnbd_clt_dev {
char *pathname;
enum rnbd_access_mode access_mode;
u32 nr_poll_queues;
- bool read_only;
bool wc;
bool fua;
u32 max_hw_sectors;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC PATCH 3/6] rnbd-clt: kill read_only from struct rnbd_clt_dev
2022-06-20 3:49 ` [RFC PATCH 3/6] rnbd-clt: kill read_only from struct rnbd_clt_dev Guoqing Jiang
@ 2022-06-22 10:57 ` Jinpu Wang
0 siblings, 0 replies; 17+ messages in thread
From: Jinpu Wang @ 2022-06-22 10:57 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> The member is not needed since we can call get_disk_ro to achieve the
> same goal.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
lgtm
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-clt.c | 8 ++------
> drivers/block/rnbd/rnbd-clt.h | 1 -
> 2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
> index 6c4970878d23..0bade2680eb9 100644
> --- a/drivers/block/rnbd/rnbd-clt.c
> +++ b/drivers/block/rnbd/rnbd-clt.c
> @@ -949,7 +949,7 @@ static int rnbd_client_open(struct block_device *block_device, fmode_t mode)
> {
> struct rnbd_clt_dev *dev = block_device->bd_disk->private_data;
>
> - if (dev->read_only && (mode & FMODE_WRITE))
> + if (get_disk_ro(dev->gd) && (mode & FMODE_WRITE))
> return -EPERM;
>
> if (dev->dev_state == DEV_STATE_UNMAPPED ||
> @@ -1402,12 +1402,8 @@ static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
>
> set_capacity(dev->gd, dev->nsectors);
>
> - if (dev->access_mode == RNBD_ACCESS_RO) {
> - dev->read_only = true;
> + if (dev->access_mode == RNBD_ACCESS_RO)
> set_disk_ro(dev->gd, true);
> - } else {
> - dev->read_only = false;
> - }
>
> /*
> * Network device does not need rotational
> diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
> index 2e2e8c4a85c1..26fb91d800e3 100644
> --- a/drivers/block/rnbd/rnbd-clt.h
> +++ b/drivers/block/rnbd/rnbd-clt.h
> @@ -117,7 +117,6 @@ struct rnbd_clt_dev {
> char *pathname;
> enum rnbd_access_mode access_mode;
> u32 nr_poll_queues;
> - bool read_only;
> bool wc;
> bool fua;
> u32 max_hw_sectors;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH 4/6] rnbd-clt: reduce the size of struct rnbd_clt_dev
2022-06-20 3:49 [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Guoqing Jiang
` (2 preceding siblings ...)
2022-06-20 3:49 ` [RFC PATCH 3/6] rnbd-clt: kill read_only from struct rnbd_clt_dev Guoqing Jiang
@ 2022-06-20 3:49 ` Guoqing Jiang
2022-06-22 10:58 ` Jinpu Wang
2022-06-20 3:49 ` [RFC PATCH 5/6] rnbd-clt: adjust the layout " Guoqing Jiang
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-20 3:49 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
Previously, both map and remap trigger rnbd_clt_set_dev_attr to set
some members in rnbd_clt_dev such as wc, fua and logical_block_size
etc, but those members are only useful for map scenario given the
setup_request_queue is only called from the path:
rnbd_clt_map_device -> rnbd_client_setup_device
Since rnbd_clt_map_device frees rsp after rnbd_client_setup_device,
we can pass rsp to rnbd_client_setup_device and it's callees, which
means queue's attributes can be set directly from relevant members
of rsp instead from rnbd_clt_dev.
After that, we can kill 11 members from rnbd_clt_dev, and we don't
need rnbd_clt_set_dev_attr either.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-clt.c | 118 ++++++++++++++++------------------
drivers/block/rnbd/rnbd-clt.h | 11 ----
2 files changed, 55 insertions(+), 74 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index 0bade2680eb9..2c63cd5ac09d 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -68,38 +68,12 @@ static inline bool rnbd_clt_get_dev(struct rnbd_clt_dev *dev)
return refcount_inc_not_zero(&dev->refcount);
}
-static int rnbd_clt_set_dev_attr(struct rnbd_clt_dev *dev,
- const struct rnbd_msg_open_rsp *rsp)
-{
- struct rnbd_clt_session *sess = dev->sess;
-
- if (!rsp->logical_block_size)
- return -EINVAL;
-
- dev->device_id = le32_to_cpu(rsp->device_id);
- dev->nsectors = le64_to_cpu(rsp->nsectors);
- dev->logical_block_size = le16_to_cpu(rsp->logical_block_size);
- dev->physical_block_size = le16_to_cpu(rsp->physical_block_size);
- dev->max_discard_sectors = le32_to_cpu(rsp->max_discard_sectors);
- dev->discard_granularity = le32_to_cpu(rsp->discard_granularity);
- dev->discard_alignment = le32_to_cpu(rsp->discard_alignment);
- dev->secure_discard = le16_to_cpu(rsp->secure_discard);
- dev->wc = !!(rsp->cache_policy & RNBD_WRITEBACK);
- dev->fua = !!(rsp->cache_policy & RNBD_FUA);
-
- dev->max_hw_sectors = sess->max_io_size / SECTOR_SIZE;
- dev->max_segments = sess->max_segments;
-
- return 0;
-}
-
static int rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
size_t new_nsectors)
{
- rnbd_clt_info(dev, "Device size changed from %zu to %zu sectors\n",
- dev->nsectors, new_nsectors);
- dev->nsectors = new_nsectors;
- set_capacity_and_notify(dev->gd, dev->nsectors);
+ rnbd_clt_info(dev, "Device size changed from %llu to %zu sectors\n",
+ get_capacity(dev->gd), new_nsectors);
+ set_capacity_and_notify(dev->gd, new_nsectors);
return 0;
}
@@ -123,15 +97,17 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
* If the device was remapped and the size changed in the
* meantime we need to revalidate it
*/
- if (dev->nsectors != nsectors)
+ if (get_capacity(dev->gd) != nsectors)
rnbd_clt_change_capacity(dev, nsectors);
gd_kobj = &disk_to_dev(dev->gd)->kobj;
kobject_uevent(gd_kobj, KOBJ_ONLINE);
rnbd_clt_info(dev, "Device online, device remapped successfully\n");
}
- err = rnbd_clt_set_dev_attr(dev, rsp);
- if (err)
+ if (!rsp->logical_block_size) {
+ err = -EINVAL;
goto out;
+ }
+ dev->device_id = le32_to_cpu(rsp->device_id);
dev->dev_state = DEV_STATE_MAPPED;
out:
@@ -970,10 +946,10 @@ static int rnbd_client_getgeo(struct block_device *block_device,
struct hd_geometry *geo)
{
u64 size;
- struct rnbd_clt_dev *dev;
+ struct rnbd_clt_dev *dev = block_device->bd_disk->private_data;
+ struct queue_limits *limit = &dev->queue->limits;
- dev = block_device->bd_disk->private_data;
- size = dev->size * (dev->logical_block_size / SECTOR_SIZE);
+ size = dev->size * (limit->logical_block_size / SECTOR_SIZE);
geo->cylinders = size >> 6; /* size/64 */
geo->heads = 4;
geo->sectors = 16;
@@ -1357,11 +1333,15 @@ static void rnbd_init_mq_hw_queues(struct rnbd_clt_dev *dev)
}
}
-static void setup_request_queue(struct rnbd_clt_dev *dev)
+static void setup_request_queue(struct rnbd_clt_dev *dev,
+ struct rnbd_msg_open_rsp *rsp)
{
- blk_queue_logical_block_size(dev->queue, dev->logical_block_size);
- blk_queue_physical_block_size(dev->queue, dev->physical_block_size);
- blk_queue_max_hw_sectors(dev->queue, dev->max_hw_sectors);
+ blk_queue_logical_block_size(dev->queue,
+ le16_to_cpu(rsp->logical_block_size));
+ blk_queue_physical_block_size(dev->queue,
+ le16_to_cpu(rsp->physical_block_size));
+ blk_queue_max_hw_sectors(dev->queue,
+ dev->sess->max_io_size / SECTOR_SIZE);
/*
* we don't support discards to "discontiguous" segments
@@ -1369,21 +1349,27 @@ static void setup_request_queue(struct rnbd_clt_dev *dev)
*/
blk_queue_max_discard_segments(dev->queue, 1);
- blk_queue_max_discard_sectors(dev->queue, dev->max_discard_sectors);
- dev->queue->limits.discard_granularity = dev->discard_granularity;
- dev->queue->limits.discard_alignment = dev->discard_alignment;
- if (dev->secure_discard)
+ blk_queue_max_discard_sectors(dev->queue,
+ le32_to_cpu(rsp->max_discard_sectors));
+ dev->queue->limits.discard_granularity =
+ le32_to_cpu(rsp->discard_granularity);
+ dev->queue->limits.discard_alignment =
+ le32_to_cpu(rsp->discard_alignment);
+ if (le16_to_cpu(rsp->secure_discard))
blk_queue_max_secure_erase_sectors(dev->queue,
- dev->max_discard_sectors);
+ le32_to_cpu(rsp->max_discard_sectors));
blk_queue_flag_set(QUEUE_FLAG_SAME_COMP, dev->queue);
blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, dev->queue);
- blk_queue_max_segments(dev->queue, dev->max_segments);
+ blk_queue_max_segments(dev->queue, dev->sess->max_segments);
blk_queue_io_opt(dev->queue, dev->sess->max_io_size);
blk_queue_virt_boundary(dev->queue, SZ_4K - 1);
- blk_queue_write_cache(dev->queue, dev->wc, dev->fua);
+ blk_queue_write_cache(dev->queue,
+ !!(rsp->cache_policy & RNBD_WRITEBACK),
+ !!(rsp->cache_policy & RNBD_FUA));
}
-static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
+static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev,
+ struct rnbd_msg_open_rsp *rsp, int idx)
{
int err;
@@ -1395,12 +1381,12 @@ static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
dev->gd->private_data = dev;
snprintf(dev->gd->disk_name, sizeof(dev->gd->disk_name), "rnbd%d",
idx);
- pr_debug("disk_name=%s, capacity=%zu\n",
+ pr_debug("disk_name=%s, capacity=%llu\n",
dev->gd->disk_name,
- dev->nsectors * (dev->logical_block_size / SECTOR_SIZE)
- );
+ le64_to_cpu(rsp->nsectors) *
+ (le16_to_cpu(rsp->logical_block_size) / SECTOR_SIZE));
- set_capacity(dev->gd, dev->nsectors);
+ set_capacity(dev->gd, le64_to_cpu(rsp->nsectors));
if (dev->access_mode == RNBD_ACCESS_RO)
set_disk_ro(dev->gd, true);
@@ -1416,11 +1402,13 @@ static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
return err;
}
-static int rnbd_client_setup_device(struct rnbd_clt_dev *dev)
+static int rnbd_client_setup_device(struct rnbd_clt_dev *dev,
+ struct rnbd_msg_open_rsp *rsp)
{
int idx = dev->clt_device_id;
- dev->size = dev->nsectors * dev->logical_block_size;
+ dev->size = le64_to_cpu(rsp->nsectors) *
+ le16_to_cpu(rsp->logical_block_size);
dev->gd = blk_mq_alloc_disk(&dev->sess->tag_set, dev);
if (IS_ERR(dev->gd))
@@ -1428,8 +1416,8 @@ static int rnbd_client_setup_device(struct rnbd_clt_dev *dev)
dev->queue = dev->gd->queue;
rnbd_init_mq_hw_queues(dev);
- setup_request_queue(dev);
- return rnbd_clt_setup_gen_disk(dev, idx);
+ setup_request_queue(dev, rsp);
+ return rnbd_clt_setup_gen_disk(dev, rsp, idx);
}
static struct rnbd_clt_dev *init_dev(struct rnbd_clt_session *sess,
@@ -1633,7 +1621,7 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
mutex_lock(&dev->lock);
pr_debug("Opened remote device: session=%s, path='%s'\n",
sess->sessname, pathname);
- ret = rnbd_client_setup_device(dev);
+ ret = rnbd_client_setup_device(dev, rsp);
if (ret) {
rnbd_clt_err(dev,
"map_device: Failed to configure device, err: %d\n",
@@ -1643,13 +1631,17 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
}
rnbd_clt_info(dev,
- "map_device: Device mapped as %s (nsectors: %zu, logical_block_size: %d, physical_block_size: %d, max_discard_sectors: %d, discard_granularity: %d, discard_alignment: %d, secure_discard: %d, max_segments: %d, max_hw_sectors: %d, wc: %d, fua: %d)\n",
- dev->gd->disk_name, dev->nsectors,
- dev->logical_block_size, dev->physical_block_size,
- dev->max_discard_sectors,
- dev->discard_granularity, dev->discard_alignment,
- dev->secure_discard, dev->max_segments,
- dev->max_hw_sectors, dev->wc, dev->fua);
+ "map_device: Device mapped as %s (nsectors: %llu, logical_block_size: %d, physical_block_size: %d, max_discard_sectors: %d, discard_granularity: %d, discard_alignment: %d, secure_discard: %d, max_segments: %d, max_hw_sectors: %d, wc: %d, fua: %d)\n",
+ dev->gd->disk_name, le64_to_cpu(rsp->nsectors),
+ le16_to_cpu(rsp->logical_block_size),
+ le16_to_cpu(rsp->physical_block_size),
+ le32_to_cpu(rsp->max_discard_sectors),
+ le32_to_cpu(rsp->discard_granularity),
+ le32_to_cpu(rsp->discard_alignment),
+ le16_to_cpu(rsp->secure_discard),
+ sess->max_segments, sess->max_io_size / SECTOR_SIZE,
+ !!(rsp->cache_policy & RNBD_WRITEBACK),
+ !!(rsp->cache_policy & RNBD_FUA));
mutex_unlock(&dev->lock);
kfree(rsp);
diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
index 26fb91d800e3..7520272541b1 100644
--- a/drivers/block/rnbd/rnbd-clt.h
+++ b/drivers/block/rnbd/rnbd-clt.h
@@ -117,17 +117,6 @@ struct rnbd_clt_dev {
char *pathname;
enum rnbd_access_mode access_mode;
u32 nr_poll_queues;
- bool wc;
- bool fua;
- u32 max_hw_sectors;
- u32 max_discard_sectors;
- u32 discard_granularity;
- u32 discard_alignment;
- u16 secure_discard;
- u16 physical_block_size;
- u16 logical_block_size;
- u16 max_segments;
- size_t nsectors;
u64 size; /* device size in bytes */
struct list_head list;
struct gendisk *gd;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC PATCH 4/6] rnbd-clt: reduce the size of struct rnbd_clt_dev
2022-06-20 3:49 ` [RFC PATCH 4/6] rnbd-clt: reduce the size of " Guoqing Jiang
@ 2022-06-22 10:58 ` Jinpu Wang
0 siblings, 0 replies; 17+ messages in thread
From: Jinpu Wang @ 2022-06-22 10:58 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Previously, both map and remap trigger rnbd_clt_set_dev_attr to set
> some members in rnbd_clt_dev such as wc, fua and logical_block_size
> etc, but those members are only useful for map scenario given the
> setup_request_queue is only called from the path:
>
> rnbd_clt_map_device -> rnbd_client_setup_device
>
> Since rnbd_clt_map_device frees rsp after rnbd_client_setup_device,
> we can pass rsp to rnbd_client_setup_device and it's callees, which
> means queue's attributes can be set directly from relevant members
> of rsp instead from rnbd_clt_dev.
>
> After that, we can kill 11 members from rnbd_clt_dev, and we don't
> need rnbd_clt_set_dev_attr either.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
lgtm
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-clt.c | 118 ++++++++++++++++------------------
> drivers/block/rnbd/rnbd-clt.h | 11 ----
> 2 files changed, 55 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
> index 0bade2680eb9..2c63cd5ac09d 100644
> --- a/drivers/block/rnbd/rnbd-clt.c
> +++ b/drivers/block/rnbd/rnbd-clt.c
> @@ -68,38 +68,12 @@ static inline bool rnbd_clt_get_dev(struct rnbd_clt_dev *dev)
> return refcount_inc_not_zero(&dev->refcount);
> }
>
> -static int rnbd_clt_set_dev_attr(struct rnbd_clt_dev *dev,
> - const struct rnbd_msg_open_rsp *rsp)
> -{
> - struct rnbd_clt_session *sess = dev->sess;
> -
> - if (!rsp->logical_block_size)
> - return -EINVAL;
> -
> - dev->device_id = le32_to_cpu(rsp->device_id);
> - dev->nsectors = le64_to_cpu(rsp->nsectors);
> - dev->logical_block_size = le16_to_cpu(rsp->logical_block_size);
> - dev->physical_block_size = le16_to_cpu(rsp->physical_block_size);
> - dev->max_discard_sectors = le32_to_cpu(rsp->max_discard_sectors);
> - dev->discard_granularity = le32_to_cpu(rsp->discard_granularity);
> - dev->discard_alignment = le32_to_cpu(rsp->discard_alignment);
> - dev->secure_discard = le16_to_cpu(rsp->secure_discard);
> - dev->wc = !!(rsp->cache_policy & RNBD_WRITEBACK);
> - dev->fua = !!(rsp->cache_policy & RNBD_FUA);
> -
> - dev->max_hw_sectors = sess->max_io_size / SECTOR_SIZE;
> - dev->max_segments = sess->max_segments;
> -
> - return 0;
> -}
> -
> static int rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
> size_t new_nsectors)
> {
> - rnbd_clt_info(dev, "Device size changed from %zu to %zu sectors\n",
> - dev->nsectors, new_nsectors);
> - dev->nsectors = new_nsectors;
> - set_capacity_and_notify(dev->gd, dev->nsectors);
> + rnbd_clt_info(dev, "Device size changed from %llu to %zu sectors\n",
> + get_capacity(dev->gd), new_nsectors);
> + set_capacity_and_notify(dev->gd, new_nsectors);
> return 0;
> }
>
> @@ -123,15 +97,17 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
> * If the device was remapped and the size changed in the
> * meantime we need to revalidate it
> */
> - if (dev->nsectors != nsectors)
> + if (get_capacity(dev->gd) != nsectors)
> rnbd_clt_change_capacity(dev, nsectors);
> gd_kobj = &disk_to_dev(dev->gd)->kobj;
> kobject_uevent(gd_kobj, KOBJ_ONLINE);
> rnbd_clt_info(dev, "Device online, device remapped successfully\n");
> }
> - err = rnbd_clt_set_dev_attr(dev, rsp);
> - if (err)
> + if (!rsp->logical_block_size) {
> + err = -EINVAL;
> goto out;
> + }
> + dev->device_id = le32_to_cpu(rsp->device_id);
> dev->dev_state = DEV_STATE_MAPPED;
>
> out:
> @@ -970,10 +946,10 @@ static int rnbd_client_getgeo(struct block_device *block_device,
> struct hd_geometry *geo)
> {
> u64 size;
> - struct rnbd_clt_dev *dev;
> + struct rnbd_clt_dev *dev = block_device->bd_disk->private_data;
> + struct queue_limits *limit = &dev->queue->limits;
>
> - dev = block_device->bd_disk->private_data;
> - size = dev->size * (dev->logical_block_size / SECTOR_SIZE);
> + size = dev->size * (limit->logical_block_size / SECTOR_SIZE);
> geo->cylinders = size >> 6; /* size/64 */
> geo->heads = 4;
> geo->sectors = 16;
> @@ -1357,11 +1333,15 @@ static void rnbd_init_mq_hw_queues(struct rnbd_clt_dev *dev)
> }
> }
>
> -static void setup_request_queue(struct rnbd_clt_dev *dev)
> +static void setup_request_queue(struct rnbd_clt_dev *dev,
> + struct rnbd_msg_open_rsp *rsp)
> {
> - blk_queue_logical_block_size(dev->queue, dev->logical_block_size);
> - blk_queue_physical_block_size(dev->queue, dev->physical_block_size);
> - blk_queue_max_hw_sectors(dev->queue, dev->max_hw_sectors);
> + blk_queue_logical_block_size(dev->queue,
> + le16_to_cpu(rsp->logical_block_size));
> + blk_queue_physical_block_size(dev->queue,
> + le16_to_cpu(rsp->physical_block_size));
> + blk_queue_max_hw_sectors(dev->queue,
> + dev->sess->max_io_size / SECTOR_SIZE);
>
> /*
> * we don't support discards to "discontiguous" segments
> @@ -1369,21 +1349,27 @@ static void setup_request_queue(struct rnbd_clt_dev *dev)
> */
> blk_queue_max_discard_segments(dev->queue, 1);
>
> - blk_queue_max_discard_sectors(dev->queue, dev->max_discard_sectors);
> - dev->queue->limits.discard_granularity = dev->discard_granularity;
> - dev->queue->limits.discard_alignment = dev->discard_alignment;
> - if (dev->secure_discard)
> + blk_queue_max_discard_sectors(dev->queue,
> + le32_to_cpu(rsp->max_discard_sectors));
> + dev->queue->limits.discard_granularity =
> + le32_to_cpu(rsp->discard_granularity);
> + dev->queue->limits.discard_alignment =
> + le32_to_cpu(rsp->discard_alignment);
> + if (le16_to_cpu(rsp->secure_discard))
> blk_queue_max_secure_erase_sectors(dev->queue,
> - dev->max_discard_sectors);
> + le32_to_cpu(rsp->max_discard_sectors));
> blk_queue_flag_set(QUEUE_FLAG_SAME_COMP, dev->queue);
> blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, dev->queue);
> - blk_queue_max_segments(dev->queue, dev->max_segments);
> + blk_queue_max_segments(dev->queue, dev->sess->max_segments);
> blk_queue_io_opt(dev->queue, dev->sess->max_io_size);
> blk_queue_virt_boundary(dev->queue, SZ_4K - 1);
> - blk_queue_write_cache(dev->queue, dev->wc, dev->fua);
> + blk_queue_write_cache(dev->queue,
> + !!(rsp->cache_policy & RNBD_WRITEBACK),
> + !!(rsp->cache_policy & RNBD_FUA));
> }
>
> -static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
> +static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev,
> + struct rnbd_msg_open_rsp *rsp, int idx)
> {
> int err;
>
> @@ -1395,12 +1381,12 @@ static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
> dev->gd->private_data = dev;
> snprintf(dev->gd->disk_name, sizeof(dev->gd->disk_name), "rnbd%d",
> idx);
> - pr_debug("disk_name=%s, capacity=%zu\n",
> + pr_debug("disk_name=%s, capacity=%llu\n",
> dev->gd->disk_name,
> - dev->nsectors * (dev->logical_block_size / SECTOR_SIZE)
> - );
> + le64_to_cpu(rsp->nsectors) *
> + (le16_to_cpu(rsp->logical_block_size) / SECTOR_SIZE));
>
> - set_capacity(dev->gd, dev->nsectors);
> + set_capacity(dev->gd, le64_to_cpu(rsp->nsectors));
>
> if (dev->access_mode == RNBD_ACCESS_RO)
> set_disk_ro(dev->gd, true);
> @@ -1416,11 +1402,13 @@ static int rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
> return err;
> }
>
> -static int rnbd_client_setup_device(struct rnbd_clt_dev *dev)
> +static int rnbd_client_setup_device(struct rnbd_clt_dev *dev,
> + struct rnbd_msg_open_rsp *rsp)
> {
> int idx = dev->clt_device_id;
>
> - dev->size = dev->nsectors * dev->logical_block_size;
> + dev->size = le64_to_cpu(rsp->nsectors) *
> + le16_to_cpu(rsp->logical_block_size);
>
> dev->gd = blk_mq_alloc_disk(&dev->sess->tag_set, dev);
> if (IS_ERR(dev->gd))
> @@ -1428,8 +1416,8 @@ static int rnbd_client_setup_device(struct rnbd_clt_dev *dev)
> dev->queue = dev->gd->queue;
> rnbd_init_mq_hw_queues(dev);
>
> - setup_request_queue(dev);
> - return rnbd_clt_setup_gen_disk(dev, idx);
> + setup_request_queue(dev, rsp);
> + return rnbd_clt_setup_gen_disk(dev, rsp, idx);
> }
>
> static struct rnbd_clt_dev *init_dev(struct rnbd_clt_session *sess,
> @@ -1633,7 +1621,7 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
> mutex_lock(&dev->lock);
> pr_debug("Opened remote device: session=%s, path='%s'\n",
> sess->sessname, pathname);
> - ret = rnbd_client_setup_device(dev);
> + ret = rnbd_client_setup_device(dev, rsp);
> if (ret) {
> rnbd_clt_err(dev,
> "map_device: Failed to configure device, err: %d\n",
> @@ -1643,13 +1631,17 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
> }
>
> rnbd_clt_info(dev,
> - "map_device: Device mapped as %s (nsectors: %zu, logical_block_size: %d, physical_block_size: %d, max_discard_sectors: %d, discard_granularity: %d, discard_alignment: %d, secure_discard: %d, max_segments: %d, max_hw_sectors: %d, wc: %d, fua: %d)\n",
> - dev->gd->disk_name, dev->nsectors,
> - dev->logical_block_size, dev->physical_block_size,
> - dev->max_discard_sectors,
> - dev->discard_granularity, dev->discard_alignment,
> - dev->secure_discard, dev->max_segments,
> - dev->max_hw_sectors, dev->wc, dev->fua);
> + "map_device: Device mapped as %s (nsectors: %llu, logical_block_size: %d, physical_block_size: %d, max_discard_sectors: %d, discard_granularity: %d, discard_alignment: %d, secure_discard: %d, max_segments: %d, max_hw_sectors: %d, wc: %d, fua: %d)\n",
> + dev->gd->disk_name, le64_to_cpu(rsp->nsectors),
> + le16_to_cpu(rsp->logical_block_size),
> + le16_to_cpu(rsp->physical_block_size),
> + le32_to_cpu(rsp->max_discard_sectors),
> + le32_to_cpu(rsp->discard_granularity),
> + le32_to_cpu(rsp->discard_alignment),
> + le16_to_cpu(rsp->secure_discard),
> + sess->max_segments, sess->max_io_size / SECTOR_SIZE,
> + !!(rsp->cache_policy & RNBD_WRITEBACK),
> + !!(rsp->cache_policy & RNBD_FUA));
>
> mutex_unlock(&dev->lock);
> kfree(rsp);
> diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
> index 26fb91d800e3..7520272541b1 100644
> --- a/drivers/block/rnbd/rnbd-clt.h
> +++ b/drivers/block/rnbd/rnbd-clt.h
> @@ -117,17 +117,6 @@ struct rnbd_clt_dev {
> char *pathname;
> enum rnbd_access_mode access_mode;
> u32 nr_poll_queues;
> - bool wc;
> - bool fua;
> - u32 max_hw_sectors;
> - u32 max_discard_sectors;
> - u32 discard_granularity;
> - u32 discard_alignment;
> - u16 secure_discard;
> - u16 physical_block_size;
> - u16 logical_block_size;
> - u16 max_segments;
> - size_t nsectors;
> u64 size; /* device size in bytes */
> struct list_head list;
> struct gendisk *gd;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH 5/6] rnbd-clt: adjust the layout of struct rnbd_clt_dev
2022-06-20 3:49 [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Guoqing Jiang
` (3 preceding siblings ...)
2022-06-20 3:49 ` [RFC PATCH 4/6] rnbd-clt: reduce the size of " Guoqing Jiang
@ 2022-06-20 3:49 ` Guoqing Jiang
2022-06-22 10:58 ` Jinpu Wang
2022-06-20 3:49 ` [RFC PATCH 6/6] rnbd-clt: refactor rnbd_clt_change_capacity Guoqing Jiang
2022-06-21 12:16 ` [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Jinpu Wang
6 siblings, 1 reply; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-20 3:49 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
While at it, let re-arrange the struct to remove holes.
Before, pahole reports
/* size: 232, cachelines: 4, members: 17 */
/* sum members: 224, holes: 2, sum holes: 8 */
/* last cacheline: 40 bytes */
After the change, the report changes to
/* size: 224, cachelines: 4, members: 17 */
/* last cacheline: 32 bytes */
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-clt.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
index 7520272541b1..df237d2ea0d9 100644
--- a/drivers/block/rnbd/rnbd-clt.h
+++ b/drivers/block/rnbd/rnbd-clt.h
@@ -106,6 +106,7 @@ struct rnbd_queue {
};
struct rnbd_clt_dev {
+ struct kobject kobj;
struct rnbd_clt_session *sess;
struct request_queue *queue;
struct rnbd_queue *hw_queues;
@@ -114,15 +115,14 @@ struct rnbd_clt_dev {
u32 clt_device_id;
struct mutex lock;
enum rnbd_clt_dev_state dev_state;
+ refcount_t refcount;
char *pathname;
enum rnbd_access_mode access_mode;
u32 nr_poll_queues;
u64 size; /* device size in bytes */
struct list_head list;
struct gendisk *gd;
- struct kobject kobj;
char *blk_symlink_name;
- refcount_t refcount;
struct work_struct unmap_on_rmmod_work;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC PATCH 5/6] rnbd-clt: adjust the layout of struct rnbd_clt_dev
2022-06-20 3:49 ` [RFC PATCH 5/6] rnbd-clt: adjust the layout " Guoqing Jiang
@ 2022-06-22 10:58 ` Jinpu Wang
0 siblings, 0 replies; 17+ messages in thread
From: Jinpu Wang @ 2022-06-22 10:58 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> While at it, let re-arrange the struct to remove holes.
>
> Before, pahole reports
>
> /* size: 232, cachelines: 4, members: 17 */
> /* sum members: 224, holes: 2, sum holes: 8 */
> /* last cacheline: 40 bytes */
>
> After the change, the report changes to
>
> /* size: 224, cachelines: 4, members: 17 */
> /* last cacheline: 32 bytes */
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
lgtm
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/block/rnbd/rnbd-clt.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
> index 7520272541b1..df237d2ea0d9 100644
> --- a/drivers/block/rnbd/rnbd-clt.h
> +++ b/drivers/block/rnbd/rnbd-clt.h
> @@ -106,6 +106,7 @@ struct rnbd_queue {
> };
>
> struct rnbd_clt_dev {
> + struct kobject kobj;
> struct rnbd_clt_session *sess;
> struct request_queue *queue;
> struct rnbd_queue *hw_queues;
> @@ -114,15 +115,14 @@ struct rnbd_clt_dev {
> u32 clt_device_id;
> struct mutex lock;
> enum rnbd_clt_dev_state dev_state;
> + refcount_t refcount;
> char *pathname;
> enum rnbd_access_mode access_mode;
> u32 nr_poll_queues;
> u64 size; /* device size in bytes */
> struct list_head list;
> struct gendisk *gd;
> - struct kobject kobj;
> char *blk_symlink_name;
> - refcount_t refcount;
> struct work_struct unmap_on_rmmod_work;
> };
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH 6/6] rnbd-clt: refactor rnbd_clt_change_capacity
2022-06-20 3:49 [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Guoqing Jiang
` (4 preceding siblings ...)
2022-06-20 3:49 ` [RFC PATCH 5/6] rnbd-clt: adjust the layout " Guoqing Jiang
@ 2022-06-20 3:49 ` Guoqing Jiang
2022-06-22 10:57 ` Jinpu Wang
2022-06-21 12:16 ` [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Jinpu Wang
6 siblings, 1 reply; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-20 3:49 UTC (permalink / raw)
To: haris.iqbal, jinpu.wang, axboe; +Cc: linux-block
1. process_msg_open_rsp checks if capacity changed or not before call
rnbd_clt_change_capacity while the checking also make sense for
rnbd_clt_resize_dev_store, let's move the checking into the function.
2. change the parameter type to 'sector_t' then we don't need to cast
it from rnbd_clt_resize_dev_store, and update rnbd_clt_resize_disk too.
3. no need to checking the return value, make it return void.
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
drivers/block/rnbd/rnbd-clt-sysfs.c | 2 +-
drivers/block/rnbd/rnbd-clt.c | 24 ++++++++++++------------
drivers/block/rnbd/rnbd-clt.h | 2 +-
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
index 2be5d87a3ca6..e7c7d9a68168 100644
--- a/drivers/block/rnbd/rnbd-clt-sysfs.c
+++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
@@ -376,7 +376,7 @@ static ssize_t rnbd_clt_resize_dev_store(struct kobject *kobj,
if (ret)
return ret;
- ret = rnbd_clt_resize_disk(dev, (size_t)sectors);
+ ret = rnbd_clt_resize_disk(dev, sectors);
if (ret)
return ret;
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index 2c63cd5ac09d..6c6c4ba3d41d 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -68,13 +68,18 @@ static inline bool rnbd_clt_get_dev(struct rnbd_clt_dev *dev)
return refcount_inc_not_zero(&dev->refcount);
}
-static int rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
- size_t new_nsectors)
+static void rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
+ sector_t new_nsectors)
{
- rnbd_clt_info(dev, "Device size changed from %llu to %zu sectors\n",
+ if (get_capacity(dev->gd) != new_nsectors)
+ return;
+
+ /*
+ * If the size changed, we need to revalidate it
+ */
+ rnbd_clt_info(dev, "Device size changed from %llu to %llu sectors\n",
get_capacity(dev->gd), new_nsectors);
set_capacity_and_notify(dev->gd, new_nsectors);
- return 0;
}
static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
@@ -93,12 +98,7 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
if (dev->dev_state == DEV_STATE_MAPPED_DISCONNECTED) {
u64 nsectors = le64_to_cpu(rsp->nsectors);
- /*
- * If the device was remapped and the size changed in the
- * meantime we need to revalidate it
- */
- if (get_capacity(dev->gd) != nsectors)
- rnbd_clt_change_capacity(dev, nsectors);
+ rnbd_clt_change_capacity(dev, nsectors);
gd_kobj = &disk_to_dev(dev->gd)->kobj;
kobject_uevent(gd_kobj, KOBJ_ONLINE);
rnbd_clt_info(dev, "Device online, device remapped successfully\n");
@@ -116,7 +116,7 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
return err;
}
-int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize)
+int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, sector_t newsize)
{
int ret = 0;
@@ -126,7 +126,7 @@ int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize)
ret = -ENOENT;
goto out;
}
- ret = rnbd_clt_change_capacity(dev, newsize);
+ rnbd_clt_change_capacity(dev, newsize);
out:
mutex_unlock(&dev->lock);
diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
index df237d2ea0d9..a48e040abe63 100644
--- a/drivers/block/rnbd/rnbd-clt.h
+++ b/drivers/block/rnbd/rnbd-clt.h
@@ -138,7 +138,7 @@ int rnbd_clt_unmap_device(struct rnbd_clt_dev *dev, bool force,
const struct attribute *sysfs_self);
int rnbd_clt_remap_device(struct rnbd_clt_dev *dev);
-int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize);
+int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, sector_t newsize);
/* rnbd-clt-sysfs.c */
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC PATCH 6/6] rnbd-clt: refactor rnbd_clt_change_capacity
2022-06-20 3:49 ` [RFC PATCH 6/6] rnbd-clt: refactor rnbd_clt_change_capacity Guoqing Jiang
@ 2022-06-22 10:57 ` Jinpu Wang
2022-06-22 11:12 ` Guoqing Jiang
0 siblings, 1 reply; 17+ messages in thread
From: Jinpu Wang @ 2022-06-22 10:57 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> 1. process_msg_open_rsp checks if capacity changed or not before call
> rnbd_clt_change_capacity while the checking also make sense for
> rnbd_clt_resize_dev_store, let's move the checking into the function.
>
> 2. change the parameter type to 'sector_t' then we don't need to cast
> it from rnbd_clt_resize_dev_store, and update rnbd_clt_resize_disk too.
>
> 3. no need to checking the return value, make it return void.
>
better to split this into 3 patches.
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
> drivers/block/rnbd/rnbd-clt-sysfs.c | 2 +-
> drivers/block/rnbd/rnbd-clt.c | 24 ++++++++++++------------
> drivers/block/rnbd/rnbd-clt.h | 2 +-
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
> index 2be5d87a3ca6..e7c7d9a68168 100644
> --- a/drivers/block/rnbd/rnbd-clt-sysfs.c
> +++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
> @@ -376,7 +376,7 @@ static ssize_t rnbd_clt_resize_dev_store(struct kobject *kobj,
> if (ret)
> return ret;
>
> - ret = rnbd_clt_resize_disk(dev, (size_t)sectors);
> + ret = rnbd_clt_resize_disk(dev, sectors);
> if (ret)
> return ret;
>
> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
> index 2c63cd5ac09d..6c6c4ba3d41d 100644
> --- a/drivers/block/rnbd/rnbd-clt.c
> +++ b/drivers/block/rnbd/rnbd-clt.c
> @@ -68,13 +68,18 @@ static inline bool rnbd_clt_get_dev(struct rnbd_clt_dev *dev)
> return refcount_inc_not_zero(&dev->refcount);
> }
>
> -static int rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
> - size_t new_nsectors)
> +static void rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
> + sector_t new_nsectors)
> {
> - rnbd_clt_info(dev, "Device size changed from %llu to %zu sectors\n",
> + if (get_capacity(dev->gd) != new_nsectors)
> + return;
This change is broken, it leads to resize no longer work, should be "=="
> +
> + /*
> + * If the size changed, we need to revalidate it
> + */
> + rnbd_clt_info(dev, "Device size changed from %llu to %llu sectors\n",
> get_capacity(dev->gd), new_nsectors);
> set_capacity_and_notify(dev->gd, new_nsectors);
> - return 0;
> }
>
> static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
> @@ -93,12 +98,7 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
> if (dev->dev_state == DEV_STATE_MAPPED_DISCONNECTED) {
> u64 nsectors = le64_to_cpu(rsp->nsectors);
>
> - /*
> - * If the device was remapped and the size changed in the
> - * meantime we need to revalidate it
> - */
> - if (get_capacity(dev->gd) != nsectors)
> - rnbd_clt_change_capacity(dev, nsectors);
> + rnbd_clt_change_capacity(dev, nsectors);
> gd_kobj = &disk_to_dev(dev->gd)->kobj;
> kobject_uevent(gd_kobj, KOBJ_ONLINE);
> rnbd_clt_info(dev, "Device online, device remapped successfully\n");
> @@ -116,7 +116,7 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
> return err;
> }
>
> -int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize)
> +int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, sector_t newsize)
> {
> int ret = 0;
>
> @@ -126,7 +126,7 @@ int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize)
> ret = -ENOENT;
> goto out;
> }
> - ret = rnbd_clt_change_capacity(dev, newsize);
> + rnbd_clt_change_capacity(dev, newsize);
>
> out:
> mutex_unlock(&dev->lock);
> diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
> index df237d2ea0d9..a48e040abe63 100644
> --- a/drivers/block/rnbd/rnbd-clt.h
> +++ b/drivers/block/rnbd/rnbd-clt.h
> @@ -138,7 +138,7 @@ int rnbd_clt_unmap_device(struct rnbd_clt_dev *dev, bool force,
> const struct attribute *sysfs_self);
>
> int rnbd_clt_remap_device(struct rnbd_clt_dev *dev);
> -int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize);
> +int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, sector_t newsize);
>
> /* rnbd-clt-sysfs.c */
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFC PATCH 6/6] rnbd-clt: refactor rnbd_clt_change_capacity
2022-06-22 10:57 ` Jinpu Wang
@ 2022-06-22 11:12 ` Guoqing Jiang
0 siblings, 0 replies; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-22 11:12 UTC (permalink / raw)
To: Jinpu Wang; +Cc: haris.iqbal, axboe, linux-block
On 6/22/22 6:57 PM, Jinpu Wang wrote:
> On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> 1. process_msg_open_rsp checks if capacity changed or not before call
>> rnbd_clt_change_capacity while the checking also make sense for
>> rnbd_clt_resize_dev_store, let's move the checking into the function.
>>
>> 2. change the parameter type to 'sector_t' then we don't need to cast
>> it from rnbd_clt_resize_dev_store, and update rnbd_clt_resize_disk too.
>>
>> 3. no need to checking the return value, make it return void.
>>
> better to split this into 3 patches.
Will do.
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>> ---
>> drivers/block/rnbd/rnbd-clt-sysfs.c | 2 +-
>> drivers/block/rnbd/rnbd-clt.c | 24 ++++++++++++------------
>> drivers/block/rnbd/rnbd-clt.h | 2 +-
>> 3 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
>> index 2be5d87a3ca6..e7c7d9a68168 100644
>> --- a/drivers/block/rnbd/rnbd-clt-sysfs.c
>> +++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
>> @@ -376,7 +376,7 @@ static ssize_t rnbd_clt_resize_dev_store(struct kobject *kobj,
>> if (ret)
>> return ret;
>>
>> - ret = rnbd_clt_resize_disk(dev, (size_t)sectors);
>> + ret = rnbd_clt_resize_disk(dev, sectors);
>> if (ret)
>> return ret;
>>
>> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
>> index 2c63cd5ac09d..6c6c4ba3d41d 100644
>> --- a/drivers/block/rnbd/rnbd-clt.c
>> +++ b/drivers/block/rnbd/rnbd-clt.c
>> @@ -68,13 +68,18 @@ static inline bool rnbd_clt_get_dev(struct rnbd_clt_dev *dev)
>> return refcount_inc_not_zero(&dev->refcount);
>> }
>>
>> -static int rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
>> - size_t new_nsectors)
>> +static void rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
>> + sector_t new_nsectors)
>> {
>> - rnbd_clt_info(dev, "Device size changed from %llu to %zu sectors\n",
>> + if (get_capacity(dev->gd) != new_nsectors)
>> + return;
> This change is broken, it leads to resize no longer work, should be "=="
Yes, thanks for catch it.
Thanks,
Guoqing
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 0/6] reduce the size of rnbd_clt_dev
2022-06-20 3:49 [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Guoqing Jiang
` (5 preceding siblings ...)
2022-06-20 3:49 ` [RFC PATCH 6/6] rnbd-clt: refactor rnbd_clt_change_capacity Guoqing Jiang
@ 2022-06-21 12:16 ` Jinpu Wang
2022-06-22 11:15 ` Guoqing Jiang
6 siblings, 1 reply; 17+ messages in thread
From: Jinpu Wang @ 2022-06-21 12:16 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: haris.iqbal, axboe, linux-block
On Mon, Jun 20, 2022 at 5:49 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Hi,
>
> The struct rnbd_clt_dev added some members (wc, fua and max_hw_sectors
> etc) which are used to set up gendisk and request_queue, but seems only
> map scenario need to setup them since rnbd_client_setup_device is not
> called from remap path.
>
> Previously, pahole reports.
>
> /* size: 272, cachelines: 5, members: 29 */
> /* sum members: 259, holes: 4, sum holes: 13 */
> /* last cacheline: 16 bytes */
>
> After the series, it changes to
>
> /* size: 224, cachelines: 4, members: 17 */
> /* last cacheline: 32 bytes */
>
> Please review.
>
> Thanks,
> Guoqing
Hi Guoqing,
Thanks for the patchset, I had a brief look, in general I like the
idea, will run a regression test. and sort out the details.
Thanks!
Jinpu
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFC PATCH 0/6] reduce the size of rnbd_clt_dev
2022-06-21 12:16 ` [RFC PATCH 0/6] reduce the size of rnbd_clt_dev Jinpu Wang
@ 2022-06-22 11:15 ` Guoqing Jiang
0 siblings, 0 replies; 17+ messages in thread
From: Guoqing Jiang @ 2022-06-22 11:15 UTC (permalink / raw)
To: Jinpu Wang; +Cc: haris.iqbal, axboe, linux-block
Hi Jinpu,
On 6/21/22 8:16 PM, Jinpu Wang wrote:
> On Mon, Jun 20, 2022 at 5:49 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Hi,
>>
>> The struct rnbd_clt_dev added some members (wc, fua and max_hw_sectors
>> etc) which are used to set up gendisk and request_queue, but seems only
>> map scenario need to setup them since rnbd_client_setup_device is not
>> called from remap path.
>>
>> Previously, pahole reports.
>>
>> /* size: 272, cachelines: 5, members: 29 */
>> /* sum members: 259, holes: 4, sum holes: 13 */
>> /* last cacheline: 16 bytes */
>>
>> After the series, it changes to
>> t
>> /* last cacheline: 32 bytes */
>>
>> Please review.
>>
>> Thanks,
>> Guoqing
> Hi Guoqing,
>
> Thanks for the patchset, I had a brief look, in general I like the
> idea, will run a regression test. and sort out the details.
>
Thanks for the review!
Thanks,
Guoqing
^ permalink raw reply [flat|nested] 17+ messages in thread