On Mon, Jun 03, 2024 at 06:59:13PM +0800, Guoqing Jiang wrote:
>
>
> On 5/31/24 21:15, cel@xxxxxxxxxx wrote:
> > From: Chuck Lever <chuck.lever@xxxxxxxxxx>
> >
> > In a moment, I will add a second consumer of CMA ID creation in
> > svcrdma. Refactor so this code can be reused.
> >
> > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > ---
> > net/sunrpc/xprtrdma/svc_rdma_transport.c | 67 ++++++++++++++----------
> > 1 file changed, 40 insertions(+), 27 deletions(-)
> >
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > index 2b1c16b9547d..fa50b7494a0a 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > @@ -65,6 +65,8 @@
> > static struct svcxprt_rdma *svc_rdma_create_xprt(struct svc_serv *serv,
> > struct net *net, int node);
> > +static int svc_rdma_listen_handler(struct rdma_cm_id *cma_id,
> > + struct rdma_cm_event *event);
> > static struct svc_xprt *svc_rdma_create(struct svc_serv *serv,
> > struct net *net,
> > struct sockaddr *sa, int salen,
> > @@ -122,6 +124,41 @@ static void qp_event_handler(struct ib_event *event, void *context)
> > }
> > }
> > +static struct rdma_cm_id *
> > +svc_rdma_create_listen_id(struct net *net, struct sockaddr *sap,
> > + void *context)
> > +{
> > + struct rdma_cm_id *listen_id;
> > + int ret;
> > +
> > + listen_id = rdma_create_id(net, svc_rdma_listen_handler, context,
> > + RDMA_PS_TCP, IB_QPT_RC);
> > + if (IS_ERR(listen_id))
> > + return listen_id;
>
> I am wondering if above need to return PTR_ERR(listen_id),
PTR_ERR would convert the listen_id error to an integer, but
svc_rdma_create_listen_id() returns a pointer or an ERR_PTR. Thus
using PTR_ERR() would be wrong in this case.
> and I find some
> callers (in net/rds/, nvme etc)
> return PTR_ERR(id) while others (rtrs-srv, ib_isert.c) return ERR_PTR(ret)
> with ret is set to PTR_ERR(id).
These functions use PTR_ERR only when the calling function returns
an int.
--
Chuck Lever
