On Thu, Jan 4, 2018 at 12:21 PM, Andy Gospodarek <a...@greyhouse.net> wrote: > From: Andy Gospodarek <go...@broadcom.com> > > This implements the changes needed for the bnxt_en driver to add support > for dynamic interrupt moderation per ring. > > This does add additional counters in the receive path, but testing shows > that any additional instructions are offset by throughput gain when the > default configuration is for low latency. > > Signed-off-by: Andy Gospodarek <go...@broadcom.com> > Cc: Michael Chan <mc...@broadcom.com>
Andy, looks good in general. I just have a few comments below. These minor issues can be cleaned up after merge if you want. .... > +int bnxt_hwrm_set_ring_coal(struct bnxt *bp, struct bnxt_napi *bnapi) > +{ > + struct hwrm_ring_cmpl_ring_cfg_aggint_params_input req_rx = {0}; > + struct bnxt_cp_ring_info *cpr = &bnapi->cp_ring; > + struct bnxt_coal coal; > + unsigned int grp_idx; > + int rc = 0; > + > + /* Tick values in micro seconds. > + * 1 coal_buf x bufs_per_record = 1 completion record. > + */ > + memcpy(&coal, &bp->rx_coal, sizeof(struct bnxt_coal)); > + > + coal.coal_ticks = cpr->rx_ring_coal.coal_ticks; > + coal.coal_bufs = cpr->rx_ring_coal.coal_bufs; > + > + if (!bnapi->rx_ring) > + return -ENODEV; > + > + bnxt_hwrm_cmd_hdr_init(bp, &req_rx, > + HWRM_RING_CMPL_RING_CFG_AGGINT_PARAMS, -1, -1); > + > + bnxt_hwrm_set_coal_params(&coal, &req_rx); > + > + mutex_lock(&bp->hwrm_cmd_lock); > + grp_idx = bnapi->index; > + > + req_rx.ring_id = cpu_to_le16(bp->grp_info[grp_idx].cp_fw_ring_id); > + > + rc = _hwrm_send_message(bp, &req_rx, sizeof(req_rx), > + HWRM_CMD_TIMEOUT); > + mutex_unlock(&bp->hwrm_cmd_lock); You can use the hwrm_send_message() variant that does not require you to take the mutex. You only need this variant and take the mutex if you need to check the firmware reply. > + return rc; > +} > + > int bnxt_hwrm_set_coal(struct bnxt *bp) > { > int i, rc = 0; > @@ -5705,7 +5753,11 @@ static void bnxt_enable_napi(struct bnxt *bp) > int i; > > for (i = 0; i < bp->cp_nr_rings; i++) { We only need to enable this for every completion ring that has an RX ring. In some cases, for example when XDP is enabled, there will be a set of completion rings with only TX rings. So I think we can optimize this for completion rings with RX only. > + struct bnxt_cp_ring_info *cpr = &bp->bnapi[i]->cp_ring; > bp->bnapi[i]->in_reset = false; > + > + INIT_WORK(&cpr->am.work, bnxt_dim_work); > + cpr->am.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE; > napi_enable(&bp->bnapi[i]->napi); > } > }