| Message ID | 20220620034923.35633-3-guoqing.jiang@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | reduce the size of rnbd_clt_dev | expand |
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>
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:
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(-)