Reducing the number of IB interrupts caused by the SRP initiator
Subsystems (2)
Hello,
Once I noticed that the SRP initiator triggers two IB interrupts per I/O (more precise, per srp_queuecommand() call) I started looking at reducing the number of IB interrupts. The patch below didn't help. Does this mean that the patch below isn't complete or is this because of the hw driver used during the test (mlx4) ?
Bart.
drivers/infiniband/ulp/srp/ib_srp.c +3 -2
@@ -241,7 +241,7 @@ static int srp_create_target_ib(struct srp_target_port *target) init_attr->cap.max_recv_wr =SRP_RQ_SIZE ; init_attr->cap.max_recv_sge =1 ; init_attr->cap.max_send_sge =1 ; - init_attr->sq_sig_type =IB_SIGNAL_ALL_WR ; + init_attr->sq_sig_type =IB_SIGNAL_REQ_WR ; init_attr->qp_type =IB_QPT_RC ; init_attr->send_cq = target->cq; init_attr->recv_cq = target->cq;
@@ -1001,7 +1001,8 @@ static int __srp_post_send(struct srp_target_port *target, wr.sg_list = &list; wr.num_sge =1 ; wr.opcode =IB_WR_SEND ; - wr.send_flags =IB_SEND_SIGNALED ; + wr.send_flags = target->tx_head - target->tx_tail ==SRP_SQ_SIZE -1 + ?IB_SEND_SIGNALED :0 ; ret =ib_post_send (target->qp, &wr, &bad_wr);
--
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
Quoted 7 lines · Bart Van Assche
Hello,
Once I noticed that the SRP initiator triggers two IB interrupts per I/O (more precise, per
srp_queuecommand()call) I started looking at reducing the number of IB interrupts. The patch below didn't help. Does this mean that the patch below isn't complete or is this because of the hw driver used during the test (mlx4) ?
When you say that the patch did not help, do you mean that it did not reduce the rate of interrupts? I also think that you have a race here since the head and tail operations are not protected (I don't know the SRP driver I think locking should be done here). It would be better if you'd choose a value that is a fraction of the queue size to request a completion so that you don't get to a situation where your queue is full and you the sender is waiting for it to be cleared. Also, you did not touch the completion handler. Seems to me you should increment the tail for each completion by the number of completions you skipped. Failing to do so will keep cause the completion to be requested each post after the first time it skipped queue len completions.
Quoted 27 lines
Bart.
drivers/infiniband/ulp/srp/ib_srp.c +3
-3@@ -241,7 +241,7 @@ static int srp_create_target_ib(struct srp_target_port *target) init_attr->cap.max_recv_wr = SRP_RQ_SIZE; init_attr->cap.max_recv_sge = 1; init_attr->cap.max_send_sge = 1; - init_attr->sq_sig_type = IB_SIGNAL_ALL_WR; + init_attr->sq_sig_type = IB_SIGNAL_REQ_WR; init_attr->qp_type = IB_QPT_RC; init_attr->send_cq = target->cq; init_attr->recv_cq = target->cq;@@ -1001,7 +1001,8 @@ static int __srp_post_send(struct srp_target_port *target, wr.sg_list = &list; wr.num_sge = 1; wr.opcode = IB_WR_SEND; - wr.send_flags = IB_SEND_SIGNALED; + wr.send_flags = target->tx_head - target->tx_tail == SRP_SQ_SIZE - 1 + ? IB_SEND_SIGNALED : 0; ret = ib_post_send(target->qp, &wr, &bad_wr); --
Quoted 4 lines
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
--
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
Quoted 11 lines · Eli Cohen
Quoted 5 lines · Bart Van Assche
Once I noticed that the SRP initiator triggers two IB interrupts per I/O (more precise, per
srp_queuecommand()call) I started looking at reducing the number of IB interrupts. The patch below didn't help. Does this mean that the patch below isn't complete or is this because of the hw driver used during the test (mlx4) ?When you say that the patch did not help, do you mean that it did not reduce the rate of interrupts?
Yes.
Quoted 3 lines
I also think that you have a race here
since the head and tail operations are not protected (I don't know the SRP driver I think locking should be done here).
The race was already present in the original SRP driver: tx_tail is not consistently protected by target->scsi_host->host_lock. As far as my analysis of the tx_tail access patterns is correct this race doesn't hurt in the original SRP driver. But you are right, I should carefully analyze whether this race is acceptable.
Quoted 8 lines
It would be better if
you'd choose a value that is a fraction of the queue size to request a completion so that you don't get to a situation where your queue is full and you the sender is waiting for it to be cleared. Also, you did not touch the completion handler. Seems to me you should increment the tail for each completion by the number of completions you skipped. Failing to do so will keep cause the completion to be requested each post after the first time it skipped queue len completions.
Thanks -- will have a look at this.
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