| Message ID | 20250220-mm-selftests-v1-6-9bbf57d64463@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/mm: Some cleanups from trying to run them | expand |
On 20/02/25 8:33 pm, Brendan Jackman wrote: > This calculation divides a fixed parameter by an environment-dependent > parameter i.e. the number of CPUs. > > The simple way to avoid machine-specific failures here is to just put a > cap on the max value of the latter. I haven't read the test, but if nr_cpus is being computed, then this value must be important to the test somehow? Would it potentially be wrong to let the test run for nr_cpus != actual number of cpus? Also, if the patch is correct then will it be better to also print a diagnostic telling the user that the number of cpus is going to be capped for the test to run? > > Suggested-by: Mateusz Guzik <mjguzik@gmail.com> > Signed-off-by: Brendan Jackman <jackmanb@google.com> > --- > tools/testing/selftests/mm/uffd-stress.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c > index 1facfb79e09aa4113e344d7d90dec06a37264058..f306accbef255c79bc3eeba8b9e42161a88fc10e 100644 > --- a/tools/testing/selftests/mm/uffd-stress.c > +++ b/tools/testing/selftests/mm/uffd-stress.c > @@ -453,6 +453,10 @@ int main(int argc, char **argv) > } > > nr_cpus = sysconf(_SC_NPROCESSORS_ONLN); > + if (nr_cpus > 32) { > + /* Don't let calculation below go to zero. */ > + nr_cpus = 32; > + } > > nr_pages_per_cpu = bytes / page_size / nr_cpus; > if (!nr_pages_per_cpu) { >
On Thu, 20 Feb 2025 at 16:48, Dev Jain <dev.jain@arm.com> wrote: > > > > On 20/02/25 8:33 pm, Brendan Jackman wrote: > > This calculation divides a fixed parameter by an environment-dependent > > parameter i.e. the number of CPUs. > > > > The simple way to avoid machine-specific failures here is to just put a > > cap on the max value of the latter. > > I haven't read the test, but if nr_cpus is being computed, then this > value must be important to the test somehow? Would it potentially be > wrong to let the test run for nr_cpus != actual number of cpus? Based on my _extremely hasty_ reading, the variable is misnamed and it's actually a thread count not a CPU count. I can double check that's the case and rename it. > Also, if the patch is correct then will it be better to also print a > diagnostic telling the user that the number of cpus is going to be > capped for the test to run? Sure. The level of detail in the logging and error messages is extremely low here so I didn't feel like being too anomalous, but why not.
On Thu, 20 Feb 2025 at 16:55, Brendan Jackman <jackmanb@google.com> wrote: > > On Thu, 20 Feb 2025 at 16:48, Dev Jain <dev.jain@arm.com> wrote: > > > > > > > > On 20/02/25 8:33 pm, Brendan Jackman wrote: > > > This calculation divides a fixed parameter by an environment-dependent > > > parameter i.e. the number of CPUs. > > > > > > The simple way to avoid machine-specific failures here is to just put a > > > cap on the max value of the latter. > > > > I haven't read the test, but if nr_cpus is being computed, then this > > value must be important to the test somehow? Would it potentially be > > wrong to let the test run for nr_cpus != actual number of cpus? > > Based on my _extremely hasty_ reading, the variable is misnamed and > it's actually a thread count not a CPU count. I can double check > that's the case and rename it. Oh yeah actually, it's only misnamed because I made it misnamed. So this patch needs to rename it for sure, thanks for pointing it out. (But yeah I upgraded my extremely hasty reading to an only hasty reading and I still don't think this test cares about the actual CPU topology).
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index 1facfb79e09aa4113e344d7d90dec06a37264058..f306accbef255c79bc3eeba8b9e42161a88fc10e 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -453,6 +453,10 @@ int main(int argc, char **argv) } nr_cpus = sysconf(_SC_NPROCESSORS_ONLN); + if (nr_cpus > 32) { + /* Don't let calculation below go to zero. */ + nr_cpus = 32; + } nr_pages_per_cpu = bytes / page_size / nr_cpus; if (!nr_pages_per_cpu) {
This calculation divides a fixed parameter by an environment-dependent parameter i.e. the number of CPUs. The simple way to avoid machine-specific failures here is to just put a cap on the max value of the latter. Suggested-by: Mateusz Guzik <mjguzik@gmail.com> Signed-off-by: Brendan Jackman <jackmanb@google.com> --- tools/testing/selftests/mm/uffd-stress.c | 4 ++++ 1 file changed, 4 insertions(+)