| Message ID | 20220510084443.14473-6-ihuguet@redhat.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | sfc: refactor of efx_set_channels | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | warning | WARNING: line length of 88 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, May 10, 2022 at 10:44:43AM +0200, Íñigo Huguet wrote: > All parameters related to what channels are used for RX, TX and/or XDP > are calculated in efx_probe_interrupts or its called function > efx_allocate_msix_channels. > > tx_channel_offset was recalculated needlessly in efx_set_queues. Remove > this from here since it's more coherent to calculate it only once, in > the same place than the rest of channels parameters. If MSIX is not used, > this value was not set in efx_probe_interrupts, so let's do it now. > > The value calculated in efx_set_queues was wrong anyway, because with > the addition of the support for XDP, additional channels had been added > after the TX channels, and efx->n_channels - efx->n_tx_channels didn't > point to the beginning of the TX channels any more. Apart from the reformatting this fixes a bug in the existing code. Please submit this bug fix part again as an individual patch. Martin > Signed-off-by: Íñigo Huguet <ihuguet@redhat.com> > --- > drivers/net/ethernet/sfc/efx_channels.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c > index f6634faa1ec4..b9bbef07bb5e 100644 > --- a/drivers/net/ethernet/sfc/efx_channels.c > +++ b/drivers/net/ethernet/sfc/efx_channels.c > @@ -220,14 +220,9 @@ static int efx_allocate_msix_channels(struct efx_nic *efx, > n_channels -= efx->n_xdp_channels; > > if (efx_separate_tx_channels) { > - efx->n_tx_channels = > - min(max(n_channels / 2, 1U), > - efx->max_tx_channels); > - efx->tx_channel_offset = > - n_channels - efx->n_tx_channels; > - efx->n_rx_channels = > - max(n_channels - > - efx->n_tx_channels, 1U); > + efx->n_tx_channels = min(max(n_channels / 2, 1U), efx->max_tx_channels); > + efx->tx_channel_offset = n_channels - efx->n_tx_channels; > + efx->n_rx_channels = max(n_channels - efx->n_tx_channels, 1U); > } else { > efx->n_tx_channels = min(n_channels, efx->max_tx_channels); > efx->tx_channel_offset = 0; > @@ -303,6 +298,7 @@ int efx_probe_interrupts(struct efx_nic *efx) > efx->n_channels = 1; > efx->n_rx_channels = 1; > efx->n_tx_channels = 1; > + efx->tx_channel_offset = 0; > efx->n_xdp_channels = 0; > efx->xdp_channel_offset = efx->n_channels; > rc = pci_enable_msi(efx->pci_dev); > @@ -323,6 +319,7 @@ int efx_probe_interrupts(struct efx_nic *efx) > efx->n_channels = 1 + (efx_separate_tx_channels ? 1 : 0); > efx->n_rx_channels = 1; > efx->n_tx_channels = 1; > + efx->tx_channel_offset = 1; > efx->n_xdp_channels = 0; > efx->xdp_channel_offset = efx->n_channels; > efx->legacy_irq = efx->pci_dev->irq; > @@ -952,10 +949,6 @@ int efx_set_queues(struct efx_nic *efx) > unsigned int queue_num = 0; > int rc; > > - efx->tx_channel_offset = > - efx_separate_tx_channels ? > - efx->n_channels - efx->n_tx_channels : 0; > - > /* We need to mark which channels really have RX and TX queues, and > * adjust the TX queue numbers if we have separate RX/TX only channels. > */ > -- > 2.34.1
diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c index f6634faa1ec4..b9bbef07bb5e 100644 --- a/drivers/net/ethernet/sfc/efx_channels.c +++ b/drivers/net/ethernet/sfc/efx_channels.c @@ -220,14 +220,9 @@ static int efx_allocate_msix_channels(struct efx_nic *efx, n_channels -= efx->n_xdp_channels; if (efx_separate_tx_channels) { - efx->n_tx_channels = - min(max(n_channels / 2, 1U), - efx->max_tx_channels); - efx->tx_channel_offset = - n_channels - efx->n_tx_channels; - efx->n_rx_channels = - max(n_channels - - efx->n_tx_channels, 1U); + efx->n_tx_channels = min(max(n_channels / 2, 1U), efx->max_tx_channels); + efx->tx_channel_offset = n_channels - efx->n_tx_channels; + efx->n_rx_channels = max(n_channels - efx->n_tx_channels, 1U); } else { efx->n_tx_channels = min(n_channels, efx->max_tx_channels); efx->tx_channel_offset = 0; @@ -303,6 +298,7 @@ int efx_probe_interrupts(struct efx_nic *efx) efx->n_channels = 1; efx->n_rx_channels = 1; efx->n_tx_channels = 1; + efx->tx_channel_offset = 0; efx->n_xdp_channels = 0; efx->xdp_channel_offset = efx->n_channels; rc = pci_enable_msi(efx->pci_dev); @@ -323,6 +319,7 @@ int efx_probe_interrupts(struct efx_nic *efx) efx->n_channels = 1 + (efx_separate_tx_channels ? 1 : 0); efx->n_rx_channels = 1; efx->n_tx_channels = 1; + efx->tx_channel_offset = 1; efx->n_xdp_channels = 0; efx->xdp_channel_offset = efx->n_channels; efx->legacy_irq = efx->pci_dev->irq; @@ -952,10 +949,6 @@ int efx_set_queues(struct efx_nic *efx) unsigned int queue_num = 0; int rc; - efx->tx_channel_offset = - efx_separate_tx_channels ? - efx->n_channels - efx->n_tx_channels : 0; - /* We need to mark which channels really have RX and TX queues, and * adjust the TX queue numbers if we have separate RX/TX only channels. */
All parameters related to what channels are used for RX, TX and/or XDP are calculated in efx_probe_interrupts or its called function efx_allocate_msix_channels. tx_channel_offset was recalculated needlessly in efx_set_queues. Remove this from here since it's more coherent to calculate it only once, in the same place than the rest of channels parameters. If MSIX is not used, this value was not set in efx_probe_interrupts, so let's do it now. The value calculated in efx_set_queues was wrong anyway, because with the addition of the support for XDP, additional channels had been added after the TX channels, and efx->n_channels - efx->n_tx_channels didn't point to the beginning of the TX channels any more. Signed-off-by: Íñigo Huguet <ihuguet@redhat.com> --- drivers/net/ethernet/sfc/efx_channels.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)