Skip to contentSkip to conversation

Patch "cpumask_set_cpu_local_first => cpumask_local_spread, lament" has been added to the 3.18-stable tree

PATCH [email protected] 1 participant 1 message May 5, 2017

Subsystems (6)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

cpumask_set_cpu_local_first => cpumask_local_spread, lament

to the 3.18-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
cpumask_set_cpu_local_first-cpumask_local_spread-lament.patch
and it can be found in the queue-3.18 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From f36963c9d3f6f415732710da3acdd8608a9fa0e5 Mon Sep 17 00:00:00 2001

From: Rusty Russell <[email protected]>
Date: Sat, 9 May 2015 03:14:13 +0930
Subject: cpumask_set_cpu_local_first => cpumask_local_spread, lament

From: Rusty Russell <[email protected]>

commit f36963c9d3f6f415732710da3acdd8608a9fa0e5 upstream.

da91309e0a7e (cpumask: Utility function to set n'th cpu...) created a genuinely weird function. I never saw it before, it went through DaveM. (He only does this to make us other maintainers feel better about our own mistakes.)

cpumask_set_cpu_local_first's purpose is say "I need to spread things across N online cpus, choose the ones on this numa node first"; you call it in a loop.

It can fail. One of the two callers ignores this, the other aborts and fails the device open.

It can fail in two ways: allocating the off-stack cpumask, or through a convoluted codepath which AFAICT can only occur if cpu_online_mask changes. Which shouldn't happen, because if cpu_online_mask can change while you call this, it could return a now-offline cpu anyway.

It contains a nonsensical test "!cpumask_of_node(numa_node)". This was drawn to my attention by Geert, who said this causes a warning on Sparc. It sets a single bit in a cpumask instead of returning a cpu number, because that's what the callers want.

It could be made more efficient by passing the previous cpu rather than an index, but that would be more invasive to the callers.

[backporting for 3.18: only two callers exist, otherwise no change.
The same warning shows up for "!cpumask_of_node()", and I thought about just addressing the warning, but using the whole fix seemed better in the end as one of the two callers also lacks the error handling]

---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   10 +--
 drivers/net/ethernet/mellanox/mlx4/en_tx.c     |    6 +-
 include/linux/cpumask.h                        |    6 --
 lib/cpumask.c                                  |   72 ++++++++-----------------
 4 files changed, 33 insertions(+), 61 deletions(-)
drivers/net/ethernet/mellanox/mlx4/en_netdev.c +3 -7
@@ -1500,17 +1500,13 @@ static int mlx4_en_init_affinity_hint(st
 {
 	struct mlx4_en_rx_ring *ring = priv->rx_ring[ring_idx];
 	int numa_node = priv->mdev->dev->numa_node;
-	int ret = 0;
 
 	if (!zalloc_cpumask_var(&ring->affinity_mask, GFP_KERNEL))
 		return -ENOMEM;
 
-	ret = cpumask_set_cpu_local_first(ring_idx, numa_node,
-					  ring->affinity_mask);
-	if (ret)
-		free_cpumask_var(ring->affinity_mask);
-
-	return ret;
+	cpumask_set_cpu(cpumask_local_spread(ring_idx, numa_node),
+			ring->affinity_mask);
+	return 0;
 }
 
 static void mlx4_en_free_affinity_hint(struct mlx4_en_priv *priv, int ring_idx)
drivers/net/ethernet/mellanox/mlx4/en_tx.c +3 -3
@@ -139,9 +139,9 @@ int mlx4_en_create_tx_ring(struct mlx4_e
 	ring->queue_index = queue_index;
 
 	if (queue_index < priv->num_tx_rings_p_up)
-		cpumask_set_cpu_local_first(queue_index,
-					    priv->mdev->dev->numa_node,
-					    &ring->affinity_mask);
+		cpumask_set_cpu(cpumask_local_spread(queue_index,
+						     priv->mdev->dev->numa_node),
+				&ring->affinity_mask);
 
 	*pring = ring;
 	return 0;
include/linux/cpumask.h +2 -4
@@ -142,10 +142,8 @@ static inline unsigned int cpumask_any_b
 	return 1;
 }
 
-static inline int cpumask_set_cpu_local_first(int i, int numa_node, cpumask_t *dstp)
+static inline unsigned int cpumask_local_spread(unsigned int i, int node)
 {
-	set_bit(0, cpumask_bits(dstp));
-
 	return 0;
 }
 
@@ -199,7 +197,7 @@ static inline unsigned int cpumask_next_
 
 int cpumask_next_and(int n, const struct cpumask *, const struct cpumask *);
 int cpumask_any_but(const struct cpumask *mask, unsigned int cpu);
-int cpumask_set_cpu_local_first(int i, int numa_node, cpumask_t *dstp);
+unsigned int cpumask_local_spread(unsigned int i, int node);
 
 /**
  * for_each_cpu - iterate over every cpu in a mask
lib/cpumask.c +25 -47

89 lines not on this page · raw message

Patches currently in stable-queue which might be from [email protected] are

queue-3.18/modpost-don-t-emit-section-mismatch-warnings-for-compiler-optimizations.patch queue-3.18/cpumask_set_cpu_local_first-cpumask_local_spread-lament.patch queue-3.18/modpost-expand-pattern-matching-to-support-substring-matches.patch