[PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Guoqing Jiang]

Since all callers (process_{read,write}) set id->dir, no need to
pass 'dir' again.

Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
 drivers/block/rnbd/rnbd-srv.h          | 1 +
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
 drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 3f6c268e04ef..9600715f1029 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -368,10 +368,9 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
 				 const void *msg, size_t len,
 				 void *data, size_t datalen);
 
-static int rnbd_srv_rdma_ev(void *priv,
-			    struct rtrs_srv_op *id, int dir,
-			    void *data, size_t datalen, const void *usr,
-			    size_t usrlen)
+static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
+			    void *data, size_t datalen,
+			    const void *usr, size_t usrlen)
 {
 	struct rnbd_srv_session *srv_sess = priv;
 	const struct rnbd_msg_hdr *hdr = usr;
@@ -398,7 +397,7 @@ static int rnbd_srv_rdma_ev(void *priv,
 		break;
 	default:
 		pr_warn("Received unexpected message type %d with dir %d from session %s\n",
-			type, dir, srv_sess->sessname);
+			type, id->dir, srv_sess->sessname);
 		return -EINVAL;
 	}
 
diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
index 081bceaf4ae9..5a0ef6c2b5c7 100644
--- a/drivers/block/rnbd/rnbd-srv.h
+++ b/drivers/block/rnbd/rnbd-srv.h
@@ -14,6 +14,7 @@
 #include <linux/kref.h>
 
 #include <rtrs.h>
+#include <rtrs-srv.h>
 #include "rnbd-proto.h"
 #include "rnbd-log.h"
 
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 34c03bde5064..9dc50ff0e1b9 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1024,7 +1024,7 @@ static void process_read(struct rtrs_srv_con *con,
 	usr_len = le16_to_cpu(msg->usr_len);
 	data_len = off - usr_len;
 	data = page_address(srv->chunks[buf_id]);
-	ret = ctx->ops.rdma_ev(srv->priv, id, READ, data, data_len,
+	ret = ctx->ops.rdma_ev(srv->priv, id, data, data_len,
 			   data + data_len, usr_len);
 
 	if (ret) {
@@ -1077,7 +1077,7 @@ static void process_write(struct rtrs_srv_con *con,
 	usr_len = le16_to_cpu(req->usr_len);
 	data_len = off - usr_len;
 	data = page_address(srv->chunks[buf_id]);
-	ret = ctx->ops.rdma_ev(srv->priv, id, WRITE, data, data_len,
+	ret = ctx->ops.rdma_ev(srv->priv, id, data, data_len,
 			       data + data_len, usr_len);
 	if (ret) {
 		rtrs_err_rl(s,
diff --git a/drivers/infiniband/ulp/rtrs/rtrs.h b/drivers/infiniband/ulp/rtrs/rtrs.h
index 5e57a7ccc7fb..b48b53a7c143 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs.h
@@ -139,7 +139,6 @@ struct rtrs_srv_ops {
 
 	 *	@priv:		Private data set by rtrs_srv_set_sess_priv()
 	 *	@id:		internal RTRS operation id
-	 *	@dir:		READ/WRITE
 	 *	@data:		Pointer to (bidirectional) rdma memory area:
 	 *			- in case of %RTRS_SRV_RDMA_EV_RECV contains
 	 *			data sent by the client
@@ -151,7 +150,7 @@ struct rtrs_srv_ops {
 	 *	@usrlen:	Size of the user message
 	 */
 	int (*rdma_ev)(void *priv,
-		       struct rtrs_srv_op *id, int dir,
+		       struct rtrs_srv_op *id,
 		       void *data, size_t datalen, const void *usr,
 		       size_t usrlen);
 	/**
-- 
2.31.1

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Jinpu Wang]

On Fri, Aug 26, 2022 at 10:11 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Since all callers (process_{read,write}) set id->dir, no need to
> pass 'dir' again.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
>  drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
>  drivers/block/rnbd/rnbd-srv.h          | 1 +
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
>  drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
>  4 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 3f6c268e04ef..9600715f1029 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -368,10 +368,9 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
>                                  const void *msg, size_t len,
>                                  void *data, size_t datalen);
>
> -static int rnbd_srv_rdma_ev(void *priv,
> -                           struct rtrs_srv_op *id, int dir,
> -                           void *data, size_t datalen, const void *usr,
> -                           size_t usrlen)
> +static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
> +                           void *data, size_t datalen,
> +                           const void *usr, size_t usrlen)
>  {
>         struct rnbd_srv_session *srv_sess = priv;
>         const struct rnbd_msg_hdr *hdr = usr;
> @@ -398,7 +397,7 @@ static int rnbd_srv_rdma_ev(void *priv,
>                 break;
>         default:
>                 pr_warn("Received unexpected message type %d with dir %d from session %s\n",
> -                       type, dir, srv_sess->sessname);
> +                       type, id->dir, srv_sess->sessname);
>                 return -EINVAL;
>         }
>
> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> index 081bceaf4ae9..5a0ef6c2b5c7 100644
> --- a/drivers/block/rnbd/rnbd-srv.h
> +++ b/drivers/block/rnbd/rnbd-srv.h
> @@ -14,6 +14,7 @@
>  #include <linux/kref.h>
>
>  #include <rtrs.h>
> +#include <rtrs-srv.h>
why do we need this?
>  #include "rnbd-proto.h"
>  #include "rnbd-log.h"
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 34c03bde5064..9dc50ff0e1b9 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1024,7 +1024,7 @@ static void process_read(struct rtrs_srv_con *con,
>         usr_len = le16_to_cpu(msg->usr_len);
>         data_len = off - usr_len;
>         data = page_address(srv->chunks[buf_id]);
> -       ret = ctx->ops.rdma_ev(srv->priv, id, READ, data, data_len,
> +       ret = ctx->ops.rdma_ev(srv->priv, id, data, data_len,
>                            data + data_len, usr_len);
>
>         if (ret) {
> @@ -1077,7 +1077,7 @@ static void process_write(struct rtrs_srv_con *con,
>         usr_len = le16_to_cpu(req->usr_len);
>         data_len = off - usr_len;
>         data = page_address(srv->chunks[buf_id]);
> -       ret = ctx->ops.rdma_ev(srv->priv, id, WRITE, data, data_len,
> +       ret = ctx->ops.rdma_ev(srv->priv, id, data, data_len,
>                                data + data_len, usr_len);
>         if (ret) {
>                 rtrs_err_rl(s,
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs.h b/drivers/infiniband/ulp/rtrs/rtrs.h
> index 5e57a7ccc7fb..b48b53a7c143 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs.h
> +++ b/drivers/infiniband/ulp/rtrs/rtrs.h
> @@ -139,7 +139,6 @@ struct rtrs_srv_ops {
>
>          *      @priv:          Private data set by rtrs_srv_set_sess_priv()
>          *      @id:            internal RTRS operation id
> -        *      @dir:           READ/WRITE
>          *      @data:          Pointer to (bidirectional) rdma memory area:
>          *                      - in case of %RTRS_SRV_RDMA_EV_RECV contains
>          *                      data sent by the client
> @@ -151,7 +150,7 @@ struct rtrs_srv_ops {
>          *      @usrlen:        Size of the user message
>          */
>         int (*rdma_ev)(void *priv,
> -                      struct rtrs_srv_op *id, int dir,
> +                      struct rtrs_srv_op *id,
>                        void *data, size_t datalen, const void *usr,
>                        size_t usrlen);
>         /**
> --
> 2.31.1
>

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Guoqing Jiang]

On 8/26/22 6:48 PM, Jinpu Wang wrote:
> On Fri, Aug 26, 2022 at 10:11 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Since all callers (process_{read,write}) set id->dir, no need to
>> pass 'dir' again.
>>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>> ---
>>   drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
>>   drivers/block/rnbd/rnbd-srv.h          | 1 +
>>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
>>   drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
>>   4 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
>> index 3f6c268e04ef..9600715f1029 100644
>> --- a/drivers/block/rnbd/rnbd-srv.c
>> +++ b/drivers/block/rnbd/rnbd-srv.c
>> @@ -368,10 +368,9 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
>>                                   const void *msg, size_t len,
>>                                   void *data, size_t datalen);
>>
>> -static int rnbd_srv_rdma_ev(void *priv,
>> -                           struct rtrs_srv_op *id, int dir,
>> -                           void *data, size_t datalen, const void *usr,
>> -                           size_t usrlen)
>> +static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
>> +                           void *data, size_t datalen,
>> +                           const void *usr, size_t usrlen)
>>   {
>>          struct rnbd_srv_session *srv_sess = priv;
>>          const struct rnbd_msg_hdr *hdr = usr;
>> @@ -398,7 +397,7 @@ static int rnbd_srv_rdma_ev(void *priv,
>>                  break;
>>          default:
>>                  pr_warn("Received unexpected message type %d with dir %d from session %s\n",
>> -                       type, dir, srv_sess->sessname);
>> +                       type, id->dir, srv_sess->sessname);
>>                  return -EINVAL;
>>          }
>>
>> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
>> index 081bceaf4ae9..5a0ef6c2b5c7 100644
>> --- a/drivers/block/rnbd/rnbd-srv.h
>> +++ b/drivers/block/rnbd/rnbd-srv.h
>> @@ -14,6 +14,7 @@
>>   #include <linux/kref.h>
>>
>>   #include <rtrs.h>
>> +#include <rtrs-srv.h>
> why do we need this?

Otherwise, compiler complains

drivers/block/rnbd/rnbd-srv.c: In function ‘rnbd_srv_rdma_ev’:
drivers/block/rnbd/rnbd-srv.c:400:33: error: invalid use of undefined 
type ‘struct rtrs_srv_op’
   400 |                         type, id->dir, srv_sess->sessname);

Thanks,
Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Jinpu Wang]

On Fri, Aug 26, 2022 at 1:26 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
>
>
> On 8/26/22 6:48 PM, Jinpu Wang wrote:
> > On Fri, Aug 26, 2022 at 10:11 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >> Since all callers (process_{read,write}) set id->dir, no need to
> >> pass 'dir' again.
> >>
> >> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> >> ---
> >>   drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
> >>   drivers/block/rnbd/rnbd-srv.h          | 1 +
> >>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> >>   drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
> >>   4 files changed, 8 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> >> index 3f6c268e04ef..9600715f1029 100644
> >> --- a/drivers/block/rnbd/rnbd-srv.c
> >> +++ b/drivers/block/rnbd/rnbd-srv.c
> >> @@ -368,10 +368,9 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
> >>                                   const void *msg, size_t len,
> >>                                   void *data, size_t datalen);
> >>
> >> -static int rnbd_srv_rdma_ev(void *priv,
> >> -                           struct rtrs_srv_op *id, int dir,
> >> -                           void *data, size_t datalen, const void *usr,
> >> -                           size_t usrlen)
> >> +static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
> >> +                           void *data, size_t datalen,
> >> +                           const void *usr, size_t usrlen)
> >>   {
> >>          struct rnbd_srv_session *srv_sess = priv;
> >>          const struct rnbd_msg_hdr *hdr = usr;
> >> @@ -398,7 +397,7 @@ static int rnbd_srv_rdma_ev(void *priv,
> >>                  break;
> >>          default:
> >>                  pr_warn("Received unexpected message type %d with dir %d from session %s\n",
> >> -                       type, dir, srv_sess->sessname);
> >> +                       type, id->dir, srv_sess->sessname);
> >>                  return -EINVAL;
> >>          }
> >>
> >> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> >> index 081bceaf4ae9..5a0ef6c2b5c7 100644
> >> --- a/drivers/block/rnbd/rnbd-srv.h
> >> +++ b/drivers/block/rnbd/rnbd-srv.h
> >> @@ -14,6 +14,7 @@
> >>   #include <linux/kref.h>
> >>
> >>   #include <rtrs.h>
> >> +#include <rtrs-srv.h>
> > why do we need this?
>
> Otherwise, compiler complains
>
> drivers/block/rnbd/rnbd-srv.c: In function ‘rnbd_srv_rdma_ev’:
> drivers/block/rnbd/rnbd-srv.c:400:33: error: invalid use of undefined
> type ‘struct rtrs_srv_op’
>    400 |                         type, id->dir, srv_sess->sessname);
>
> Thanks,
> Guoqing
ah, okay, this reminds me, why we have dir there, we don't want to
export too much detail regarding the rtrs_srv_op to
rnbd-server, it is supposed to be transparent  to rnbd-srv.

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Guoqing Jiang]

On 8/26/22 7:29 PM, Jinpu Wang wrote:
> On Fri, Aug 26, 2022 at 1:26 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>>
>>
>> On 8/26/22 6:48 PM, Jinpu Wang wrote:
>>> On Fri, Aug 26, 2022 at 10:11 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>>>> Since all callers (process_{read,write}) set id->dir, no need to
>>>> pass 'dir' again.
>>>>
>>>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>>>> ---
>>>>    drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
>>>>    drivers/block/rnbd/rnbd-srv.h          | 1 +
>>>>    drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
>>>>    drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
>>>>    4 files changed, 8 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
>>>> index 3f6c268e04ef..9600715f1029 100644
>>>> --- a/drivers/block/rnbd/rnbd-srv.c
>>>> +++ b/drivers/block/rnbd/rnbd-srv.c
>>>> @@ -368,10 +368,9 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
>>>>                                    const void *msg, size_t len,
>>>>                                    void *data, size_t datalen);
>>>>
>>>> -static int rnbd_srv_rdma_ev(void *priv,
>>>> -                           struct rtrs_srv_op *id, int dir,
>>>> -                           void *data, size_t datalen, const void *usr,
>>>> -                           size_t usrlen)
>>>> +static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
>>>> +                           void *data, size_t datalen,
>>>> +                           const void *usr, size_t usrlen)
>>>>    {
>>>>           struct rnbd_srv_session *srv_sess = priv;
>>>>           const struct rnbd_msg_hdr *hdr = usr;
>>>> @@ -398,7 +397,7 @@ static int rnbd_srv_rdma_ev(void *priv,
>>>>                   break;
>>>>           default:
>>>>                   pr_warn("Received unexpected message type %d with dir %d from session %s\n",
>>>> -                       type, dir, srv_sess->sessname);
>>>> +                       type, id->dir, srv_sess->sessname);
>>>>                   return -EINVAL;
>>>>           }
>>>>
>>>> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
>>>> index 081bceaf4ae9..5a0ef6c2b5c7 100644
>>>> --- a/drivers/block/rnbd/rnbd-srv.h
>>>> +++ b/drivers/block/rnbd/rnbd-srv.h
>>>> @@ -14,6 +14,7 @@
>>>>    #include <linux/kref.h>
>>>>
>>>>    #include <rtrs.h>
>>>> +#include <rtrs-srv.h>
>>> why do we need this?
>> Otherwise, compiler complains
>>
>> drivers/block/rnbd/rnbd-srv.c: In function ‘rnbd_srv_rdma_ev’:
>> drivers/block/rnbd/rnbd-srv.c:400:33: error: invalid use of undefined
>> type ‘struct rtrs_srv_op’
>>     400 |                         type, id->dir, srv_sess->sessname);
>>
>> Thanks,
>> Guoqing
> ah, okay, this reminds me, why we have dir there, we don't want to
> export too much detail regarding the rtrs_srv_op to
> rnbd-server, it is supposed to be transparent  to rnbd-srv.

What is the issue with more details are exported from rtrs-srv? Both of 
the modules
are run in the same machine.

And I guess we can just pass parameters with register after remove an 
argument,
otherwise need to push/pop stack with more than six parameters for x64.

Thanks,
Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Jinpu Wang]

On Fri, Aug 26, 2022 at 1:38 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
>
>
> On 8/26/22 7:29 PM, Jinpu Wang wrote:
> > On Fri, Aug 26, 2022 at 1:26 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >>
> >>
> >> On 8/26/22 6:48 PM, Jinpu Wang wrote:
> >>> On Fri, Aug 26, 2022 at 10:11 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >>>> Since all callers (process_{read,write}) set id->dir, no need to
> >>>> pass 'dir' again.
> >>>>
> >>>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> >>>> ---
> >>>>    drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
> >>>>    drivers/block/rnbd/rnbd-srv.h          | 1 +
> >>>>    drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> >>>>    drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
> >>>>    4 files changed, 8 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> >>>> index 3f6c268e04ef..9600715f1029 100644
> >>>> --- a/drivers/block/rnbd/rnbd-srv.c
> >>>> +++ b/drivers/block/rnbd/rnbd-srv.c
> >>>> @@ -368,10 +368,9 @@ static int process_msg_sess_info(struct rnbd_srv_session *srv_sess,
> >>>>                                    const void *msg, size_t len,
> >>>>                                    void *data, size_t datalen);
> >>>>
> >>>> -static int rnbd_srv_rdma_ev(void *priv,
> >>>> -                           struct rtrs_srv_op *id, int dir,
> >>>> -                           void *data, size_t datalen, const void *usr,
> >>>> -                           size_t usrlen)
> >>>> +static int rnbd_srv_rdma_ev(void *priv, struct rtrs_srv_op *id,
> >>>> +                           void *data, size_t datalen,
> >>>> +                           const void *usr, size_t usrlen)
> >>>>    {
> >>>>           struct rnbd_srv_session *srv_sess = priv;
> >>>>           const struct rnbd_msg_hdr *hdr = usr;
> >>>> @@ -398,7 +397,7 @@ static int rnbd_srv_rdma_ev(void *priv,
> >>>>                   break;
> >>>>           default:
> >>>>                   pr_warn("Received unexpected message type %d with dir %d from session %s\n",
> >>>> -                       type, dir, srv_sess->sessname);
> >>>> +                       type, id->dir, srv_sess->sessname);
> >>>>                   return -EINVAL;
> >>>>           }
> >>>>
> >>>> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> >>>> index 081bceaf4ae9..5a0ef6c2b5c7 100644
> >>>> --- a/drivers/block/rnbd/rnbd-srv.h
> >>>> +++ b/drivers/block/rnbd/rnbd-srv.h
> >>>> @@ -14,6 +14,7 @@
> >>>>    #include <linux/kref.h>
> >>>>
> >>>>    #include <rtrs.h>
> >>>> +#include <rtrs-srv.h>
> >>> why do we need this?
> >> Otherwise, compiler complains
> >>
> >> drivers/block/rnbd/rnbd-srv.c: In function ‘rnbd_srv_rdma_ev’:
> >> drivers/block/rnbd/rnbd-srv.c:400:33: error: invalid use of undefined
> >> type ‘struct rtrs_srv_op’
> >>     400 |                         type, id->dir, srv_sess->sessname);
> >>
> >> Thanks,
> >> Guoqing
> > ah, okay, this reminds me, why we have dir there, we don't want to
> > export too much detail regarding the rtrs_srv_op to
> > rnbd-server, it is supposed to be transparent  to rnbd-srv.
>
> What is the issue with more details are exported from rtrs-srv? Both of
> the modules
> are run in the same machine.
with including rtrs-srv.h, the code size is bigger, not to mention
many unnecessary
info are exported to rnbd-srv module, ideally we want to have
rnbd/rtrs layered properly
with clear separation.

>
> And I guess we can just pass parameters with register after remove an
> argument,
> otherwise need to push/pop stack with more than six parameters for x64.
I doubt it makes any notable performance change.
>
> Thanks,
> Guoqing
Thx!

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Guoqing Jiang]

On 8/26/22 7:58 PM, Jinpu Wang wrote:
>> And I guess we can just pass parameters with register after remove an
>> argument,
>> otherwise need to push/pop stack with more than six parameters for x64.
> I doubt it makes any notable performance change.

I think it is called in the IO path (process_read and process_write) ...

BTW, do you agree if the 'usr' can be dropped from rdma_ev given it is 
always
same as 'data + data_len'?

Thanks,
Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Jinpu Wang]

On Fri, Aug 26, 2022 at 4:02 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
>
>
> On 8/26/22 7:58 PM, Jinpu Wang wrote:
> >> And I guess we can just pass parameters with register after remove an
> >> argument,
> >> otherwise need to push/pop stack with more than six parameters for x64.
> > I doubt it makes any notable performance change.
>
> I think it is called in the IO path (process_read and process_write) ...
>
> BTW, do you agree if the 'usr' can be dropped from rdma_ev given it is
> always
> same as 'data + data_len'?
in the current form, yes, usr  can be dropped. we can add later if
there is a need.
>
> Thanks,
> Guoqing
Thx!

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Leon Romanovsky]

On Fri, Aug 26, 2022 at 01:29:28PM +0200, Jinpu Wang wrote:
> On Fri, Aug 26, 2022 at 1:26 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >
> >
> >
> > On 8/26/22 6:48 PM, Jinpu Wang wrote:
> > > On Fri, Aug 26, 2022 at 10:11 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> > >> Since all callers (process_{read,write}) set id->dir, no need to
> > >> pass 'dir' again.
> > >>
> > >> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> > >> ---
> > >>   drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
> > >>   drivers/block/rnbd/rnbd-srv.h          | 1 +
> > >>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> > >>   drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
> > >>   4 files changed, 8 insertions(+), 9 deletions(-)

<...>

> > >>   #include <rtrs.h>
> > >> +#include <rtrs-srv.h>
> > > why do we need this?
> >
> > Otherwise, compiler complains
> >
> > drivers/block/rnbd/rnbd-srv.c: In function ‘rnbd_srv_rdma_ev’:
> > drivers/block/rnbd/rnbd-srv.c:400:33: error: invalid use of undefined
> > type ‘struct rtrs_srv_op’
> >    400 |                         type, id->dir, srv_sess->sessname);
> >
> > Thanks,
> > Guoqing
> ah, okay, this reminds me, why we have dir there, we don't want to
> export too much detail regarding the rtrs_srv_op to
> rnbd-server, it is supposed to be transparent  to rnbd-srv.

So decouple it from rtrs-srv.h and hide everything that not-needed to be
exported to separate header file.

Thanks

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Leon Romanovsky]

On Fri, Aug 26, 2022 at 04:11:17PM +0800, Guoqing Jiang wrote:
> Since all callers (process_{read,write}) set id->dir, no need to
> pass 'dir' again.
> 
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
>  drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
>  drivers/block/rnbd/rnbd-srv.h          | 1 +
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
>  drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
>  4 files changed, 8 insertions(+), 9 deletions(-)

I applied the patch and cleanup of rtrs-srv.h can be done later.

Thanks

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Guoqing Jiang]

On 8/29/22 3:44 PM, Leon Romanovsky wrote:
> On Fri, Aug 26, 2022 at 04:11:17PM +0800, Guoqing Jiang wrote:
>> Since all callers (process_{read,write}) set id->dir, no need to
>> pass 'dir' again.
>>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>> ---
>>   drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
>>   drivers/block/rnbd/rnbd-srv.h          | 1 +
>>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
>>   drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
>>   4 files changed, 8 insertions(+), 9 deletions(-)

> I applied the patch and cleanup of rtrs-srv.h can be done later.

Thanks! I suppose below

> So decouple it from rtrs-srv.h and hide everything that not-needed to be
> exported to separate header file.

means move 'struct rtrs_srv_op' to rtrs.h, which seems not appropriate to me
because both client and server include the header. Pls correct me if I 
am wrong.

Since process_{read,write} prints direction info if ctx->ops.rdma_ev 
fails, how
about remove the 'dir' info from rnbd_srv_rdma_ev? Then we  don't need to
include rtrs-srv.h.

diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 431c6da19d3f..d07ff3ba560c 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -387,8 +387,8 @@ static int rnbd_srv_rdma_ev(void *priv, struct 
rtrs_srv_op *id,
                                             datalen);
                 break;
         default:
-               pr_warn("Received unexpected message type %d with dir %d 
from session %s\n",
-                       type, id->dir, srv_sess->sessname);
+               pr_warn("Received unexpected message type %d from 
session %s\n",
+                       type, srv_sess->sessname);
                 return -EINVAL;
         }

diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
index 5a0ef6c2b5c7..081bceaf4ae9 100644
--- a/drivers/block/rnbd/rnbd-srv.h
+++ b/drivers/block/rnbd/rnbd-srv.h
@@ -14,7 +14,6 @@
  #include <linux/kref.h>

  #include <rtrs.h>
-#include <rtrs-srv.h>
  #include "rnbd-proto.h"
  #include "rnbd-log.h"


Thoughts?

Thanks,
Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Jinpu Wang]

On Mon, Aug 29, 2022 at 3:33 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
>
>
> On 8/29/22 3:44 PM, Leon Romanovsky wrote:
> > On Fri, Aug 26, 2022 at 04:11:17PM +0800, Guoqing Jiang wrote:
> >> Since all callers (process_{read,write}) set id->dir, no need to
> >> pass 'dir' again.
> >>
> >> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> >> ---
> >>   drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
> >>   drivers/block/rnbd/rnbd-srv.h          | 1 +
> >>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> >>   drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
> >>   4 files changed, 8 insertions(+), 9 deletions(-)
>
> > I applied the patch and cleanup of rtrs-srv.h can be done later.
>
> Thanks! I suppose below
>
> > So decouple it from rtrs-srv.h and hide everything that not-needed to be
> > exported to separate header file.
>
> means move 'struct rtrs_srv_op' to rtrs.h, which seems not appropriate to me
> because both client and server include the header. Pls correct me if I
> am wrong.
>
> Since process_{read,write} prints direction info if ctx->ops.rdma_ev
> fails, how
> about remove the 'dir' info from rnbd_srv_rdma_ev? Then we  don't need to
> include rtrs-srv.h.
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 431c6da19d3f..d07ff3ba560c 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -387,8 +387,8 @@ static int rnbd_srv_rdma_ev(void *priv, struct
> rtrs_srv_op *id,
>                                              datalen);
>                  break;
>          default:
> -               pr_warn("Received unexpected message type %d with dir %d
> from session %s\n",
> -                       type, id->dir, srv_sess->sessname);
> +               pr_warn("Received unexpected message type %d from
> session %s\n",
> +                       type, srv_sess->sessname);
>                  return -EINVAL;
>          }
>
> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> index 5a0ef6c2b5c7..081bceaf4ae9 100644
> --- a/drivers/block/rnbd/rnbd-srv.h
> +++ b/drivers/block/rnbd/rnbd-srv.h
> @@ -14,7 +14,6 @@
>   #include <linux/kref.h>
>
>   #include <rtrs.h>
> -#include <rtrs-srv.h>
>   #include "rnbd-proto.h"
>   #include "rnbd-log.h"
>
>
> Thoughts?
I like the idea. Please post a formal patch based on leon's
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=wip/leon-for-next

Thx!
>
> Thanks,
> Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Leon Romanovsky]

On Mon, Aug 29, 2022 at 03:43:43PM +0200, Jinpu Wang wrote:
> On Mon, Aug 29, 2022 at 3:33 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >
> >
> >
> > On 8/29/22 3:44 PM, Leon Romanovsky wrote:
> > > On Fri, Aug 26, 2022 at 04:11:17PM +0800, Guoqing Jiang wrote:
> > >> Since all callers (process_{read,write}) set id->dir, no need to
> > >> pass 'dir' again.
> > >>
> > >> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> > >> ---
> > >>   drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
> > >>   drivers/block/rnbd/rnbd-srv.h          | 1 +
> > >>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> > >>   drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
> > >>   4 files changed, 8 insertions(+), 9 deletions(-)
> >
> > > I applied the patch and cleanup of rtrs-srv.h can be done later.
> >
> > Thanks! I suppose below
> >
> > > So decouple it from rtrs-srv.h and hide everything that not-needed to be
> > > exported to separate header file.
> >
> > means move 'struct rtrs_srv_op' to rtrs.h, which seems not appropriate to me
> > because both client and server include the header. Pls correct me if I
> > am wrong.
> >
> > Since process_{read,write} prints direction info if ctx->ops.rdma_ev
> > fails, how
> > about remove the 'dir' info from rnbd_srv_rdma_ev? Then we  don't need to
> > include rtrs-srv.h.
> >
> > diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> > index 431c6da19d3f..d07ff3ba560c 100644
> > --- a/drivers/block/rnbd/rnbd-srv.c
> > +++ b/drivers/block/rnbd/rnbd-srv.c
> > @@ -387,8 +387,8 @@ static int rnbd_srv_rdma_ev(void *priv, struct
> > rtrs_srv_op *id,
> >                                              datalen);
> >                  break;
> >          default:
> > -               pr_warn("Received unexpected message type %d with dir %d
> > from session %s\n",
> > -                       type, id->dir, srv_sess->sessname);
> > +               pr_warn("Received unexpected message type %d from
> > session %s\n",
> > +                       type, srv_sess->sessname);
> >                  return -EINVAL;
> >          }
> >
> > diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> > index 5a0ef6c2b5c7..081bceaf4ae9 100644
> > --- a/drivers/block/rnbd/rnbd-srv.h
> > +++ b/drivers/block/rnbd/rnbd-srv.h
> > @@ -14,7 +14,6 @@
> >   #include <linux/kref.h>
> >
> >   #include <rtrs.h>
> > -#include <rtrs-srv.h>
> >   #include "rnbd-proto.h"
> >   #include "rnbd-log.h"
> >
> >
> > Thoughts?
> I like the idea. Please post a formal patch based on leon's
> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=wip/leon-for-next

I squashed this hunk into original patch.

Thanks

> 
> Thx!
> >
> > Thanks,
> > Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Guoqing Jiang]

On 8/30/22 4:44 PM, Leon Romanovsky wrote:
> On Mon, Aug 29, 2022 at 03:43:43PM +0200, Jinpu Wang wrote:
>> On Mon, Aug 29, 2022 at 3:33 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>>>
>>>
>>> On 8/29/22 3:44 PM, Leon Romanovsky wrote:
>>>> On Fri, Aug 26, 2022 at 04:11:17PM +0800, Guoqing Jiang wrote:
>>>>> Since all callers (process_{read,write}) set id->dir, no need to
>>>>> pass 'dir' again.
>>>>>
>>>>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>>>>> ---
>>>>>    drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
>>>>>    drivers/block/rnbd/rnbd-srv.h          | 1 +
>>>>>    drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
>>>>>    drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
>>>>>    4 files changed, 8 insertions(+), 9 deletions(-)
>>>> I applied the patch and cleanup of rtrs-srv.h can be done later.
>>> Thanks! I suppose below
>>>
>>>> So decouple it from rtrs-srv.h and hide everything that not-needed to be
>>>> exported to separate header file.
>>> means move 'struct rtrs_srv_op' to rtrs.h, which seems not appropriate to me
>>> because both client and server include the header. Pls correct me if I
>>> am wrong.
>>>
>>> Since process_{read,write} prints direction info if ctx->ops.rdma_ev
>>> fails, how
>>> about remove the 'dir' info from rnbd_srv_rdma_ev? Then we  don't need to
>>> include rtrs-srv.h.
>>>
>>> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
>>> index 431c6da19d3f..d07ff3ba560c 100644
>>> --- a/drivers/block/rnbd/rnbd-srv.c
>>> +++ b/drivers/block/rnbd/rnbd-srv.c
>>> @@ -387,8 +387,8 @@ static int rnbd_srv_rdma_ev(void *priv, struct
>>> rtrs_srv_op *id,
>>>                                               datalen);
>>>                   break;
>>>           default:
>>> -               pr_warn("Received unexpected message type %d with dir %d
>>> from session %s\n",
>>> -                       type, id->dir, srv_sess->sessname);
>>> +               pr_warn("Received unexpected message type %d from
>>> session %s\n",
>>> +                       type, srv_sess->sessname);
>>>                   return -EINVAL;
>>>           }
>>>
>>> diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
>>> index 5a0ef6c2b5c7..081bceaf4ae9 100644
>>> --- a/drivers/block/rnbd/rnbd-srv.h
>>> +++ b/drivers/block/rnbd/rnbd-srv.h
>>> @@ -14,7 +14,6 @@
>>>    #include <linux/kref.h>
>>>
>>>    #include <rtrs.h>
>>> -#include <rtrs-srv.h>
>>>    #include "rnbd-proto.h"
>>>    #include "rnbd-log.h"
>>>
>>>
>>> Thoughts?
>> I like the idea. Please post a formal patch based on leon's
>> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=wip/leon-for-next
> I squashed this hunk into original patch.

Great, thank you!

If possible, to better reflect the change, please at your convenience to 
replace the original commit
message with below.

"Since process_{read,write} already prints direction info if 
ctx->ops.rdma_ev fails, no need to pass 'dir'"

Thanks,
Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Leon Romanovsky]

On Tue, Aug 30, 2022 at 04:57:19PM +0800, Guoqing Jiang wrote:
> 
> 
> On 8/30/22 4:44 PM, Leon Romanovsky wrote:
> > On Mon, Aug 29, 2022 at 03:43:43PM +0200, Jinpu Wang wrote:
> > > On Mon, Aug 29, 2022 at 3:33 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> > > > 
> > > > 
> > > > On 8/29/22 3:44 PM, Leon Romanovsky wrote:
> > > > > On Fri, Aug 26, 2022 at 04:11:17PM +0800, Guoqing Jiang wrote:
> > > > > > Since all callers (process_{read,write}) set id->dir, no need to
> > > > > > pass 'dir' again.
> > > > > > 
> > > > > > Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> > > > > > ---
> > > > > >    drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
> > > > > >    drivers/block/rnbd/rnbd-srv.h          | 1 +
> > > > > >    drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> > > > > >    drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
> > > > > >    4 files changed, 8 insertions(+), 9 deletions(-)
> > > > > I applied the patch and cleanup of rtrs-srv.h can be done later.
> > > > Thanks! I suppose below
> > > > 
> > > > > So decouple it from rtrs-srv.h and hide everything that not-needed to be
> > > > > exported to separate header file.
> > > > means move 'struct rtrs_srv_op' to rtrs.h, which seems not appropriate to me
> > > > because both client and server include the header. Pls correct me if I
> > > > am wrong.
> > > > 
> > > > Since process_{read,write} prints direction info if ctx->ops.rdma_ev
> > > > fails, how
> > > > about remove the 'dir' info from rnbd_srv_rdma_ev? Then we  don't need to
> > > > include rtrs-srv.h.
> > > > 
> > > > diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> > > > index 431c6da19d3f..d07ff3ba560c 100644
> > > > --- a/drivers/block/rnbd/rnbd-srv.c
> > > > +++ b/drivers/block/rnbd/rnbd-srv.c
> > > > @@ -387,8 +387,8 @@ static int rnbd_srv_rdma_ev(void *priv, struct
> > > > rtrs_srv_op *id,
> > > >                                               datalen);
> > > >                   break;
> > > >           default:
> > > > -               pr_warn("Received unexpected message type %d with dir %d
> > > > from session %s\n",
> > > > -                       type, id->dir, srv_sess->sessname);
> > > > +               pr_warn("Received unexpected message type %d from
> > > > session %s\n",
> > > > +                       type, srv_sess->sessname);
> > > >                   return -EINVAL;
> > > >           }
> > > > 
> > > > diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> > > > index 5a0ef6c2b5c7..081bceaf4ae9 100644
> > > > --- a/drivers/block/rnbd/rnbd-srv.h
> > > > +++ b/drivers/block/rnbd/rnbd-srv.h
> > > > @@ -14,7 +14,6 @@
> > > >    #include <linux/kref.h>
> > > > 
> > > >    #include <rtrs.h>
> > > > -#include <rtrs-srv.h>
> > > >    #include "rnbd-proto.h"
> > > >    #include "rnbd-log.h"
> > > > 
> > > > 
> > > > Thoughts?
> > > I like the idea. Please post a formal patch based on leon's
> > > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=wip/leon-for-next
> > I squashed this hunk into original patch.
> 
> Great, thank you!
> 
> If possible, to better reflect the change, please at your convenience to
> replace the original commit
> message with below.
> 
> "Since process_{read,write} already prints direction info if
> ctx->ops.rdma_ev fails, no need to pass 'dir'"

Done, as long as patches are in wip/* branches, we can rebase them.

Thanks

> 
> Thanks,
> Guoqing

Re: [PATCH] rnbd-srv: remove 'dir' argument from rnbd_srv_rdma_ev

[Jinpu Wang]

On Tue, Aug 30, 2022 at 10:44 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, Aug 29, 2022 at 03:43:43PM +0200, Jinpu Wang wrote:
> > On Mon, Aug 29, 2022 at 3:33 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> > >
> > >
> > >
> > > On 8/29/22 3:44 PM, Leon Romanovsky wrote:
> > > > On Fri, Aug 26, 2022 at 04:11:17PM +0800, Guoqing Jiang wrote:
> > > >> Since all callers (process_{read,write}) set id->dir, no need to
> > > >> pass 'dir' again.
> > > >>
> > > >> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> > > >> ---
> > > >>   drivers/block/rnbd/rnbd-srv.c          | 9 ++++-----
> > > >>   drivers/block/rnbd/rnbd-srv.h          | 1 +
> > > >>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> > > >>   drivers/infiniband/ulp/rtrs/rtrs.h     | 3 +--
> > > >>   4 files changed, 8 insertions(+), 9 deletions(-)
> > >
> > > > I applied the patch and cleanup of rtrs-srv.h can be done later.
> > >
> > > Thanks! I suppose below
> > >
> > > > So decouple it from rtrs-srv.h and hide everything that not-needed to be
> > > > exported to separate header file.
> > >
> > > means move 'struct rtrs_srv_op' to rtrs.h, which seems not appropriate to me
> > > because both client and server include the header. Pls correct me if I
> > > am wrong.
> > >
> > > Since process_{read,write} prints direction info if ctx->ops.rdma_ev
> > > fails, how
> > > about remove the 'dir' info from rnbd_srv_rdma_ev? Then we  don't need to
> > > include rtrs-srv.h.
> > >
> > > diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> > > index 431c6da19d3f..d07ff3ba560c 100644
> > > --- a/drivers/block/rnbd/rnbd-srv.c
> > > +++ b/drivers/block/rnbd/rnbd-srv.c
> > > @@ -387,8 +387,8 @@ static int rnbd_srv_rdma_ev(void *priv, struct
> > > rtrs_srv_op *id,
> > >                                              datalen);
> > >                  break;
> > >          default:
> > > -               pr_warn("Received unexpected message type %d with dir %d
> > > from session %s\n",
> > > -                       type, id->dir, srv_sess->sessname);
> > > +               pr_warn("Received unexpected message type %d from
> > > session %s\n",
> > > +                       type, srv_sess->sessname);
> > >                  return -EINVAL;
> > >          }
> > >
> > > diff --git a/drivers/block/rnbd/rnbd-srv.h b/drivers/block/rnbd/rnbd-srv.h
> > > index 5a0ef6c2b5c7..081bceaf4ae9 100644
> > > --- a/drivers/block/rnbd/rnbd-srv.h
> > > +++ b/drivers/block/rnbd/rnbd-srv.h
> > > @@ -14,7 +14,6 @@
> > >   #include <linux/kref.h>
> > >
> > >   #include <rtrs.h>
> > > -#include <rtrs-srv.h>
> > >   #include "rnbd-proto.h"
> > >   #include "rnbd-log.h"
> > >
> > >
> > > Thoughts?
> > I like the idea. Please post a formal patch based on leon's
> > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=wip/leon-for-next
>
> I squashed this hunk into original patch.
>
Hi Leon,

Thank you!
> Thanks
>
> >
> > Thx!
> > >
> > > Thanks,
> > > Guoqing