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
