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

[Guoqing Jiang <guoqing.jiang@linux.dev>]

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