Re: [PATCH v3 00/17] Catch syscall group

[Pedro Alves <palves at redhat dot com>]

On 04/28/2015 09:28 PM, Sergio Durigan Junior wrote:
> On Tuesday, April 28 2015, Pedro Alves wrote:
> 
>> I was wondering if we couldn't share most of the grouping
>> per-architecture, e.g., by having each arch syscall file xi:include a
>> base Linux default groups file, that listed the grouping without
>> the syscall number.  E.g., create a linux-defaults.xml like:
>>
> Thanks for the review, Pedro.  I think this is a nice idea, but I would
> like to propose that we accept the patches as-is, without this
> improvement, and then work on it later.  First, it's been a long time
> since we're discussing this feature, and I don't want Krisman to not
> feel encouraged to continue contributing :-).  

I think there's a difference from the feature itself, which was
discussed and I'm generally fine with, and the whole set of patches that are
new in this new revision of the series.  Those are revealing a problem
that I think should be addressed.

> Also, I think the syscall
> XML generation really needs a revamp, independently of how/if we use
> groups or not.  There should be possible, for example, to easily update
> the XML's with the latest Linux kernel source.  This task is on my
> plate, though it's a low priority.  So, for now, I think Krisman's work
> is good enough.

Updating the XML's from the Linux source is a different thing.  That
would only regenerate the set of syscalls and its numbers.  The grouping
is not something that can be extracted out of the kernel.

Note Gabriel is already using scripts to generate the xmls (hence
my questions in the previous email):

> The previous version only implemented syscalls for amd64.  I used a
> script to generate the xmls, and based the group field information on
> strace, so please share your thoughts if you disagree with any group.

... and this shows the issue that I'm seeing: we have seemingly the
same grouping done _14_ times.  That's not a sign of something that
is maintainable.

Maybe the linux-defaults.xml idea was overcomplicated.  The main
take away is that the syscall grouping should be the same
everywhere, which suggests that the xml files in their current
xml scheme should be generated.  The existing xml files map the syscall
names to syscall numbers, and that necessarily need to be per-architecture.
I'm super fine with adding the "groups" attribute to each
arch file.  But we can centralize that.  It does not have to be a
complicated thing at all.  Something like:

 - rename the existing files:

     $arch-linux.xml -> $arch-xml.in

 - add a simple linux-defaults file, containing a simple table, like:

~~~
# First column is syscall name, second column is syscall groups.
...
signalfd4 descriptor,signal
vfork process
...
~~~

And then add a build step that with a simple perl/python/awk/sed/whatever
script goes over each line of that table and seds each $arch-linux-xml.in
the file transforming:

  name="$first_column"

into

  name="$first_column" groups="$second_column"

etc.

E.g., from:
  <syscall name="signalfd4" number="289"/>

to:
  <syscall name="signalfd4" groups="descriptor,signal" number="289"/>

That is, a simple text substitution.

This makes is much easier to

 - review
 - maintain
 - extend for future syscalls / groups
 - extend for future architectures

than the same info in 14 (and counting) different files, probably
all in agreement with each other, but pretty much unverifiable.

> In sum: I propose we go ahead now ("don't let the perfect be the enemy
> of the good"), and concentrate on the XML problem later.

I'd like to hear what Gabriel thinks (both of my questions in the
previous email, and this one).

If there are grouping differences between the architectures (other
than which syscalls are wired/supported), Gabriel will have noticed
them, but that knowledge is lost (not encoded anywhere) in the
current form.

As a quick experiment, I did (which Gabriel's grouping patches applied):

 $ cat amd64-linux.xml | sed 's/ number=\"[0-9]*\"//g' | sort > amd64-linux-no-number.xml
 $ cat ppc-linux.xml | sed 's/ number=\"[0-9]*\"//g' | sort > ppc-linux-no-number.xml
 $ diff -up amd64-linux-no-number.xml ppc-linux-no-number.xml

and saw no difference in the grouping, but I did not do that
for all archs.

Thanks,
Pedro Alves