All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next V2 0/6] net/mlx5: hw counters refactor
@ 2024-10-01 10:37 Tariq Toukan
  2024-10-01 10:37 ` [PATCH net-next V2 1/6] net/mlx5: hw counters: Make fc_stats & fc_pool private Tariq Toukan
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Tariq Toukan @ 2024-10-01 10:37 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: netdev, Saeed Mahameed, Gal Pressman, Leon Romanovsky,
	Simon Horman, Tariq Toukan

This is a patchset re-post, see:
https://lore.kernel.org/netdev/20240815054656.2210494-7-tariqt@nvidia.com/T/

In this patchset, Cosmin refactors hw counters and solves perf scaling
issue. 

Series generated against:
commit c824deb1a897 ("cxgb4: clip_tbl: Fix spelling mistake "wont" -> "won't"")

HW counters are central to mlx5 driver operations. They are hardware
objects created and used alongside most steering operations, and queried
from a variety of places. Most counters are queried in bulk from a
periodic task in fs_counters.c.

Counter performance is important and as such, a variety of improvements
have been done over the years. Currently, counters are allocated from
pools, which are bulk allocated to amortize the cost of firmware
commands. Counters are managed through an IDR, a doubly linked list and
two atomic single linked lists. Adding/removing counters is a complex
dance between user contexts requesting it and the mlx5_fc_stats_work
task which does most of the work.

Under high load (e.g. from connection tracking flow insertion/deletion),
the counter code becomes a bottleneck, as seen on flame graphs. Whenever
a counter is deleted, it gets added to a list and the wq task is
scheduled to run immediately to actually delete it. This is done via
mod_delayed_work which uses an internal spinlock. In some tests, waiting
for this spinlock took up to 66% of all samples.

This series refactors the counter code to use a more straight-forward
approach, avoiding the mod_delayed_work problem and making the code
easier to understand. For that:

- patch #1 moves counters data structs to a more appropriate place.
- patch #2 simplifies the bulk query allocation scheme by using vmalloc.
- patch #3 replaces the IDR+3 lists with an xarray. This is the main
  patch of the series, solving the spinlock congestion issue.
- patch #4 removes an unnecessary cacheline alignment causing a lot of
  memory to be wasted.
- patches #5 and #6 are small cleanups enabled by the refactoring.

Regards,
Tariq

V2:
- no changes, re-posting.

Cosmin Ratiu (6):
  net/mlx5: hw counters: Make fc_stats & fc_pool private
  net/mlx5: hw counters: Use kvmalloc for bulk query buffer
  net/mlx5: hw counters: Replace IDR+lists with xarray
  net/mlx5: hw counters: Drop unneeded cacheline alignment
  net/mlx5: hw counters: Don't maintain a counter count
  net/mlx5: hw counters: Remove mlx5_fc_create_ex

 .../ethernet/mellanox/mlx5/core/en/tc_ct.c    |   2 +-
 .../ethernet/mellanox/mlx5/core/fs_counters.c | 387 +++++++-----------
 include/linux/mlx5/driver.h                   |  33 +-
 include/linux/mlx5/fs.h                       |   3 -
 4 files changed, 147 insertions(+), 278 deletions(-)

-- 
2.44.0


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH net-next V2 3/6] net/mlx5: hw counters: Replace IDR+lists with xarray
@ 2024-10-05  8:17 kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2024-10-05  8:17 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20241001103709.58127-4-tariqt@nvidia.com>
References: <20241001103709.58127-4-tariqt@nvidia.com>
TO: Tariq Toukan <tariqt@nvidia.com>

Hi Tariq,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Tariq-Toukan/net-mlx5-hw-counters-Make-fc_stats-fc_pool-private/20241001-184125
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241001103709.58127-4-tariqt%40nvidia.com
patch subject: [PATCH net-next V2 3/6] net/mlx5: hw counters: Replace IDR+lists with xarray
:::::: branch date: 4 days ago
:::::: commit date: 4 days ago
config: loongarch-randconfig-r071-20241003 (https://download.01.org/0day-ci/archive/20241005/202410051611.V2fRGx99-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202410051611.V2fRGx99-lkp@intel.com/

smatch warnings:
drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c:173 mlx5_fc_stats_query_all_counters() error: uninitialized symbol 'bulk_query_time'.
drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c:174 mlx5_fc_stats_query_all_counters() error: uninitialized symbol 'bulk_base_id'.

vim +/bulk_query_time +173 drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c

6f06e04b67baa1 Gavi Teitz   2019-07-29  119  
a76cdab93204e4 Cosmin Ratiu 2024-10-01  120  /* Synchronization notes
a76cdab93204e4 Cosmin Ratiu 2024-10-01  121   *
a76cdab93204e4 Cosmin Ratiu 2024-10-01  122   * Access to counter array:
a76cdab93204e4 Cosmin Ratiu 2024-10-01  123   * - create - mlx5_fc_create() (user context)
a76cdab93204e4 Cosmin Ratiu 2024-10-01  124   *   - inserts the counter into the xarray.
a76cdab93204e4 Cosmin Ratiu 2024-10-01  125   *
a76cdab93204e4 Cosmin Ratiu 2024-10-01  126   * - destroy - mlx5_fc_destroy() (user context)
a76cdab93204e4 Cosmin Ratiu 2024-10-01  127   *   - erases the counter from the xarray and releases it.
a76cdab93204e4 Cosmin Ratiu 2024-10-01  128   *
a76cdab93204e4 Cosmin Ratiu 2024-10-01  129   * - query mlx5_fc_query(), mlx5_fc_query_cached{,_raw}() (user context)
a76cdab93204e4 Cosmin Ratiu 2024-10-01  130   *   - user should not access a counter after destroy.
a76cdab93204e4 Cosmin Ratiu 2024-10-01  131   *
a76cdab93204e4 Cosmin Ratiu 2024-10-01  132   * - bulk query (single thread workqueue context)
a76cdab93204e4 Cosmin Ratiu 2024-10-01  133   *   - create: query relies on 'lastuse' to avoid updating counters added
a76cdab93204e4 Cosmin Ratiu 2024-10-01  134   *             around the same time as the current bulk cmd.
a76cdab93204e4 Cosmin Ratiu 2024-10-01  135   *   - destroy: destroyed counters will not be accessed, even if they are
a76cdab93204e4 Cosmin Ratiu 2024-10-01  136   *              destroyed during a bulk query command.
a76cdab93204e4 Cosmin Ratiu 2024-10-01  137   */
a76cdab93204e4 Cosmin Ratiu 2024-10-01  138  static void mlx5_fc_stats_query_all_counters(struct mlx5_core_dev *dev)
a351a1b03bf169 Amir Vadai   2016-07-14  139  {
80cdc8314c3af3 Cosmin Ratiu 2024-10-01  140  	struct mlx5_fc_stats *fc_stats = dev->priv.fc_stats;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  141  	u32 bulk_len = fc_stats->bulk_query_len;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  142  	XA_STATE(xas, &fc_stats->counters, 0);
6f06e04b67baa1 Gavi Teitz   2019-07-29  143  	u32 *data = fc_stats->bulk_query_out;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  144  	struct mlx5_fc *counter;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  145  	u32 last_bulk_id = 0;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  146  	u64 bulk_query_time;
6f06e04b67baa1 Gavi Teitz   2019-07-29  147  	u32 bulk_base_id;
a351a1b03bf169 Amir Vadai   2016-07-14  148  	int err;
a8ffcc741acb3c Rabie Loulou 2017-07-09  149  
a76cdab93204e4 Cosmin Ratiu 2024-10-01  150  	xas_lock(&xas);
a76cdab93204e4 Cosmin Ratiu 2024-10-01  151  	xas_for_each(&xas, counter, U32_MAX) {
a76cdab93204e4 Cosmin Ratiu 2024-10-01  152  		if (xas_retry(&xas, counter))
a76cdab93204e4 Cosmin Ratiu 2024-10-01  153  			continue;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  154  		if (unlikely(counter->id >= last_bulk_id)) {
a76cdab93204e4 Cosmin Ratiu 2024-10-01  155  			/* Start new bulk query. */
a76cdab93204e4 Cosmin Ratiu 2024-10-01  156  			/* First id must be aligned to 4 when using bulk query. */
6f06e04b67baa1 Gavi Teitz   2019-07-29  157  			bulk_base_id = counter->id & ~0x3;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  158  			last_bulk_id = bulk_base_id + bulk_len;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  159  			/* The lock is released while querying the hw and reacquired after. */
a76cdab93204e4 Cosmin Ratiu 2024-10-01  160  			xas_unlock(&xas);
a76cdab93204e4 Cosmin Ratiu 2024-10-01  161  			/* The same id needs to be processed again in the next loop iteration. */
a76cdab93204e4 Cosmin Ratiu 2024-10-01  162  			xas_reset(&xas);
a76cdab93204e4 Cosmin Ratiu 2024-10-01  163  			bulk_query_time = jiffies;
a76cdab93204e4 Cosmin Ratiu 2024-10-01  164  			err = mlx5_cmd_fc_bulk_query(dev, bulk_base_id, bulk_len, data);
a351a1b03bf169 Amir Vadai   2016-07-14  165  			if (err) {
a351a1b03bf169 Amir Vadai   2016-07-14  166  				mlx5_core_err(dev, "Error doing bulk query: %d\n", err);
6f06e04b67baa1 Gavi Teitz   2019-07-29  167  				return;
a351a1b03bf169 Amir Vadai   2016-07-14  168  			}
a76cdab93204e4 Cosmin Ratiu 2024-10-01  169  			xas_lock(&xas);
a76cdab93204e4 Cosmin Ratiu 2024-10-01  170  			continue;
6f06e04b67baa1 Gavi Teitz   2019-07-29  171  		}
a76cdab93204e4 Cosmin Ratiu 2024-10-01  172  		/* Do not update counters added after bulk query was started. */
a76cdab93204e4 Cosmin Ratiu 2024-10-01 @173  		if (time_after64(bulk_query_time, counter->cache.lastuse))
a76cdab93204e4 Cosmin Ratiu 2024-10-01 @174  			update_counter_cache(counter->id - bulk_base_id, data,
a76cdab93204e4 Cosmin Ratiu 2024-10-01  175  					     &counter->cache);
a351a1b03bf169 Amir Vadai   2016-07-14  176  	}
a76cdab93204e4 Cosmin Ratiu 2024-10-01  177  	xas_unlock(&xas);
a351a1b03bf169 Amir Vadai   2016-07-14  178  }
a351a1b03bf169 Amir Vadai   2016-07-14  179  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-10-05  8:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-01 10:37 [PATCH net-next V2 0/6] net/mlx5: hw counters refactor Tariq Toukan
2024-10-01 10:37 ` [PATCH net-next V2 1/6] net/mlx5: hw counters: Make fc_stats & fc_pool private Tariq Toukan
2024-10-01 10:37 ` [PATCH net-next V2 2/6] net/mlx5: hw counters: Use kvmalloc for bulk query buffer Tariq Toukan
2024-10-01 10:37 ` [PATCH net-next V2 3/6] net/mlx5: hw counters: Replace IDR+lists with xarray Tariq Toukan
2024-10-04  8:58   ` Simon Horman
2024-10-04  9:32     ` Cosmin Ratiu
2024-10-04 10:34       ` Simon Horman
2024-10-01 10:37 ` [PATCH net-next V2 4/6] net/mlx5: hw counters: Drop unneeded cacheline alignment Tariq Toukan
2024-10-01 10:37 ` [PATCH net-next V2 5/6] net/mlx5: hw counters: Don't maintain a counter count Tariq Toukan
2024-10-01 10:37 ` [PATCH net-next V2 6/6] net/mlx5: hw counters: Remove mlx5_fc_create_ex Tariq Toukan
2024-10-04 18:50 ` [PATCH net-next V2 0/6] net/mlx5: hw counters refactor patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2024-10-05  8:17 [PATCH net-next V2 3/6] net/mlx5: hw counters: Replace IDR+lists with xarray kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.