| Message ID | 20240529112337.3639084-4-maciej.fijalkowski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice: fix AF_XDP ZC timeout and concurrency issues | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/apply | fail | Patch does not apply to net-0 |
On 5/29/2024 4:23 AM, Maciej Fijalkowski wrote: > > Given that ice_qp_dis() is called under rtnl_lock, synchronize_net() can > be called instead of synchronize_rcu() so that XDP rings can finish its > job in a faster way. Also let us do this as earlier in XSK queue disable > flow. > > Additionally, turn off regular Tx queue before disabling irqs and NAPI. > > Fixes: 2d4238f55697 ("ice: Add support for AF_XDP") > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_xsk.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c > index 4f606a1055b0..e93cb0ca4106 100644 > --- a/drivers/net/ethernet/intel/ice/ice_xsk.c > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c > @@ -53,7 +53,6 @@ static void ice_qp_clean_rings(struct ice_vsi *vsi, u16 q_idx) > { > ice_clean_tx_ring(vsi->tx_rings[q_idx]); > if (ice_is_xdp_ena_vsi(vsi)) { > - synchronize_rcu(); > ice_clean_tx_ring(vsi->xdp_rings[q_idx]); > } With only one statement left in the block you'll probably want to remove the {}'s sln > ice_clean_rx_ring(vsi->rx_rings[q_idx]); > @@ -180,11 +179,12 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx) > usleep_range(1000, 2000); > } > > + synchronize_net(); > + netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > + > ice_qvec_dis_irq(vsi, rx_ring, q_vector); > ice_qvec_toggle_napi(vsi, q_vector, false); > > - netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > - > ice_fill_txq_meta(vsi, tx_ring, &txq_meta); > err = ice_vsi_stop_tx_ring(vsi, ICE_NO_RESET, 0, tx_ring, &txq_meta); > if (err) > -- > 2.34.1 > >
>-----Original Message----- >From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of >Fijalkowski, Maciej >Sent: Wednesday, May 29, 2024 4:54 PM >To: intel-wired-lan@lists.osuosl.org >Cc: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>; Zaremba, Larysa ><larysa.zaremba@intel.com>; netdev@vger.kernel.org; Kubiak, Michal ><michal.kubiak@intel.com>; Nguyen, Anthony L ><anthony.l.nguyen@intel.com>; Karlsson, Magnus ><magnus.karlsson@intel.com> >Subject: [Intel-wired-lan] [PATCH v2 iwl-net 3/8] ice: replace synchronize_rcu >with synchronize_net > >Given that ice_qp_dis() is called under rtnl_lock, synchronize_net() can be called >instead of synchronize_rcu() so that XDP rings can finish its job in a faster way. >Also let us do this as earlier in XSK queue disable flow. > >Additionally, turn off regular Tx queue before disabling irqs and NAPI. > >Fixes: 2d4238f55697 ("ice: Add support for AF_XDP") >Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> >--- > drivers/net/ethernet/intel/ice/ice_xsk.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
On Fri, May 31, 2024 at 04:49:01PM -0700, Nelson, Shannon wrote: > On 5/29/2024 4:23 AM, Maciej Fijalkowski wrote: > > > > Given that ice_qp_dis() is called under rtnl_lock, synchronize_net() can > > be called instead of synchronize_rcu() so that XDP rings can finish its > > job in a faster way. Also let us do this as earlier in XSK queue disable > > flow. > > > > Additionally, turn off regular Tx queue before disabling irqs and NAPI. > > > > Fixes: 2d4238f55697 ("ice: Add support for AF_XDP") > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > --- > > drivers/net/ethernet/intel/ice/ice_xsk.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c > > index 4f606a1055b0..e93cb0ca4106 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_xsk.c > > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c > > @@ -53,7 +53,6 @@ static void ice_qp_clean_rings(struct ice_vsi *vsi, u16 q_idx) > > { > > ice_clean_tx_ring(vsi->tx_rings[q_idx]); > > if (ice_is_xdp_ena_vsi(vsi)) { > > - synchronize_rcu(); > > ice_clean_tx_ring(vsi->xdp_rings[q_idx]); > > } > > With only one statement left in the block you'll probably want to remove the > {}'s Thanks for catching this. To explain myself here, I was getting rid of a patch which removes the XDP prog presence check altogether because it is a false positive - we can't get into these queue pair disabling/enabling functions if there is no XDP prog on interface. And the reason why I was tossing this out of patch set was that it was not -net candidate. Thanks again and will fix on v3. > > sln > > > > ice_clean_rx_ring(vsi->rx_rings[q_idx]); > > @@ -180,11 +179,12 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx) > > usleep_range(1000, 2000); > > } > > > > + synchronize_net(); > > + netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > > + > > ice_qvec_dis_irq(vsi, rx_ring, q_vector); > > ice_qvec_toggle_napi(vsi, q_vector, false); > > > > - netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > > - > > ice_fill_txq_meta(vsi, tx_ring, &txq_meta); > > err = ice_vsi_stop_tx_ring(vsi, ICE_NO_RESET, 0, tx_ring, &txq_meta); > > if (err) > > -- > > 2.34.1 > > > > >
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index 4f606a1055b0..e93cb0ca4106 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -53,7 +53,6 @@ static void ice_qp_clean_rings(struct ice_vsi *vsi, u16 q_idx) { ice_clean_tx_ring(vsi->tx_rings[q_idx]); if (ice_is_xdp_ena_vsi(vsi)) { - synchronize_rcu(); ice_clean_tx_ring(vsi->xdp_rings[q_idx]); } ice_clean_rx_ring(vsi->rx_rings[q_idx]); @@ -180,11 +179,12 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx) usleep_range(1000, 2000); } + synchronize_net(); + netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); + ice_qvec_dis_irq(vsi, rx_ring, q_vector); ice_qvec_toggle_napi(vsi, q_vector, false); - netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); - ice_fill_txq_meta(vsi, tx_ring, &txq_meta); err = ice_vsi_stop_tx_ring(vsi, ICE_NO_RESET, 0, tx_ring, &txq_meta); if (err)
Given that ice_qp_dis() is called under rtnl_lock, synchronize_net() can be called instead of synchronize_rcu() so that XDP rings can finish its job in a faster way. Also let us do this as earlier in XSK queue disable flow. Additionally, turn off regular Tx queue before disabling irqs and NAPI. Fixes: 2d4238f55697 ("ice: Add support for AF_XDP") Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- drivers/net/ethernet/intel/ice/ice_xsk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)