Skip to content

Commit 8ca09d5

Browse files
committed
cpumask: fix incorrect cpumask scanning result checks
It turns out that commit 596ff4a ("cpumask: re-introduce constant-sized cpumask optimizations") exposed a number of cases of drivers not checking the result of "cpumask_next()" and friends correctly. The documented correct check for "no more cpus in the cpumask" is to check for the result being equal or larger than the number of possible CPU ids, exactly _because_ we've always done those constant-sized cpumask scans using a widened type before. So the return value of a cpumask scan should be checked with if (cpu >= nr_cpu_ids) ... because the cpumask scan did not necessarily stop exactly *at* that maximum CPU id. But a few cases ended up instead using checks like if (cpu == nr_cpumask_bits) ... which used that internal "widened" number of bits. And that used to work pretty much by accident (ok, in this case "by accident" is simply because it matched the historical internal implementation of the cpumask scanning, so it was more of a "intentionally using implementation details rather than an accident"). But the extended constant-sized optimizations then did that internal implementation differently, and now that code that did things wrong but matched the old implementation no longer worked at all. Which then causes subsequent odd problems due to using what ends up being an invalid CPU ID. Most of these cases require either unusual hardware or special uses to hit, but the random.c one triggers quite easily. All you really need is to have a sufficiently small CONFIG_NR_CPUS value for the bit scanning optimization to be triggered, but not enough CPUs to then actually fill that widened cpumask. At that point, the cpumask scanning will return the NR_CPUS constant, which is _not_ the same as nr_cpumask_bits. This just does the mindless fix with sed -i 's/== nr_cpumask_bits/>= nr_cpu_ids/' to fix the incorrect uses. The ones in the SCSI lpfc driver in particular could probably be fixed more cleanly by just removing that repeated pattern entirely, but I am not emptionally invested enough in that driver to care. Reported-and-tested-by: Guenter Roeck <[email protected]> Link: https://lore.kernel.org/lkml/[email protected]/ Reported-and-tested-by: Geert Uytterhoeven <[email protected]> Link: https://lore.kernel.org/lkml/CAMuHMdUKo_Sf7TjKzcNDa8Ve+6QrK+P8nSQrSQ=6LTRmcBKNww@mail.gmail.com/ Reported-by: Vernon Yang <[email protected]> Link: https://lore.kernel.org/lkml/[email protected]/ Cc: Yury Norov <[email protected]> Cc: Jason A. Donenfeld <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 80c16b2 commit 8ca09d5

File tree

4 files changed

+10
-10
lines changed

4 files changed

+10
-10
lines changed

arch/powerpc/xmon/xmon.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1275,7 +1275,7 @@ static int xmon_batch_next_cpu(void)
12751275
while (!cpumask_empty(&xmon_batch_cpus)) {
12761276
cpu = cpumask_next_wrap(smp_processor_id(), &xmon_batch_cpus,
12771277
xmon_batch_start_cpu, true);
1278-
if (cpu == nr_cpumask_bits)
1278+
if (cpu >= nr_cpu_ids)
12791279
break;
12801280
if (xmon_batch_start_cpu == -1)
12811281
xmon_batch_start_cpu = cpu;

drivers/char/random.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1311,7 +1311,7 @@ static void __cold try_to_generate_entropy(void)
13111311
/* Basic CPU round-robin, which avoids the current CPU. */
13121312
do {
13131313
cpu = cpumask_next(cpu, &timer_cpus);
1314-
if (cpu == nr_cpumask_bits)
1314+
if (cpu >= nr_cpu_ids)
13151315
cpu = cpumask_first(&timer_cpus);
13161316
} while (cpu == smp_processor_id() && num_cpus > 1);
13171317

drivers/net/wireguard/queueing.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ static inline int wg_cpumask_choose_online(int *stored_cpu, unsigned int id)
106106
{
107107
unsigned int cpu = *stored_cpu, cpu_index, i;
108108

109-
if (unlikely(cpu == nr_cpumask_bits ||
109+
if (unlikely(cpu >= nr_cpu_ids ||
110110
!cpumask_test_cpu(cpu, cpu_online_mask))) {
111111
cpu_index = id % cpumask_weight(cpu_online_mask);
112112
cpu = cpumask_first(cpu_online_mask);

drivers/scsi/lpfc/lpfc_init.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12563,7 +12563,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors)
1256312563
goto found_same;
1256412564
new_cpu = cpumask_next(
1256512565
new_cpu, cpu_present_mask);
12566-
if (new_cpu == nr_cpumask_bits)
12566+
if (new_cpu >= nr_cpu_ids)
1256712567
new_cpu = first_cpu;
1256812568
}
1256912569
/* At this point, we leave the CPU as unassigned */
@@ -12577,7 +12577,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors)
1257712577
* selecting the same IRQ.
1257812578
*/
1257912579
start_cpu = cpumask_next(new_cpu, cpu_present_mask);
12580-
if (start_cpu == nr_cpumask_bits)
12580+
if (start_cpu >= nr_cpu_ids)
1258112581
start_cpu = first_cpu;
1258212582

1258312583
lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
@@ -12613,7 +12613,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors)
1261312613
goto found_any;
1261412614
new_cpu = cpumask_next(
1261512615
new_cpu, cpu_present_mask);
12616-
if (new_cpu == nr_cpumask_bits)
12616+
if (new_cpu >= nr_cpu_ids)
1261712617
new_cpu = first_cpu;
1261812618
}
1261912619
/* We should never leave an entry unassigned */
@@ -12631,7 +12631,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors)
1263112631
* selecting the same IRQ.
1263212632
*/
1263312633
start_cpu = cpumask_next(new_cpu, cpu_present_mask);
12634-
if (start_cpu == nr_cpumask_bits)
12634+
if (start_cpu >= nr_cpu_ids)
1263512635
start_cpu = first_cpu;
1263612636

1263712637
lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
@@ -12704,7 +12704,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors)
1270412704
goto found_hdwq;
1270512705
}
1270612706
new_cpu = cpumask_next(new_cpu, cpu_present_mask);
12707-
if (new_cpu == nr_cpumask_bits)
12707+
if (new_cpu >= nr_cpu_ids)
1270812708
new_cpu = first_cpu;
1270912709
}
1271012710

@@ -12719,7 +12719,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors)
1271912719
goto found_hdwq;
1272012720

1272112721
new_cpu = cpumask_next(new_cpu, cpu_present_mask);
12722-
if (new_cpu == nr_cpumask_bits)
12722+
if (new_cpu >= nr_cpu_ids)
1272312723
new_cpu = first_cpu;
1272412724
}
1272512725

@@ -12730,7 +12730,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors)
1273012730
found_hdwq:
1273112731
/* We found an available entry, copy the IRQ info */
1273212732
start_cpu = cpumask_next(new_cpu, cpu_present_mask);
12733-
if (start_cpu == nr_cpumask_bits)
12733+
if (start_cpu >= nr_cpu_ids)
1273412734
start_cpu = first_cpu;
1273512735
cpup->hdwq = new_cpup->hdwq;
1273612736
logit:

0 commit comments

Comments
 (0)