Messages in this thread | | | From | Michal Schmidt <> | Date | Wed, 13 Mar 2024 14:47:17 +0100 | Subject | Re: [PATCH net] i40e: Enforce software interrupt during busy-poll exit |
| |
On Wed, Mar 13, 2024 at 1:55 PM Ivan Vecera <ivecera@redhat.com> wrote: > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h > index 9b701615c7c6..4d2b05de6c63 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e.h > +++ b/drivers/net/ethernet/intel/i40e/i40e.h > @@ -908,6 +908,7 @@ struct i40e_q_vector { > struct rcu_head rcu; /* to avoid race with update stats on free */ > char name[I40E_INT_NAME_STR_LEN]; > bool arm_wb_state; > + bool in_busy_poll; > int irq_num; /* IRQ assigned to this q_vector */ > } ____cacheline_internodealigned_in_smp; > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c > index 89a3401d20ab..1ea6d06b0acc 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c > @@ -3915,6 +3915,12 @@ static void i40e_vsi_configure_msix(struct i40e_vsi *vsi) > q_vector->tx.target_itr >> 1); > q_vector->tx.current_itr = q_vector->tx.target_itr; > > + /* Set ITR for software interrupts triggered after exiting > + * busy-loop polling. > + */ > + wr32(hw, I40E_PFINT_ITRN(I40E_SW_ITR, vector - 1), > + I40E_ITR_20K); > + > wr32(hw, I40E_PFINT_RATEN(vector - 1), > i40e_intrl_usec_to_reg(vsi->int_rate_limit)); > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_register.h b/drivers/net/ethernet/intel/i40e/i40e_register.h > index 14ab642cafdb..baa6bb68bcf8 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_register.h > +++ b/drivers/net/ethernet/intel/i40e/i40e_register.h > @@ -335,6 +335,8 @@ > #define I40E_PFINT_DYN_CTLN_INTERVAL_SHIFT 5 > #define I40E_PFINT_DYN_CTLN_SW_ITR_INDX_ENA_SHIFT 24 > #define I40E_PFINT_DYN_CTLN_SW_ITR_INDX_ENA_MASK I40E_MASK(0x1, I40E_PFINT_DYN_CTLN_SW_ITR_INDX_ENA_SHIFT) > +#define I40E_PFINT_DYN_CTLN_SW_ITR_INDX_SHIFT 25 > +#define I40E_PFINT_DYN_CTLN_SW_ITR_INDX_MASK I40E_MASK(0x3, I40E_PFINT_DYN_CTLN_SW_ITR_INDX_SHIFT) > #define I40E_PFINT_ICR0 0x00038780 /* Reset: CORER */ > #define I40E_PFINT_ICR0_INTEVENT_SHIFT 0 > #define I40E_PFINT_ICR0_INTEVENT_MASK I40E_MASK(0x1, I40E_PFINT_ICR0_INTEVENT_SHIFT) > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c > index 0d7177083708..356c3140adf3 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c > @@ -2658,8 +2658,22 @@ static inline u32 i40e_buildreg_itr(const int type, u16 itr) > return val; > } > > -/* a small macro to shorten up some long lines */ > -#define INTREG I40E_PFINT_DYN_CTLN > +static inline u32 i40e_buildreg_swint(int type) > +{ > + u32 val; > + > + /* 1. Enable the interrupt > + * 2. Do not modify any ITR interval > + * 3. Trigger a SW interrupt specified by type > + */ > + val = I40E_PFINT_DYN_CTLN_INTENA_MASK | > + I40E_PFINT_DYN_CTLN_ITR_INDX_MASK | /* set noitr */ > + I40E_PFINT_DYN_CTLN_SWINT_TRIG_MASK | > + I40E_PFINT_DYN_CTLN_SW_ITR_INDX_ENA_MASK | > + FIELD_PREP(I40E_PFINT_DYN_CTLN_SW_ITR_INDX_MASK, type); > + > + return val; > +}
This function is called only from one place and with a constant argument. Does it really need to be a function, as opposed to a constant? Or are you going to add more callers soon?
> > /* The act of updating the ITR will cause it to immediately trigger. In order > * to prevent this from throwing off adaptive update statistics we defer the > @@ -2702,8 +2716,8 @@ static inline void i40e_update_enable_itr(struct i40e_vsi *vsi, > */ > if (q_vector->rx.target_itr < q_vector->rx.current_itr) { > /* Rx ITR needs to be reduced, this is highest priority */ > - intval = i40e_buildreg_itr(I40E_RX_ITR, > - q_vector->rx.target_itr); > + wr32(hw, I40E_PFINT_ITRN(I40E_RX_ITR, q_vector->reg_idx), > + q_vector->rx.target_itr >> 1); > q_vector->rx.current_itr = q_vector->rx.target_itr; > q_vector->itr_countdown = ITR_COUNTDOWN_START; > } else if ((q_vector->tx.target_itr < q_vector->tx.current_itr) || > @@ -2712,25 +2726,33 @@ static inline void i40e_update_enable_itr(struct i40e_vsi *vsi, > /* Tx ITR needs to be reduced, this is second priority > * Tx ITR needs to be increased more than Rx, fourth priority > */ > - intval = i40e_buildreg_itr(I40E_TX_ITR, > - q_vector->tx.target_itr); > + wr32(hw, I40E_PFINT_ITRN(I40E_TX_ITR, q_vector->reg_idx), > + q_vector->tx.target_itr >> 1); > q_vector->tx.current_itr = q_vector->tx.target_itr; > q_vector->itr_countdown = ITR_COUNTDOWN_START; > } else if (q_vector->rx.current_itr != q_vector->rx.target_itr) { > /* Rx ITR needs to be increased, third priority */ > - intval = i40e_buildreg_itr(I40E_RX_ITR, > - q_vector->rx.target_itr); > + wr32(hw, I40E_PFINT_ITRN(I40E_RX_ITR, q_vector->reg_idx), > + q_vector->rx.target_itr >> 1); > q_vector->rx.current_itr = q_vector->rx.target_itr; > q_vector->itr_countdown = ITR_COUNTDOWN_START; > } else { > /* No ITR update, lowest priority */ > - intval = i40e_buildreg_itr(I40E_ITR_NONE, 0); > if (q_vector->itr_countdown) > q_vector->itr_countdown--; > } > > - if (!test_bit(__I40E_VSI_DOWN, vsi->state)) > - wr32(hw, INTREG(q_vector->reg_idx), intval); > + /* Do not enable interrupt if VSI is down */ > + if (test_bit(__I40E_VSI_DOWN, vsi->state)) > + return; > + > + if (!q_vector->in_busy_poll) { > + intval = i40e_buildreg_itr(I40E_ITR_NONE, 0); > + } else { > + q_vector->in_busy_poll = false; > + intval = i40e_buildreg_swint(I40E_SW_ITR); > + } > + wr32(hw, I40E_PFINT_DYN_CTLN(q_vector->reg_idx), intval); > } > > /** > @@ -2845,6 +2867,8 @@ int i40e_napi_poll(struct napi_struct *napi, int budget) > */ > if (likely(napi_complete_done(napi, work_done))) > i40e_update_enable_itr(vsi, q_vector); > + else > + q_vector->in_busy_poll = true; > > return min(work_done, budget - 1); > } > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h > index abf15067eb5d..2cdc7de6301c 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h > @@ -68,6 +68,7 @@ enum i40e_dyn_idx { > /* these are indexes into ITRN registers */ > #define I40E_RX_ITR I40E_IDX_ITR0 > #define I40E_TX_ITR I40E_IDX_ITR1 > +#define I40E_SW_ITR I40E_IDX_ITR2 > > /* Supported RSS offloads */ > #define I40E_DEFAULT_RSS_HENA ( \ > -- > 2.43.0 >
| |