On Tue, Aug 3, 2010 at 5:44 PM, David Dillow <[email protected]> wrote:
>
> On Tue, 2010-08-03 at 17:26 +0200, Bart Van Assche wrote:
> [ ... ]
> > I'm not sure it is a good idea to allow that all transmit buffers get
> > allocated for sending CMD_RSP information units and that none remain
> > for replying to incoming SRP_CRED_REQ / SRP_AER_REQ / SRP_T_LOGOUT
> > requests. Wouldn't that be a protocol violation ?
>
> You mean SRP_CMD? We don't do that, we clamp the outstanding requests we
> generate so we'll always have a buffer to reply to SRP_CRED_REQ et al.
>
> If SRQ_SQ_SIZE is 64, we set .can_queue to 63 which will leave a buffer
> available for the reply.
>
> Now, if the target sends us multiple SRP_CRED_REQ without waiting for a
> reply, then we could use up buffers and not be able to send requests,
> but that would be a protocol violation on the target's side.

(resending as plain text)

I have been looking further into this. There is a slight asymmetry in
the current transmit buffer allocation code: one buffer element is
reserved on the target for task management requests but not on the
initiator. So - at least in theory - it is possible that all elements
of the initiator transmit ring are allocated for SRP_CMD requests at
the time an SRP_TSK_MGMT request should be sent. Wouldn't it be more
symmetric to change __srp_get_tx_iu() as follows (diff against the
current for-next branch) ?

--- a/drivers/infiniband/ulp/srp/
ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -989,14 +989,14 @@ static void srp_send_completion(struct ib_cq
*cq, void *target_ptr)
 static struct srp_iu *__srp_get_tx_iu(struct srp_target_port *target,
                                        enum srp_request_type req_type)
 {
-       s32 min = (req_type == SRP_REQ_TASK_MGMT) ? 1 : 2;
+       s32 rsv = (req_type == SRP_REQ_TASK_MGMT) ? 0 : 1;

        srp_send_completion(target->send_cq, target);

-       if (target->tx_head - target->tx_tail >= SRP_SQ_SIZE)
+       if (target->tx_head - target->tx_tail >= SRP_SQ_SIZE - rsv)
                return NULL;

-       if (target->req_lim < min) {
+       if (target->req_lim <= rsv) {
                ++target->zero_req_lim;
                return NULL;
        }

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to