On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>
