Messages in this thread Patch in this message | | | Date | Fri, 17 Dec 2021 14:57:12 +0800 | From | Ming Lei <> | Subject | Re: [PATCH] irq: consider cpus on nodes are unbalanced |
| |
On Fri, Dec 17, 2021 at 11:48:05AM +0900, Rei Yamamoto wrote: > On Wed, Dec 15, 2021 at 12:33, Ming Lei wrote: > >> >> If cpus on a node are offline at boot time, there are > >> >> difference in the number of nodes between when building affinity > >> >> masks for present cpus and when building affinity masks for possible > >> >> cpus. > > > > There is always difference between the two number of nodes, the 1st is > > node number covering present cpus, and the 2nd one is the node number > > covering other possible cpus not spread. > > In this case, building affinity masks for possible cpus would change even > the affinity mask bits for present cpus in the "if (numvecs <= nodes)" route. > This is the second problem I mentioned. > I will explain about the actual case later. > > > > >>> This patch fixes 2 problems caused by the difference of the > > > > Is there any user visible problem? > > The panic occured in lpfc driver. > > > > >> >> number of nodes: > >> >> > >> >> - If some unused vectors remain after building masks for present cpus, > > > > We just select a new vector for starting the spread if un-allocated > > vectors remains, but the number for allocation is still numvecs. We hope both > > present cpus and non-present cpus can be balanced on each vector, so that each > > vector may get present cpu allocated. > > I understood. > I withdraw the first problem I mentioned. > > > > >> >> remained vectors are assigned for building masks for possible cpus. > >> >> Therefore "numvecs <= nodes" condition must be > >> >> "vecs_to_assign <= nodes_to_assign". Fix this problem by making this > >> >> condition appropriate. > >> >> > >> >> - The routine of "numvecs <= nodes" condition can overwrite bits of > >> >> masks for present cpus in building masks for possible cpus. Fix this > >> >> problem by making CPU bits, which is not target, not changing. > > > > 'numvecs' is always the total number of vectors for assigning CPUs, if > > the number is <= nodes, we just assign interested cpus in the whole > > node into each vector until all interested cpus are allocated out. > > > > > >> Do you have any comments? > > > > Not see issues in current way, or can you explain a bit the real > > user visible problem in details? > > I experienced a panic occurred in lpfc driver with broken affinity masks. > > The system had the following configuration: > ----- > node num: cpu num > Node #0: #0 #1 (#4 #8 #12) > Node #1: #2 #3 (#5 #9 #13) > Node #2: (#6 #10 #14) > Node #3: (#7 #11 #15) > > Number of CPUs: 16 > Present CPU: cpu0, cpu1, cpu2, cpu3 > Number of nodes covering present cpus: 2 > Number of nodes covering possible cpus: 4 > Number of vectors: 4 > ----- > > Due to the configuration above, cpumask_var_t *node_to_cpumask was as follows: > ----- > node_to_cpumask[0] = 0x1113 > node_to_cpumask[1] = 0x222c > node_to_cpumask[2] = 0x4440 > node_to_cpumask[3] = 0x8880 > ----- > > As the result of assigning vertors for present cpus, masks[].mask were as follows: > ----- > masks[vec1].mask = 0x0004 > masks[vec2].mask = 0x0008 > masks[vec3].mask = 0x0001 > masks[vec4].mask = 0x0002 > ----- > > As the result of assigning vertors for possible cpus, masks[].mask were as follows: > ----- > masks[vec1].mask = 0x1117 > masks[vec2].mask = 0x222c > masks[vec3].mask = 0x4441 > masks[vec4].mask = 0x8882 > ----- > > The problem I encountered was that multiple vectors were assigned for > a single present cpu unexpectedly. > For example, vec1 and vec3 were assigned to cpu0. > Due to this mask, the panic occured in lpfc driver.
OK, I can understand the issue now, and only the following part is enough since nmsk won't be empty:
diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c index f7ff8919dc9b..d2d01565d2ec 100644 --- a/kernel/irq/affinity.c +++ b/kernel/irq/affinity.c @@ -269,8 +269,9 @@ static int __irq_build_affinity_masks(unsigned int startvec, */ if (numvecs <= nodes) { for_each_node_mask(n, nodemsk) { + cpumask_and(nmsk, cpu_mask, node_to_cpumask[n]); cpumask_or(&masks[curvec].mask, &masks[curvec].mask, - node_to_cpumask[n]); + nmsk); if (++curvec == last_affv) curvec = firstvec; } Thanks, Ming
| |