Re: [PATCH 0/3] get_abi: improve message output and fix a regression

Em Tue, 28 Sep 2021 13:04:22 +0200
Greg Kroah-Hartman [off-list ref] escreveu:

On Tue, Sep 28, 2021 at 12:14:01PM +0200, Mauro Carvalho Chehab wrote:

quoted

Hi Greg,

As promised on

	https://lore.kernel.org/lkml/20210928120304.62319fba@coco.lan/T/#u (local)

I'm adding progress info when  get_abi.pl is checking for undefined ABI symbols
on patches 1 and 2.

That will help not only to identify what is causing delays on the script, but also
to notify the user that processing it could take some time on some systems.

If you run it on your big server with:

  scripts/get_abi.pl undefined 2>logs

The "logs" file will contain timestamps relative to the time the script started to
do the regex matches for sysfs files. It should be printing one line every
time the progress completes 1% or one second after the last progress output.  

Adding more debugging and tweaking the script a bit to show the file it
is about to check, not the one it finished checking,

Feel free to modify the script and add such debug/tweaks if you find
it useful. 

I got the following
debug output that seems to pinpoint the problem file.

The sysfs file that is causing problems is:
	/sys/devices/pci0000:40/0000:40:00.2/iommu/ivhd1/amd-iommu/cap

and here's some debugging output for the regex it needs to search for
this:

/sys/devices/pci0000:40/0000:40:00.2/iommu/ivhd1/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/amd\-iommu/cap$)$/
/sys/devices/pci0000:40/0000:40:00.2/iommu/ivhd1/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/intel\-iommu/cap$)$/
/sys/devices/pci0000:40/0000:40:00.2/iommu/ivhd1/amd-iommu/cap =~ /^(?^:^/sys/devices/pci.*.*.*.*\:.*.*/0000\:.*.*\:.*.*..*/dma/dma.*chan.*/quickdata/cap$)$/
/sys/devices/pci0000:40/0000:40:07.0/iommu/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/amd\-iommu/cap$)$/
/sys/devices/pci0000:40/0000:40:07.0/iommu/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/intel\-iommu/cap$)$/
/sys/devices/pci0000:40/0000:40:07.0/iommu/amd-iommu/cap =~ /^(?^:^/sys/devices/pci.*.*.*.*\:.*.*/0000\:.*.*\:.*.*..*/dma/dma.*chan.*/quickdata/cap$)$/
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:02/device:7a/physical_node/iommu/ivhd1/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/amd\-iommu/cap$)$/
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:02/device:7a/physical_node/iommu/ivhd1/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/intel\-iommu/cap$)$/
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:02/device:7a/physical_node/iommu/ivhd1/amd-iommu/cap =~ /^(?^:^/sys/devices/pci.*.*.*.*\:.*.*/0000\:.*.*\:.*.*..*/dma/dma.*chan.*/quickdata/cap$)$/
/sys/devices/pci0000:40/0000:40:01.3/0000:4a:00.0/0000:4b:0a.0/0000:50:00.0/iommu/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/amd\-iommu/cap$)$/
/sys/devices/pci0000:40/0000:40:01.3/0000:4a:00.0/0000:4b:0a.0/0000:50:00.0/iommu/amd-iommu/cap =~ /^(?^:^/sys/class/iommu/.*/intel\-iommu/cap$)$/
/sys/devices/pci0000:40/0000:40:01.3/0000:4a:00.0/0000:4b:0a.0/0000:50:00.0/iommu/amd-iommu/cap =~ /^(?^:^/sys/devices/pci.*.*.*.*\:.*.*/0000\:.*.*\:.*.*..*/dma/dma.*chan.*/quickdata/cap$)$/

Hmm... interesting. Perhaps the problem is on regexes like this:

	/^(?^:^/sys/devices/pci.*.*.*.*\:.*.*/0000\:.*.*\:.*.*..*/dma/dma.*chan.*/quickdata/cap$)$/

Which actually represents this What:
	/sys/devices/pciXXXX:XX/0000:XX:XX.X/dma/dma<n>chan<n>/quickdata/cap

The script could have done a better job escaping "." character on
it (but that is not too trivial) and grouping altogether ".*" 
repetitions, although, in this specific case, probably the best
regex would be, instead:

	/sys/devices/pci[\da-f:\.]+/dma/dma\d+chan\d+/quickdata/cap

One possible long-term solution would be to directly use regexes 
directly on "What" fields inside Documentation/ABI, but on some parts
this would require some changes, like, for instance:

	/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/kone/roccatkone<minor>/weight

Ok, we could likely use capture groups like:

	(?<NAME>pattern)

but IMO that would make it a lot harder to be understood by humans.

And sometimes this thing finishes in 20 seconds, and others, many many
minutes.  It's not deterministic at all, which is odd.  Is the sysfs
tree being sorted so that this should always have the same search order?

No, because it uses a lot of hashes in order to speed it up. Yet,
it shouldn't be hard - nor it would significantly affect the processing
time - to make it more deterministic. See the enclosed path.

Anyway, I've applied this series as well, this helps in finding the
problems :)

Thanks!

Note, I can provide an off-list tarball of /sys/ if that would help in
debugging anything on your end.

Yeah, that can help. Feel free to send it to me.

Btw, I just got an arm64 server with 128 CPUs for testing. I'm trying
to allocate also a big x86 server here, but I'm not sure if it is AMD or
Intel.

Thanks,
Mauro

[PATCH] scripts: get_abi.pl: make undefined search more deterministic

Sort keys on hashes during undefined search, in order to
make the script more deterministic.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/scripts/get_abi.pl b/scripts/get_abi.pl
index 841d889747c0..d32dcd7cca5d 100755
--- a/scripts/get_abi.pl
+++ b/scripts/get_abi.pl

@@ -775,6 +775,8 @@ sub check_undefined_symbols {
 	my $next_i = 0;
 	my $start_time = times;
 
+	@files = sort @files;
+
 	my $last_time = $start_time;
 
 	# When either debug or hint is enabled, there's no sense showing

@@ -909,16 +911,16 @@ sub undefined_symbols {
 		}
 	}
 	# Compile regexes
-	foreach my $l (keys %leaf) {
+	foreach my $l (sort keys %leaf) {
 		my @expr;
-		foreach my $w(split /\xac/, $leaf{$l}->{what}) {
+		foreach my $w(sort split /\xac/, $leaf{$l}->{what}) {
 			push @expr, qr /^$w$/;
 		}
 		$leaf{$l}->{expr} = \@expr;
 	}
 
 	# Take links into account
-	foreach my $link (keys %aliases) {
+	foreach my $link (sort keys %aliases) {
 		my $abs_file = $aliases{$link};
 		graph_add_link($abs_file, $link);
 	}