Re: [PATCH] chainlint.pl: Extend regexp pattern for /proc/cpuinfo on Linux SPARC
Hi Junio,
On Mon, 2024-05-20 at 09:16 -0700, Junio C Hamano wrote:
Show 26 quoted lines
> John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:
>
> > On SPARC systems running Linux, individual processors are denoted with
> > "CPUnn:" in /proc/cpuinfo instead of the usual "processor NN:" so that
> > the current regexp in ncores() returns 0. Extend the regexp to match
> > lines with "CPUnn:" as well to properly detect the number of available
> > cores on these systems.
> >
> > Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> > ---
> > t/chainlint.pl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/chainlint.pl b/t/chainlint.pl
> > index 556ee91a15..63cac942ac 100755
> > --- a/t/chainlint.pl
> > +++ b/t/chainlint.pl
> > @@ -718,7 +718,7 @@ sub ncores {
> > # Windows
> > return $ENV{NUMBER_OF_PROCESSORS} if exists($ENV{NUMBER_OF_PROCESSORS});
> > # Linux / MSYS2 / Cygwin / WSL
> > - do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:/, <>)); } if -r '/proc/cpuinfo';
> > + do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:||^CPU[\d]*:/, <>)); } if -r '/proc/cpuinfo';
>
> Is the doubled || intended? Doesn't it introduce an empty pattern
> that slurps every single line of /proc/cpuinfo?I'm not a Perl expert by any means, so I wasn't sure what the correct logical OR operator would be. If it turns out to be wrong, let's fix that.
Show 26 quoted lines
> I was wondering if we want to first add the "reasonable fallback"
> Eric mentioned ealier, and then build on top, whose result may look
> like the attached. You can enable the STDERR thing with your double
> "||" added back and see what "cd t && perl chainlint.pl" produces.
>
> Thanks.
>
> diff --git i/t/chainlint.pl w/t/chainlint.pl
> index 556ee91a15..775f06281b 100755
> --- i/t/chainlint.pl
> +++ w/t/chainlint.pl
> @@ -718,7 +718,13 @@ sub ncores {
> # Windows
> return $ENV{NUMBER_OF_PROCESSORS} if exists($ENV{NUMBER_OF_PROCESSORS});
> # Linux / MSYS2 / Cygwin / WSL
> - do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:/, <>)); } if -r '/proc/cpuinfo';
> + do {
> + local @ARGV='/proc/cpuinfo';
> + my @num = grep(/^processor[\s\d]*:|^CPU[\d]*:/, <>);
> +# print STDERR "FOUND <@num>\n";
> + return 1 if (!@num);
> + return scalar(@num);
> + } if -r '/proc/cpuinfo';
> # macOS & BSD
> return qx/sysctl -n hw.ncpu/ if $^O =~ /(?:^darwin$|bsd)/;
> return 1;This seems to work fine for me as well. If you post it as a patch, I'm more than happy to give it a Tested-By.
Btw, it would be great if this could be extended to support the output for the Alpha architecture as well since the testsuite fails the same way [1]. The output for /proc/cpuinfo looks like this [2]:
(alpha-chroot)root@p100:/# cat /proc/cpuinfo cpu : Alpha cpu model : ev67 cpu variation : 0 cpu revision : 0 cpu serial number : JA00000000 system type : QEMU system variation : QEMU_v8.0.92 system revision : 0 system serial number : AY00000000 cycle frequency [Hz] : 250000000 timer frequency [Hz] : 250.00 page size [bytes] : 8192 phys. address bits : 44 max. addr. space # : 255 BogoMIPS : 2500.00 platform string : AlphaServer QEMU user-mode VM cpus detected : 8 cpus active : 4 cpu active mask : 0000000000000095 L1 Icache : n/a L1 Dcache : n/a L2 cache : n/a L3 cache : n/a
Thanks so much for helping with the fix!
Adrian
> [1] https://buildd.debian.org/status/fetch.php?pkg=git&arch=alpha&ver=1%3A2.45.1-1&stamp=1716194983&raw=0
> [2] https://lore.kernel.org/all/20230901204251.137307-4-richard.henderson@linaro.org/
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
John Paul Adrian Glaubitz
