On Fri, Jun 18, 2021 at 01:28:36PM +0200, Mauro Carvalho Chehab wrote:

Hi Greg,

I was talking today with Jonathan Cameron today about how to ensure that
the ABI is not missing something.

While it would be doable to validate the ABI by searching __ATTR and similar
macros around the driver, this would probably be very complex and would
take a while to parse.

Yet, there's one way that should be quick and easier to implement:

Read the symbols from the current system in runtime, and check if
everything is declared under Documentation/ABI.

Nice!

As you know, scripts/get_abi.pl has already a search command, that would
allow seeking for a symbol inside the ABI. Using a logic similar to that,
but checking for all symbols under /sys is not hard to implemenent.
That's what patch 1 does.

Right now, the results aren't exaustive (I opted this way for the RFC
version, as otherwise there will be too many symbols that won't match
the regexes generated from the What:  fields).

It basically reports results where the sysfs nodename matches one or
more What, but doesn't match the regex.

This implementation runs very quick on my desktop: it takes less than
2 seconds to run. So, it sounds a good start to help identifying what's
missing.

One of the problems with the ABI definitions is how to define wildcards
there. Different ABI declarations use different notations. For this first
RFC version, it all the above as wildcards[1]:

	<foo>
	{foo}
	[foo]
	/.../
	*

and convert them into:

	.*

[1] perhaps the better would be to just use regex on What:, as this would
    avoid extra heuristics at get_abi.pl, but this is somewhat OOT from
    this patch.

One of the first results is that some /sys symbols that are present
on *lots* of sysfs nodes, but they aren't properly defined at ABI:

	 /sys/.*/(initstate|bind|unbind)

(there are definitions, but those aren't covering all occurrences)

We should fix that up.

Another problem it caught is that slab definitions are like:
	 /sys/kernel/slab/cache/alloc_calls

Instead of using a wildcard, like:
	/sys/kernel/slab/*/alloc_calls
or:
	/sys/kernel/slab/<cache>/alloc_calls

So, they don't  match the actual symbols found at the system.

Then we should also fix those up.

What do you think?

I like this, thanks for doing this.  We should fix up the text files to
match what we have in a format that we can actually test for things.
That will be very helpful to run on some devices so that I can go yell
at driver developers :)

thanks,

greg k-h