| Message ID | 20220913140206.64330-1-kamil.maziarz@intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [net,v2] i40e: Fix not setting xps_cpus after reset | expand |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Kamil Maziarz > Sent: Tuesday, September 13, 2022 7:32 PM > To: intel-wired-lan@lists.osuosl.org > Cc: Jaron, MichalX <michalx.jaron@intel.com>; Maziarz, Kamil > <kamil.maziarz@intel.com> > Subject: [Intel-wired-lan] [PATCH net v2] i40e: Fix not setting xps_cpus after > reset > > From: Michal Jaron <michalx.jaron@intel.com> > > During tx rings configuration default XPS queue config is set and > __I40E_TX_XPS_INIT_DONE is locked. XPS CPUs maps are cleared in every > reset by netdev_set_num_tc() call regardless it was set by user or driver. If > reset with reinit occurs __I40E_TX_XPS_INIT_DONE flag is removed and XPS > mapping is set to default again but after reset without reinit this flag is still set > and XPS CPUs to queues mapping stays cleared. > > Add code to preserve xps_cpus mapping as cpumask for every queue and > restore those mapping at the end of reset. > > Fixes: 6f853d4f8e93 ("i40e: allow XPS with QoS enabled") > Signed-off-by: Michal Jaron <michalx.jaron@intel.com> > Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com> > --- > drivers/net/ethernet/intel/i40e/i40e.h | 6 ++ > drivers/net/ethernet/intel/i40e/i40e_main.c | 109 > ++++++++++++++++++++ > 2 files changed, 115 insertions(+) > Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel)
On Tue, 13 Sep 2022 16:02:06 +0200 Kamil Maziarz <kamil.maziarz@intel.com> wrote: > From: Michal Jaron <michalx.jaron@intel.com> > > ... Tony, I'd like to ask what did happen with this commit? During Monday you applied the patch to 40GBe branch as: commit ce8d8ae335f469a399805f56e74b98390f327184 Author: Michal Jaron <michalx.jaron@intel.com> Date: Tue Sep 13 16:02:06 2022 +0200 i40e: Fix not setting xps_cpus after reset During tx rings configuration default XPS queue config is set and __I40E_TX_XPS_INIT_DONE is locked. XPS CPUs maps are cleared in every reset by netdev_set_num_tc() call regardless it was set by user or driver. If reset with reinit occurs __I40E_TX_XPS_INIT_DONE flag is removed and XPS mapping is set to default again but after reset without reinit this flag is still set and XPS CPUs to queues mapping stays cleared. Add code to preserve xps_cpus mapping as cpumask for every queue and restore those mapping at the end of reset. Fixes: 6f853d4f8e93 ("i40e: allow XPS with QoS enabled") Signed-off-by: Michal Jaron <michalx.jaron@intel.com> Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com> Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel) Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> but later the branch was force-pushed, the commit is not present there anymore nor in upstream... Was it silently dropped? What was the reason? Thanks for clarification. Ivan
Hi Ivan, On 9/29/2022 12:53 AM, Ivan Vecera wrote: > On Tue, 13 Sep 2022 16:02:06 +0200 > Kamil Maziarz <kamil.maziarz@intel.com> wrote: > >> From: Michal Jaron <michalx.jaron@intel.com> >> >> ... > > Tony, I'd like to ask what did happen with this commit? During Monday you applied the patch to > 40GBe branch as: > > commit ce8d8ae335f469a399805f56e74b98390f327184 > Author: Michal Jaron <michalx.jaron@intel.com> > Date: Tue Sep 13 16:02:06 2022 +0200 > > i40e: Fix not setting xps_cpus after reset > > During tx rings configuration default XPS queue config is set and > __I40E_TX_XPS_INIT_DONE is locked. XPS CPUs maps are cleared in > every reset by netdev_set_num_tc() call regardless it was set by > user or driver. If reset with reinit occurs __I40E_TX_XPS_INIT_DONE > flag is removed and XPS mapping is set to default again but after > reset without reinit this flag is still set and XPS CPUs to queues > mapping stays cleared. > > Add code to preserve xps_cpus mapping as cpumask for every queue > and restore those mapping at the end of reset. > > Fixes: 6f853d4f8e93 ("i40e: allow XPS with QoS enabled") > Signed-off-by: Michal Jaron <michalx.jaron@intel.com> > Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com> > Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel) > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> > > but later the branch was force-pushed, the commit is not present there anymore nor > in upstream... I'm not sure exactly where you are referring to when you say upstream, but it is still currently on the IWL trees [1]. > Was it silently dropped? What was the reason? When sent to netdev, there are questions/concerns with the patch [2]. As I mentioned in the netdev patch, in an effort to try to get the other patches in the series accepted for this kernel (since it's closing soon), I've omitted this patch from the pull request while the discussion continues. This may or may not be dropped from the tree based on where the netdev discussion goes. For the IWL trees, dev-queue is the branch that you should follow. The other branches are branches for pull requests so their content is fluid and doesn't necessarily represent what is current with IWL patches. Thanks, Tony [1] https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=e312239075d77f11b45c3997a4e89153393c259d [2] https://lore.kernel.org/netdev/20220927182933.30d691d2@kernel.org/#t
On 9/29/2022 8:47 AM, Tony Nguyen wrote: > I'm not sure exactly where you are referring to when you say upstream, > but it is still currently on the IWL trees [1]. ... > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=e312239075d77f11b45c3997a4e89153393c259d Sorry, gave the wrong link/commit. Should be: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=aea4f7bb6c656b852771747538ca264b72967055 > [2] https://lore.kernel.org/netdev/20220927182933.30d691d2@kernel.org/#t > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
On Thu, 29 Sep 2022 08:54:39 -0700 Tony Nguyen <anthony.l.nguyen@intel.com> wrote: > On 9/29/2022 8:47 AM, Tony Nguyen wrote: > > I'm not sure exactly where you are referring to when you say upstream, > > but it is still currently on the IWL trees [1]. > > ... > > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=e312239075d77f11b45c3997a4e89153393c259d > > Sorry, gave the wrong link/commit. Should be: > https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=aea4f7bb6c656b852771747538ca264b72967055 Hmm, ok... it is now in next repo... I'm a little bit confused why the patch with Fixes: tag is present in next-queue repo. This is not a -next material or is it? Ivan
On 9/30/2022 4:21 AM, Ivan Vecera wrote: > On Thu, 29 Sep 2022 08:54:39 -0700 > Tony Nguyen <anthony.l.nguyen@intel.com> wrote: > >> On 9/29/2022 8:47 AM, Tony Nguyen wrote: >>> I'm not sure exactly where you are referring to when you say upstream, >>> but it is still currently on the IWL trees [1]. >> >> ... >> >>> [1] >>> https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=e312239075d77f11b45c3997a4e89153393c259d >> >> Sorry, gave the wrong link/commit. Should be: >> https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=aea4f7bb6c656b852771747538ca264b72967055 > > Hmm, ok... it is now in next repo... I'm a little bit confused why the patch with Fixes: tag is present in next-queue repo. > This is not a -next material or is it? For net patches, they are applied to both trees to help minimize conflicts when they get rebased into next. Here's the net link of it [1], I just used the -next link as it's what I had handy. Thanks, Tony [1] https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git/commit/?h=dev-queue&id=7670a1167933ca825bbe4d3614b6936e1a6f7c19
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index d86b6d349ea9..e01af5943bfe 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -1306,4 +1306,10 @@ static inline u32 i40e_is_tc_mqprio_enabled(struct i40e_pf *pf) return pf->flags & I40E_FLAG_TC_MQPRIO; } +/* reverse XPS CPUs to tx queues map */ +struct i40e_qmap_rev { + struct cpumask cpus; + int vsi_id; +}; + #endif /* _I40E_H_ */ diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index e3d9804aeb25..14e29580ebed 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -10793,6 +10793,83 @@ static int i40e_reset(struct i40e_pf *pf) return ret; } +#ifdef CONFIG_XPS +/** + * i40e_preserve_xps_settings - preserve XPS maps before reset + * @vsi: pointer to the targeted VSI + * @qr: pointer to the structure with XPS mapping + * + * Read queues mapping from every CPU and save it as a CPU mask for every + * queue. + **/ +static void +i40e_preserve_xps_settings(struct i40e_vsi *vsi, struct i40e_qmap_rev *qr) +{ + int cpu, q_idx, cpu_idx, cpus = num_online_cpus(); + struct net_device *netdev = vsi->netdev; + struct xps_dev_maps *dev_maps; + struct xps_map *map; + u64 bitmap_arr; + + if (!netdev || vsi->type != I40E_VSI_MAIN) + return; + + if (cpus < vsi->num_queue_pairs) { + dev_warn(&vsi->back->pdev->dev, + "There are more queues than cpus. To set xps maps properly reinitialize queues.\n"); + return; + } + + rcu_read_lock(); + + if (!netdev->xps_maps[XPS_CPUS]) + goto out; + + dev_maps = rcu_dereference(netdev->xps_maps[XPS_CPUS]); + + for (cpu = 0; cpu < cpus; cpu++) { + cpu_idx = cpumask_local_spread(cpu, -1); + if (!dev_maps->attr_map[cpu_idx]) + continue; + + map = rcu_dereference(dev_maps->attr_map[cpu_idx]); + bitmap_arr = cpu_idx; + do_div(bitmap_arr, BITS_PER_LONG); + for (q_idx = 0; q_idx < map->len; q_idx++) { + qr[map->queues[q_idx]].vsi_id = vsi->id; + qr[map->queues[q_idx]].cpus.bits[bitmap_arr] |= + BIT(cpu_idx); + } + } + +out: + rcu_read_unlock(); +} + +/** + * i40e_restore_xps_settings - restore XPS maps after reset + * @vsi: pointer to the targeted VSI + * @qr: pointer to the structure with XPS mapping + * + * Set previously preserved XPS CPUs to queues mapping. + **/ +static void +i40e_restore_xps_settings(struct i40e_vsi *vsi, struct i40e_qmap_rev *qr) +{ + struct net_device *netdev = vsi->netdev; + int q_count, q; + + q_count = min_t(unsigned int, num_online_cpus(), vsi->num_queue_pairs); + + if (vsi->type != I40E_VSI_MAIN) + return; + + for (q = 0; q < q_count; q++) + if (qr[q].vsi_id == vsi->id) + netif_set_xps_queue(netdev, &qr[q].cpus, q); +} + +#endif /* CONFIG_XPS */ /** * i40e_rebuild - rebuild using a saved config * @pf: board private structure @@ -10804,6 +10881,9 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired) { const bool is_recovery_mode_reported = i40e_check_recovery_mode(pf); struct i40e_vsi *vsi = pf->vsi[pf->lan_vsi]; +#ifdef CONFIG_XPS + struct i40e_qmap_rev *qr = NULL; +#endif /* CONFIG_XPS */ struct i40e_hw *hw = &pf->hw; i40e_status ret; u32 val; @@ -10919,6 +10999,22 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired) } #endif /* CONFIG_I40E_DCB */ +#ifdef CONFIG_XPS + if (!reinit) { + int cpus = num_possible_cpus(); + + qr = kcalloc(cpus, sizeof(struct i40e_qmap_rev), GFP_KERNEL); + if (!qr) { + ret = -ENOMEM; + goto end_unlock; + } + + for (v = 0; v < pf->num_alloc_vsi; v++) + if (pf->vsi[v]) + i40e_preserve_xps_settings(pf->vsi[v], qr); + } + +#endif /* CONFIG_XPS */ if (!lock_acquired) rtnl_lock(); ret = i40e_setup_pf_switch(pf, reinit, true); @@ -11073,6 +11169,16 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired) i40e_reset_all_vfs(pf, true); +#ifdef CONFIG_XPS + if (!reinit) { + for (v = 0; v < pf->num_alloc_vsi; v++) + if (pf->vsi[v]) + i40e_restore_xps_settings(pf->vsi[v], qr); + } else { + dev_info(&pf->pdev->dev, "XPS maps were reset to default after queue re-initialization"); + } + +#endif /* CONFIG_XPS */ /* tell the firmware that we're starting */ i40e_send_version(pf); @@ -11082,6 +11188,9 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired) end_unlock: if (!lock_acquired) rtnl_unlock(); +#ifdef CONFIG_XPS + kfree(qr); +#endif /* CONFIG_XPS */ end_core_reset: clear_bit(__I40E_RESET_FAILED, pf->state); clear_recovery: