On Tue, Feb 19, 2019 at 11:08:58AM +0100, Bernard Metzler wrote:
> +/*
> + * Must be called with SQ locked.
> + * To avoid complete SQ starvation by constant inbound READ requests,
> + * the active IRQ will not be served after qp->irq_burst, if the
> + * SQ has pending work.
> + */
> +int siw_activate_tx(struct siw_qp *qp)
> +{
> + struct siw_sqe *irqe, *sqe;
> + struct siw_wqe *wqe = tx_wqe(qp);
> + int rv = 1;
> +
> + irqe = &qp->irq[qp->irq_get % qp->attrs.irq_size];
> +
> + if (irqe->flags & SIW_WQE_VALID) {
> + sqe = sq_get_next(qp);
> +
> + /*
> + * Avoid local WQE processing starvation in case
> + * of constant inbound READ request stream
> + */
> + if (sqe && ++qp->irq_burst >= SIW_IRQ_MAXBURST_SQ_ACTIVE) {
> + qp->irq_burst = 0;
> + goto skip_irq;
> + }
> + memset(wqe->mem, 0, sizeof(*wqe->mem) * SIW_MAX_SGE);
> + wqe->wr_status = SIW_WR_QUEUED;
> +
> + /* start READ RESPONSE */
> + wqe->sqe.opcode = SIW_OP_READ_RESPONSE;
> + wqe->sqe.flags = 0;
> + if (irqe->num_sge) {
> + wqe->sqe.num_sge = 1;
> + wqe->sqe.sge[0].length = irqe->sge[0].length;
> + wqe->sqe.sge[0].laddr = irqe->sge[0].laddr;
> + wqe->sqe.sge[0].lkey = irqe->sge[0].lkey;
> + } else {
> + wqe->sqe.num_sge = 0;
> + }
> +
> + /* Retain original RREQ's message sequence number for
> + * potential error reporting cases.
> + */
> + wqe->sqe.sge[1].length = irqe->sge[1].length;
> +
> + wqe->sqe.rkey = irqe->rkey;
> + wqe->sqe.raddr = irqe->raddr;
> +
> + wqe->processed = 0;
> + qp->irq_get++;
> +
> + /* mark current IRQ entry free */
> + smp_store_mb(irqe->flags, 0);
I really dislike seeing attempts at lock-free constructions like this
in drivers...
.. and I'm having doubts it is right, as directly above we read
irqe->flags without a barrier/atomic/etc in the if.
What is this attempting to do, and why can't it have an standard
scheme?
'lock-free' with test_bit/set_bit/clear_bit atomics would be better
that this..
Same remark for all these magic cases.
Jason
