From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sagi Grimberg Subject: Re: [PATCH 2/9] IB/srp: Introduce an additional local variable Date: Wed, 07 May 2014 19:58:17 +0300 Message-ID: <536A6629.1040503@dev.mellanox.co.il> References: <5368DA5B.80609@acm.org> <5368DADF.2070105@acm.org> <536A0CF7.7080307@dev.mellanox.co.il> <536A3AA4.5010100@acm.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <536A3AA4.5010100-HInyCGIudOg@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Bart Van Assche , Roland Dreier Cc: Sagi Grimberg , Vu Pham , David Dillow , Sebastian Parschauer , linux-rdma List-Id: linux-rdma@vger.kernel.org On 5/7/2014 4:52 PM, Bart Van Assche wrote: > On 05/07/14 12:37, Sagi Grimberg wrote: >> On 5/6/2014 3:51 PM, Bart Van Assche wrote: >>> This patch does not change any functionality. >>> >>> Signed-off-by: Bart Van Assche >>> Cc: Roland Dreier >>> Cc: David Dillow >>> Cc: Sagi Grimberg >>> Cc: Vu Pham >>> Cc: Sebastian Parschauer >>> --- >>> drivers/infiniband/ulp/srp/ib_srp.c | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c >>> b/drivers/infiniband/ulp/srp/ib_srp.c >>> index cf80f7a..8c03371 100644 >>> --- a/drivers/infiniband/ulp/srp/ib_srp.c >>> +++ b/drivers/infiniband/ulp/srp/ib_srp.c >>> @@ -290,6 +290,7 @@ static int srp_new_cm_id(struct srp_target_port >>> *target) >>> static int srp_create_target_ib(struct srp_target_port *target) >>> { >>> + struct srp_device *dev = target->srp_host->srp_dev; >>> struct ib_qp_init_attr *init_attr; >>> struct ib_cq *recv_cq, *send_cq; >>> struct ib_qp *qp; >>> @@ -299,16 +300,14 @@ static int srp_create_target_ib(struct >>> srp_target_port *target) >>> if (!init_attr) >>> return -ENOMEM; >>> - recv_cq = ib_create_cq(target->srp_host->srp_dev->dev, >>> - srp_recv_completion, NULL, target, >>> + recv_cq = ib_create_cq(dev->dev, srp_recv_completion, NULL, target, >>> target->queue_size, target->comp_vector); >>> if (IS_ERR(recv_cq)) { >>> ret = PTR_ERR(recv_cq); >>> goto err; >>> } >>> - send_cq = ib_create_cq(target->srp_host->srp_dev->dev, >>> - srp_send_completion, NULL, target, >>> + send_cq = ib_create_cq(dev->dev, srp_send_completion, NULL, target, >>> target->queue_size, target->comp_vector); >>> if (IS_ERR(send_cq)) { >>> ret = PTR_ERR(send_cq); >>> @@ -327,7 +326,7 @@ static int srp_create_target_ib(struct >>> srp_target_port *target) >>> init_attr->send_cq = send_cq; >>> init_attr->recv_cq = recv_cq; >>> - qp = ib_create_qp(target->srp_host->srp_dev->pd, init_attr); >>> + qp = ib_create_qp(dev->pd, init_attr); >>> if (IS_ERR(qp)) { >>> ret = PTR_ERR(qp); >>> goto err_send_cq; >> I understand why you need it later, but I'm not sure that use_fastreg >> should be a device attribute. > Hello Sagi, > > Can you clarify this comment ? The use_fast_reg member variable is > introduced in patch 9/9, so I'm not sure why a comment about that member > variable is made on patch 2/9 ? I was referring to the reason why you added local variable dev - it is for use_fastreg right? Since I agree with you that use_fastreg can be a per-device attribute, this comment is irrelevant. So, Reviewed-by: Sagi Grimberg Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html