[Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Andy Lutomirski]

There doesn't currently seem to be any real process for reviewing new
core APIs for sanity of design, appropriateness to solve the problem
they're targetting, or correctness of implementation.  Some examples
that come to mind recently:

 - A lot of people seem to think that O_TMPFILE is a terrible
interface, despite the fact that the functionality it provides is very
useful.  It was also rather badly broken until -rc8 or so.

 - The x86 32-bit vdso clock functions almost made it in with a
questionable symbol version.

 - 3.15 is currently slated to include an unfortunate ABI glitch in
the MIPS seccomp filter interface.  There's a patch.

 - There are some aspects of the keyring API that seem to me to be quite bad.

 - An impressive number of new APIs are missing -EINVAL returns if
reserved parameters are set.

(I'm not trying to point a finger at anyone with these examples;
they're just things that I was involved in to some extent.)

The current process is confused.   For example, I currently plan on
trying to remember to ask Linus to revert the MIPS seccomp stuff or
fix it sometime around -rc6 if the patch hasn't landed, but this
sucks.

I think that the process could be improved.  I think that there are
people who are willing to spend time to read API docs and thinking
about these kinds of issues.  (I am, and Michael Kerrisk seems to do a
fair amount of it.)

I would like to discuss what we could change to make this work better
in the future.  In an ideal world, I would like to see every core API
change come with documentation, tests (possibly simple ones), and a
post, with acks, to a list that covers just API changes.  This might
be tough, but it could add a lot of value.

I've occasionally thought that API docs should live in the kernel tree
in some reasonably well organized place so that patch sets can include
doc patches.  I realize that this is contentious.

I'm sure that other people have other suggestions here.

--Andy

P.S.  I'm not on the invitation nominee list, so if people like this
topic, I'd appreciate a nomination :)

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[josh]

On Tue, May 06, 2014 at 10:45:05AM -0700, Andy Lutomirski wrote:
> There doesn't currently seem to be any real process for reviewing new
> core APIs for sanity of design, appropriateness to solve the problem
> they're targetting, or correctness of implementation.
[...]
> I think that the process could be improved.  I think that there are
> people who are willing to spend time to read API docs and thinking
> about these kinds of issues.  (I am, and Michael Kerrisk seems to do a
> fair amount of it.)
> 
> I would like to discuss what we could change to make this work better
> in the future.  In an ideal world, I would like to see every core API
> change come with documentation, tests (possibly simple ones), and a
> post, with acks, to a list that covers just API changes.  This might
> be tough, but it could add a lot of value.
> 
> I've occasionally thought that API docs should live in the kernel tree
> in some reasonably well organized place so that patch sets can include
> doc patches.  I realize that this is contentious.
> 
> I'm sure that other people have other suggestions here.
> 
> --Andy
> 
> P.S.  I'm not on the invitation nominee list, so if people like this
> topic, I'd appreciate a nomination :)

I'm interested in this topic, and I'll second that nomination.  I'd
like to partipate in this discussion.

We need to have better processes for vetting new syscalls and ABIs far
more carefully than we currently do.  Right now, we require benchmarks
for any claimed performance increase; it's almost a given that if you
post an optimization without including benchmarks in the commit message,
it'll get rejected with a request to come back with numbers.  We need
similar standards for new syscalls or other userspace ABIs: come back
with test programs, test coverage information, etc.

- Josh Triplett

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Shuah Khan]

On Tue, May 6, 2014 at 11:58 AM,  <josh@joshtriplett.org> wrote:
>
> I'm interested in this topic, and I'll second that nomination.  I'd
> like to partipate in this discussion.
>
> We need to have better processes for vetting new syscalls and ABIs far
> more carefully than we currently do.  Right now, we require benchmarks
> for any claimed performance increase; it's almost a given that if you
> post an optimization without including benchmarks in the commit message,
> it'll get rejected with a request to come back with numbers.  We need
> similar standards for new syscalls or other userspace ABIs: come back
> with test programs, test coverage information, etc.
>

I am interested in this topic as well. To be effective and keep the
momentum going long term , we will need a way to regression test when
new APIs, new syscalls and ABIs are introduced. That would require a
look at existing tests and look into putting in some kind of framework
to easily test for regressions. It would also mean, when new API, ABIs
get added, "strongly" encourage developers add documentation and tests
cases.

-- Shuah


> _______________________________________________
> Ksummit-discuss mailing list
> Ksummit-discuss@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Andy Lutomirski]

On Tue, May 6, 2014 at 12:12 PM, Shuah Khan <shuahkhan@gmail.com> wrote:
> On Tue, May 6, 2014 at 11:58 AM,  <josh@joshtriplett.org> wrote:
>>
>> I'm interested in this topic, and I'll second that nomination.  I'd
>> like to partipate in this discussion.
>>
>> We need to have better processes for vetting new syscalls and ABIs far
>> more carefully than we currently do.  Right now, we require benchmarks
>> for any claimed performance increase; it's almost a given that if you
>> post an optimization without including benchmarks in the commit message,
>> it'll get rejected with a request to come back with numbers.  We need
>> similar standards for new syscalls or other userspace ABIs: come back
>> with test programs, test coverage information, etc.
>>
>
> I am interested in this topic as well. To be effective and keep the
> momentum going long term , we will need a way to regression test when
> new APIs, new syscalls and ABIs are introduced. That would require a
> look at existing tests and look into putting in some kind of framework
> to easily test for regressions. It would also mean, when new API, ABIs
> get added, "strongly" encourage developers add documentation and tests
> cases.

I think there was some discussion about in-tree kernel tests.  This
might fit in.

--Andy

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Shuah Khan]

On Tue, May 6, 2014 at 1:16 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Tue, May 6, 2014 at 12:12 PM, Shuah Khan <shuahkhan@gmail.com> wrote:
>> On Tue, May 6, 2014 at 11:58 AM,  <josh@joshtriplett.org> wrote:
>>>
>>> I'm interested in this topic, and I'll second that nomination.  I'd
>>> like to partipate in this discussion.
>>>
>>> We need to have better processes for vetting new syscalls and ABIs far
>>> more carefully than we currently do.  Right now, we require benchmarks
>>> for any claimed performance increase; it's almost a given that if you
>>> post an optimization without including benchmarks in the commit message,
>>> it'll get rejected with a request to come back with numbers.  We need
>>> similar standards for new syscalls or other userspace ABIs: come back
>>> with test programs, test coverage information, etc.
>>>
>>
>> I am interested in this topic as well. To be effective and keep the
>> momentum going long term , we will need a way to regression test when
>> new APIs, new syscalls and ABIs are introduced. That would require a
>> look at existing tests and look into putting in some kind of framework
>> to easily test for regressions. It would also mean, when new API, ABIs
>> get added, "strongly" encourage developers add documentation and tests
>> cases.
>
> I think there was some discussion about in-tree kernel tests.  This
> might fit in.
>

I am proposing adding regression test angle to this discussion. Review
is a good first step, however to ensure breakages don't occur, we have
to go one step further.

-- Shuah

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Johannes Berg]

On Tue, 2014-05-06 at 10:58 -0700, josh@joshtriplett.org wrote:

> We need to have better processes for vetting new syscalls and ABIs far
> more carefully than we currently do.

How far would you want to take this? New syscalls is one thing, but
there are frequently additions to "subsystem APIs", e.g. in networking,
that aren't really syscalls but part of netlink etc. Trying to vet all
of that might very well end up just overwhelming the process, but on the
other hand it's still something that probably should be done in some
form.

johannes

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Andy Lutomirski]

On Tue, May 6, 2014 at 12:21 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Tue, 2014-05-06 at 10:58 -0700, josh@joshtriplett.org wrote:
>
>> We need to have better processes for vetting new syscalls and ABIs far
>> more carefully than we currently do.
>
> How far would you want to take this? New syscalls is one thing, but
> there are frequently additions to "subsystem APIs", e.g. in networking,
> that aren't really syscalls but part of netlink etc. Trying to vet all
> of that might very well end up just overwhelming the process, but on the
> other hand it's still something that probably should be done in some
> form.
>

The snarky answer is: CVE-2014-0181.  I don't like netlink for
anything other than broadcasts from kernel space to user space.

A possibly better answer is that I think there are things that are
worthy of more care and things that are worthy of less care.  I also
think that it's more a question of the scope of the API than the
mechanism.  A debugfs thing, a sysfs entry for a particular device or
obscure configuration setting, or an ioctl on a device node are
possibly of less broad applicability.  Something like AF_ALG really is
a global API, though.  I would tend to classify many things that use
netlink in more-review category, since I don't think that the fact
that a new API uses netlink should exempt it from the same kind of
review it would need if it used a different mechanism.

--Andy

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Johannes Berg]

On Tue, 2014-05-06 at 12:43 -0700, Andy Lutomirski wrote:

> > How far would you want to take this? New syscalls is one thing, but
> > there are frequently additions to "subsystem APIs", e.g. in networking,
> > that aren't really syscalls but part of netlink etc. Trying to vet all
> > of that might very well end up just overwhelming the process, but on the
> > other hand it's still something that probably should be done in some
> > form.
> 
> The snarky answer is: CVE-2014-0181.  I don't like netlink for
> anything other than broadcasts from kernel space to user space.

That's also an entirely useless statement - netlink is neither going
away nor getting used less or being restricted. :)

> A possibly better answer is that I think there are things that are
> worthy of more care and things that are worthy of less care.  I also
> think that it's more a question of the scope of the API than the
> mechanism.  A debugfs thing, a sysfs entry for a particular device or
> obscure configuration setting, or an ioctl on a device node are
> possibly of less broad applicability.  Something like AF_ALG really is
> a global API, though.  I would tend to classify many things that use
> netlink in more-review category, since I don't think that the fact
> that a new API uses netlink should exempt it from the same kind of
> review it would need if it used a different mechanism.

Sure - still I'd think that the review process might be overwhelmed.
Particularly for domain-specific APIs (e.g. networking, or for me in
particular wireless) are not always entirely clear without that
domain-specific knowledge, nor am I convinced that it makes sense to try
to explain it in "laymen's terms", so to speak.

johannes

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Andy Lutomirski]

On Tue, May 6, 2014 at 12:48 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Tue, 2014-05-06 at 12:43 -0700, Andy Lutomirski wrote:
>
>> > How far would you want to take this? New syscalls is one thing, but
>> > there are frequently additions to "subsystem APIs", e.g. in networking,
>> > that aren't really syscalls but part of netlink etc. Trying to vet all
>> > of that might very well end up just overwhelming the process, but on the
>> > other hand it's still something that probably should be done in some
>> > form.
>>
>> The snarky answer is: CVE-2014-0181.  I don't like netlink for
>> anything other than broadcasts from kernel space to user space.
>
> That's also an entirely useless statement - netlink is neither going
> away nor getting used less or being restricted. :)

True.  And it's not really clear what kind of review would have caught
CVE-2014-0181.

>
>> A possibly better answer is that I think there are things that are
>> worthy of more care and things that are worthy of less care.  I also
>> think that it's more a question of the scope of the API than the
>> mechanism.  A debugfs thing, a sysfs entry for a particular device or
>> obscure configuration setting, or an ioctl on a device node are
>> possibly of less broad applicability.  Something like AF_ALG really is
>> a global API, though.  I would tend to classify many things that use
>> netlink in more-review category, since I don't think that the fact
>> that a new API uses netlink should exempt it from the same kind of
>> review it would need if it used a different mechanism.
>
> Sure - still I'd think that the review process might be overwhelmed.
> Particularly for domain-specific APIs (e.g. networking, or for me in
> particular wireless) are not always entirely clear without that
> domain-specific knowledge, nor am I convinced that it makes sense to try
> to explain it in "laymen's terms", so to speak.

That's fair.

Maybe one kind of test would be that APIs used by non-root or whole
new mechanisms (e.g. "how do I configure wireless in general") should
be reviewed more carefully than extensions of existing mechanisms
within their domain (e.g. "here's a new wireless thingy that works
just like the rest of them").

I certainly have zero interest in reviewing new wireless API calls,
despite the fact that my laptop is quite happy that they're there.

--Andy

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[josh]

On Tue, May 06, 2014 at 09:21:35PM +0200, Johannes Berg wrote:
> On Tue, 2014-05-06 at 10:58 -0700, josh@joshtriplett.org wrote:
> > We need to have better processes for vetting new syscalls and ABIs far
> > more carefully than we currently do.
> 
> How far would you want to take this? New syscalls is one thing, but
> there are frequently additions to "subsystem APIs", e.g. in networking,
> that aren't really syscalls but part of netlink etc. Trying to vet all
> of that might very well end up just overwhelming the process, but on the
> other hand it's still something that probably should be done in some
> form.

We ought to give such new APIs as much scrutiny as we do new system
calls.  But let's not let the lack of perfection prevent us from trying
to do better than we have.

- Josh Triplett

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Daniel Vetter]

On Tue, May 6, 2014 at 9:21 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2014-05-06 at 10:58 -0700, josh@joshtriplett.org wrote:
>
>> We need to have better processes for vetting new syscalls and ABIs far
>> more carefully than we currently do.
>
> How far would you want to take this? New syscalls is one thing, but
> there are frequently additions to "subsystem APIs", e.g. in networking,
> that aren't really syscalls but part of netlink etc. Trying to vet all
> of that might very well end up just overwhelming the process, but on the
> other hand it's still something that probably should be done in some
> form.

drm drivers add massive amounts of per-driver ioctls/sysfs files/...
and of course we've done all the usual (and a lot of the unusual) api
design mistakes. For i915 I've got fed up with all that and we've
switched to a fairly strict regime:

1. Patches that add new abi need to have full userspace ready before
the kernel side gets merged. Usually that means an (open source)
implementation of the OpenGL feature for that feature in mesa,
including all the functionality tests mesa requires (which are again
open source in piglit). New interfaces without fully validated and
working userspace parts get rejected.

2. I also require detailed testcases of all the corner-cases (reserved
bits/fields, signal handling, corener-cases/interactions with other
interfaces, overflow/array bounds issues, ...). Those low-level tests
are in intel-gpu-tools. I also have a lenghty blog with all the
mistakes we've done thus far wrt userspace interface design and what
kind of test coverage I expect:
http://blog.ffwll.ch/2013/11/botching-up-ioctls.html

3. Patches which require a testcase should have a Testcase: tag to
make sure both pieces (kernel patch + testcase) are linked together.
They usually also get reviewed in tandem by the same person.

I've made it very clear (and enforced this a bunch of times already)
that patches which don't follow this will be rejected. In my
experience (we've been doing this since about a year or so) this takes
care of the abi design fun completely.

I guess in a year or so I'll also add

4. Create/update the relevant manpage.

But currently we don't yet have that part set up (and also higher
priorities for documentation elsewhere in the stack).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Andy Lutomirski]

On Tue, May 6, 2014 at 1:10 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, May 6, 2014 at 9:21 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Tue, 2014-05-06 at 10:58 -0700, josh@joshtriplett.org wrote:
>>
>>> We need to have better processes for vetting new syscalls and ABIs far
>>> more carefully than we currently do.
>>
>> How far would you want to take this? New syscalls is one thing, but
>> there are frequently additions to "subsystem APIs", e.g. in networking,
>> that aren't really syscalls but part of netlink etc. Trying to vet all
>> of that might very well end up just overwhelming the process, but on the
>> other hand it's still something that probably should be done in some
>> form.
>
> drm drivers add massive amounts of per-driver ioctls/sysfs files/...
> and of course we've done all the usual (and a lot of the unusual) api
> design mistakes. For i915 I've got fed up with all that and we've
> switched to a fairly strict regime:
>
> 1. Patches that add new abi need to have full userspace ready before
> the kernel side gets merged. Usually that means an (open source)
> implementation of the OpenGL feature for that feature in mesa,
> including all the functionality tests mesa requires (which are again
> open source in piglit). New interfaces without fully validated and
> working userspace parts get rejected.
>
> 2. I also require detailed testcases of all the corner-cases (reserved
> bits/fields, signal handling, corener-cases/interactions with other
> interfaces, overflow/array bounds issues, ...). Those low-level tests
> are in intel-gpu-tools. I also have a lenghty blog with all the
> mistakes we've done thus far wrt userspace interface design and what
> kind of test coverage I expect:
> http://blog.ffwll.ch/2013/11/botching-up-ioctls.html
>
> 3. Patches which require a testcase should have a Testcase: tag to
> make sure both pieces (kernel patch + testcase) are linked together.
> They usually also get reviewed in tandem by the same person.
>
> I've made it very clear (and enforced this a bunch of times already)
> that patches which don't follow this will be rejected. In my
> experience (we've been doing this since about a year or so) this takes
> care of the abi design fun completely.
>
> I guess in a year or so I'll also add
>
> 4. Create/update the relevant manpage.
>
> But currently we don't yet have that part set up (and also higher
> priorities for documentation elsewhere in the stack).

I would love to see this at least for any new syscalls.

--Andy

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Laurent Pinchart]

On Tuesday 06 May 2014 21:21:35 Johannes Berg wrote:
> On Tue, 2014-05-06 at 10:58 -0700, josh@joshtriplett.org wrote:
> > We need to have better processes for vetting new syscalls and ABIs far
> > more carefully than we currently do.
> 
> How far would you want to take this? New syscalls is one thing, but
> there are frequently additions to "subsystem APIs", e.g. in networking,
> that aren't really syscalls but part of netlink etc. Trying to vet all
> of that might very well end up just overwhelming the process, but on the
> other hand it's still something that probably should be done in some
> form.

Part of the issue is not an unwillingness of subsystem maintainers and 
developers to apply proper review procedure to their new APIs, but rather a 
lack of knowledge regarding what such a review procedure should be.

We have seen several review, test and documentation procedures being developed 
for different subsystems in the recent past (two examples are the DRM i915 
driver API test rules explained by Daniel Vetter in a reply to this mail 
thread, and the V4L test suite and documentation procedure) but I have seen 
little effort to consolidate good practice rules. The kernel would certainly 
benefit from both sharing information about how various subsystems tackle the 
API review/test/documentation problem.

Forcing all subsystems to adhere and enforce a superset of rules would likely 
put too much burden on maintainers and developers, especially for the smaller 
subsystems. However, I believe we could help by gathering and consolidating 
the good practice rules in a single location under Documentation/. Maintainers 
could then implement those rules (or a subset thereof) without having to 
reinvent the wheel. Rules such as "return -EINVAL when a reserved parameter is 
set" are not complex to implement in code, the real challenge is to implement 
them in the brain of all developers and reviewers.

-- 
Regards,

Laurent Pinchart

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Daniel Vetter]

On Wed, May 7, 2014 at 12:12 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> We have seen several review, test and documentation procedures being developed
> for different subsystems in the recent past (two examples are the DRM i915
> driver API test rules explained by Daniel Vetter in a reply to this mail
> thread, and the V4L test suite and documentation procedure) but I have seen
> little effort to consolidate good practice rules. The kernel would certainly
> benefit from both sharing information about how various subsystems tackle the
> API review/test/documentation problem.
>
> Forcing all subsystems to adhere and enforce a superset of rules would likely
> put too much burden on maintainers and developers, especially for the smaller
> subsystems. However, I believe we could help by gathering and consolidating
> the good practice rules in a single location under Documentation/. Maintainers
> could then implement those rules (or a subset thereof) without having to
> reinvent the wheel. Rules such as "return -EINVAL when a reserved parameter is
> set" are not complex to implement in code, the real challenge is to implement
> them in the brain of all developers and reviewers.

I'd be very interested in a discussions about existing best practices
already developed in different subsystems and figuring out what
minimal standards we should requires across the board.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Laurent Pinchart]

On Wednesday 07 May 2014 14:36:27 Daniel Vetter wrote:
> On Wed, May 7, 2014 at 12:12 PM, Laurent Pinchart wrote:
> > We have seen several review, test and documentation procedures being
> > developed for different subsystems in the recent past (two examples are
> > the DRM i915 driver API test rules explained by Daniel Vetter in a reply
> > to this mail thread, and the V4L test suite and documentation procedure)
> > but I have seen little effort to consolidate good practice rules. The
> > kernel would certainly benefit from both sharing information about how
> > various subsystems tackle the API review/test/documentation problem.
> > 
> > Forcing all subsystems to adhere and enforce a superset of rules would
> > likely put too much burden on maintainers and developers, especially for
> > the smaller subsystems. However, I believe we could help by gathering and
> > consolidating the good practice rules in a single location under
> > Documentation/. Maintainers could then implement those rules (or a subset
> > thereof) without having to reinvent the wheel. Rules such as "return
> > -EINVAL when a reserved parameter is set" are not complex to implement in
> > code, the real challenge is to implement them in the brain of all
> > developers and reviewers.
> 
> I'd be very interested in a discussions about existing best practices
> already developed in different subsystems and figuring out what
> minimal standards we should requires across the board.

So would I. The same topic is being discussed in the "stable issues" mail 
thread, it thus looks like a good candidate to me. On the V4L side Hans 
Verkuil (CC'ed) would likely be interested as a core V4L reviewer and 
userspace test case developer.

-- 
Regards,

Laurent Pinchart

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Hans Verkuil]

On 05/07/14 15:30, Laurent Pinchart wrote:
> On Wednesday 07 May 2014 14:36:27 Daniel Vetter wrote:
>> On Wed, May 7, 2014 at 12:12 PM, Laurent Pinchart wrote:
>>> We have seen several review, test and documentation procedures being
>>> developed for different subsystems in the recent past (two examples are
>>> the DRM i915 driver API test rules explained by Daniel Vetter in a reply
>>> to this mail thread, and the V4L test suite and documentation procedure)
>>> but I have seen little effort to consolidate good practice rules. The
>>> kernel would certainly benefit from both sharing information about how
>>> various subsystems tackle the API review/test/documentation problem.
>>>
>>> Forcing all subsystems to adhere and enforce a superset of rules would
>>> likely put too much burden on maintainers and developers, especially for
>>> the smaller subsystems. However, I believe we could help by gathering and
>>> consolidating the good practice rules in a single location under
>>> Documentation/. Maintainers could then implement those rules (or a subset
>>> thereof) without having to reinvent the wheel. Rules such as "return
>>> -EINVAL when a reserved parameter is set" are not complex to implement in
>>> code, the real challenge is to implement them in the brain of all
>>> developers and reviewers.
>>
>> I'd be very interested in a discussions about existing best practices
>> already developed in different subsystems and figuring out what
>> minimal standards we should requires across the board.
> 
> So would I. The same topic is being discussed in the "stable issues" mail 
> thread, it thus looks like a good candidate to me. On the V4L side Hans 
> Verkuil (CC'ed) would likely be interested as a core V4L reviewer and 
> userspace test case developer.
> 

Absolutely. I've done a lot of work in that area. The V4L API is very large,
and without compliance tools it is almost impossible to write a faultless
driver. We now have about 95% coverage of the whole API and these days it is
a requirement of new drivers that they pass the test suite, otherwise I
will reject them.

And the flip-side of that is to provide applications with good test drivers
so they can test their application without requiring hard-to-get hardware
if they want to test support for features that are not commonly found.

I developed a very nice test driver for that. It needs a bit more work before
I can upstream it, though.

My experience with creating these tools is that they force you to take a
very critical look at the API. Any ambiguities you missed tend to be exposed
by creating these compliance tests.

Regards,

	Hans

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]

On Wed, May 07, 2014 at 02:36:27PM +0200, Daniel Vetter wrote:
> On Wed, May 7, 2014 at 12:12 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > We have seen several review, test and documentation procedures being developed
> > for different subsystems in the recent past (two examples are the DRM i915
> > driver API test rules explained by Daniel Vetter in a reply to this mail
> > thread, and the V4L test suite and documentation procedure) but I have seen
> > little effort to consolidate good practice rules. The kernel would certainly
> > benefit from both sharing information about how various subsystems tackle the
> > API review/test/documentation problem.
> >
> > Forcing all subsystems to adhere and enforce a superset of rules would likely
> > put too much burden on maintainers and developers, especially for the smaller
> > subsystems. However, I believe we could help by gathering and consolidating
> > the good practice rules in a single location under Documentation/. Maintainers
> > could then implement those rules (or a subset thereof) without having to
> > reinvent the wheel. Rules such as "return -EINVAL when a reserved parameter is
> > set" are not complex to implement in code, the real challenge is to implement
> > them in the brain of all developers and reviewers.
> 
> I'd be very interested in a discussions about existing best practices
> already developed in different subsystems and figuring out what
> minimal standards we should requires across the board.

+1

I'd like to have some procedures for my subsystem as well and was
wondering what other people have experienced so far. I already started
digging and trying to find a scheme where that stuff could be collected.
Never got too far sadly :(


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Michael Kerrisk (man-pages)]

On 05/06/2014 07:58 PM, josh@joshtriplett.org wrote:
> On Tue, May 06, 2014 at 10:45:05AM -0700, Andy Lutomirski wrote:
>> There doesn't currently seem to be any real process for reviewing new
>> core APIs for sanity of design, appropriateness to solve the problem
>> they're targetting, or correctness of implementation.
> [...]
>> I think that the process could be improved.  

I agree, and I think the required steps are pretty clear.
The question is how much will there is to implement them.

>>  I think that there are
>> people who are willing to spend time to read API docs and thinking
>> about these kinds of issues.  (I am, and Michael Kerrisk seems to do a
>> fair amount of it.)

Yes. And I'd do more if I had more time... My free (as in beer) time
is already overcommitted on that task.

>> I would like to discuss what we could change to make this work better
>> in the future.  In an ideal world, I would like to see every core API
>> change come with documentation, tests (possibly simple ones), and a
>> post, with acks, to a list that covers just API changes.  This might

That list would be (the largely overlooked) linux-api@

>> be tough, but it could add a lot of value.

Yes. As I've already said recently, there's an awful lot of crap
still hitting userspace.

>> I've occasionally thought that API docs should live in the kernel tree
>> in some reasonably well organized place so that patch sets can include
>> doc patches.  I realize that this is contentious.

I understand the arguments in favor, but I'm not sure that that 
technological measure would make much difference. And it's not so much
that it's contentious, it's just that there are, from my point of 
view, downsides to implementing it, which I've described here:
https://www.kernel.org/doc/man-pages/todo.html#migrate_to_kernel_source
There, among other things I hypothesize about an alternative:

    Employ a couple of patch tags in the style of "Signed-off-by:". 
    One of these could be (say) "ABI-changes-noted-by:", indicating 
    that someone has noted that this patch changes the API/ABI. The 
    other would be (say) "ABI-changes-doc-acked-by:", an indication
    from the appropriate documentation maintainer that the ABI
    changes have been documented in man-pages (or elsewhere if
    appropriate); details of the actual documentation could be
    included elsewhere in the patch's log message. 

What do others think of this?

>> I'm sure that other people have other suggestions here.
>>
>> --Andy
>>
>> P.S.  I'm not on the invitation nominee list, so if people like this
>> topic, I'd appreciate a nomination :)
> 
> I'm interested in this topic, and I'll second that nomination.  I'd
> like to partipate in this discussion.

So, would I.

> We need to have better processes for vetting new syscalls and ABIs far
> more carefully than we currently do.  Right now, we require benchmarks
> for any claimed performance increase; it's almost a given that if you
> post an optimization without including benchmarks in the commit message,
> it'll get rejected with a request to come back with numbers.  We need
> similar standards for new syscalls or other userspace ABIs: come back
> with test programs, test coverage information, etc.

I'd just want to add there that thorough documentation is integral part 
of this. Otherwise, how do we tell tell what we're testing against?

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Greg KH]

On Tue, May 06, 2014 at 10:45:05AM -0700, Andy Lutomirski wrote:
> There doesn't currently seem to be any real process for reviewing new
> core APIs for sanity of design, appropriateness to solve the problem
> they're targetting, or correctness of implementation.  Some examples
> that come to mind recently:
> 
>  - A lot of people seem to think that O_TMPFILE is a terrible
> interface, despite the fact that the functionality it provides is very
> useful.  It was also rather badly broken until -rc8 or so.
> 
>  - The x86 32-bit vdso clock functions almost made it in with a
> questionable symbol version.
> 
>  - 3.15 is currently slated to include an unfortunate ABI glitch in
> the MIPS seccomp filter interface.  There's a patch.
> 
>  - There are some aspects of the keyring API that seem to me to be quite bad.
> 
>  - An impressive number of new APIs are missing -EINVAL returns if
> reserved parameters are set.
> 
> (I'm not trying to point a finger at anyone with these examples;
> they're just things that I was involved in to some extent.)
> 
> The current process is confused.   For example, I currently plan on
> trying to remember to ask Linus to revert the MIPS seccomp stuff or
> fix it sometime around -rc6 if the patch hasn't landed, but this
> sucks.
> 
> I think that the process could be improved.  I think that there are
> people who are willing to spend time to read API docs and thinking
> about these kinds of issues.  (I am, and Michael Kerrisk seems to do a
> fair amount of it.)

We do have linux-api, which should be cc:ed for new api additions.  But
it usually isn't :(

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Steven Rostedt]

On Tue, 6 May 2014 12:00:24 -0700
Greg KH <greg@kroah.com> wrote:


> We do have linux-api, which should be cc:ed for new api additions.  But
> it usually isn't :(

That's the first I've heard of that list. Maybe that's the reason it
hasn't been cc'd much. Hard to Cc lists that you don't know about.

/me goes to look in MAINTAINERS for linux-api mailing list. Nope, not
there.

git grep linux-api gives me:

Documentation/HOWTO:linux-api@vger.kernel.org.
Documentation/SubmitChecklist:    linux-api@vger.kernel.org.
Documentation/ja_JP/HOWTO:linux-api@ver.kernel.org
に送ることを勧めます。

Cool, it's in Japanese too :-)

Commit 1d992ce90 added the email to SubmitChecklist in 2008. Not
something I've looked at since :-/

I guess I should really look at the Documentation directory more often.
God, it's hard enough to get people to write documentation. It is even
harder to get people to read it. :-p


-- Steve

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Josh Triplett]

On Tue, May 06, 2014 at 04:07:40PM -0400, Steven Rostedt wrote:
> On Tue, 6 May 2014 12:00:24 -0700
> Greg KH <greg@kroah.com> wrote:
> 
> 
> > We do have linux-api, which should be cc:ed for new api additions.  But
> > it usually isn't :(
> 
> That's the first I've heard of that list. Maybe that's the reason it
> hasn't been cc'd much. Hard to Cc lists that you don't know about.
> 
> /me goes to look in MAINTAINERS for linux-api mailing list. Nope, not
> there.

Here's a patch:

---8<---
>From 424c7365ed5ed64190a1d08fe30c8e86832368b4 Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@joshtriplett.org>
Date: Tue, 6 May 2014 13:20:25 -0700
Subject: [PATCH] MAINTAINERS: Add linux-api for review of API/ABI changes

This makes it more likely that patch submitters will CC API/ABI changes
to the linux-api list, and tools like get_maintainer.pl will do so
automatically.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7578deb..52a282b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -210,6 +210,13 @@ S:	Supported
 F:	Documentation/scsi/aacraid.txt
 F:	drivers/scsi/aacraid/
 
+ABI/API
+L:	linux-api@vger.kernel.org
+F:	Documentation/ABI/
+F:	include/linux/syscalls.h
+F:	include/uapi/
+F:	kernel/sys_ni.c
+
 ABIT UGURU 1,2 HARDWARE MONITOR DRIVER
 M:	Hans de Goede <hdegoede@redhat.com>
 L:	lm-sensors@lm-sensors.org
-- 
2.0.0.rc0

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Steven Rostedt]

On Tue, 6 May 2014 13:34:26 -0700
Josh Triplett <josh@joshtriplett.org> wrote:

> On Tue, May 06, 2014 at 04:07:40PM -0400, Steven Rostedt wrote:
> > On Tue, 6 May 2014 12:00:24 -0700
> > Greg KH <greg@kroah.com> wrote:
> > 
> > 
> > > We do have linux-api, which should be cc:ed for new api additions.  But
> > > it usually isn't :(
> > 
> > That's the first I've heard of that list. Maybe that's the reason it
> > hasn't been cc'd much. Hard to Cc lists that you don't know about.
> > 
> > /me goes to look in MAINTAINERS for linux-api mailing list. Nope, not
> > there.
> 
> Here's a patch:
> 
> ---8<---
> >From 424c7365ed5ed64190a1d08fe30c8e86832368b4 Mon Sep 17 00:00:00 2001
> From: Josh Triplett <josh@joshtriplett.org>
> Date: Tue, 6 May 2014 13:20:25 -0700
> Subject: [PATCH] MAINTAINERS: Add linux-api for review of API/ABI changes
> 
> This makes it more likely that patch submitters will CC API/ABI changes
> to the linux-api list, and tools like get_maintainer.pl will do so
> automatically.
> 

Although patches need to go to LKML.

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7578deb..52a282b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -210,6 +210,13 @@ S:	Supported
>  F:	Documentation/scsi/aacraid.txt
>  F:	drivers/scsi/aacraid/
>  
> +ABI/API
> +L:	linux-api@vger.kernel.org
> +F:	Documentation/ABI/
> +F:	include/linux/syscalls.h
> +F:	include/uapi/
> +F:	kernel/sys_ni.c
> +
>  ABIT UGURU 1,2 HARDWARE MONITOR DRIVER
>  M:	Hans de Goede <hdegoede@redhat.com>
>  L:	lm-sensors@lm-sensors.org

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[josh]

On Tue, May 06, 2014 at 04:42:01PM -0400, Steven Rostedt wrote:
> On Tue, 6 May 2014 13:34:26 -0700
> Josh Triplett <josh@joshtriplett.org> wrote:
> 
> > On Tue, May 06, 2014 at 04:07:40PM -0400, Steven Rostedt wrote:
> > > On Tue, 6 May 2014 12:00:24 -0700
> > > Greg KH <greg@kroah.com> wrote:
> > > 
> > > 
> > > > We do have linux-api, which should be cc:ed for new api additions.  But
> > > > it usually isn't :(
> > > 
> > > That's the first I've heard of that list. Maybe that's the reason it
> > > hasn't been cc'd much. Hard to Cc lists that you don't know about.
> > > 
> > > /me goes to look in MAINTAINERS for linux-api mailing list. Nope, not
> > > there.
> > 
> > Here's a patch:
> > 
> > ---8<---
> > >From 424c7365ed5ed64190a1d08fe30c8e86832368b4 Mon Sep 17 00:00:00 2001
> > From: Josh Triplett <josh@joshtriplett.org>
> > Date: Tue, 6 May 2014 13:20:25 -0700
> > Subject: [PATCH] MAINTAINERS: Add linux-api for review of API/ABI changes
> > 
> > This makes it more likely that patch submitters will CC API/ABI changes
> > to the linux-api list, and tools like get_maintainer.pl will do so
> > automatically.
> > 
> 
> Although patches need to go to LKML.
> 
> Acked-by: Steven Rostedt <rostedt@goodmis.org>

Resent to LKML.

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Jiri Kosina]

On Tue, 6 May 2014, Josh Triplett wrote:

> From 424c7365ed5ed64190a1d08fe30c8e86832368b4 Mon Sep 17 00:00:00 2001
> From: Josh Triplett <josh@joshtriplett.org>
> Date: Tue, 6 May 2014 13:20:25 -0700
> Subject: [PATCH] MAINTAINERS: Add linux-api for review of API/ABI changes
> 
> This makes it more likely that patch submitters will CC API/ABI changes
> to the linux-api list, and tools like get_maintainer.pl will do so
> automatically.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7578deb..52a282b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -210,6 +210,13 @@ S:	Supported
>  F:	Documentation/scsi/aacraid.txt
>  F:	drivers/scsi/aacraid/
>  
> +ABI/API
> +L:	linux-api@vger.kernel.org
> +F:	Documentation/ABI/
> +F:	include/linux/syscalls.h
> +F:	include/uapi/
> +F:	kernel/sys_ni.c
> +

It'd be nice to have also sysfs changes covered as well if this is not 
just about API, but also ABI; I am not sure whether this could be covered 
by MAINTAINERS pattern, but at least mentioning this list in 
Documentation/ABI/README seems appropriate.

(I am also wondering who is actually subsribed to the linux-api@ 
mailinglist, given the low general awareness of its existance :) ).

-- 
Jiri Kosina
SUSE Labs

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Li Zefan]

On 2014/5/7 19:48, Jiri Kosina wrote:
> On Tue, 6 May 2014, Josh Triplett wrote:
> 
>> From 424c7365ed5ed64190a1d08fe30c8e86832368b4 Mon Sep 17 00:00:00 2001
>> From: Josh Triplett <josh@joshtriplett.org>
>> Date: Tue, 6 May 2014 13:20:25 -0700
>> Subject: [PATCH] MAINTAINERS: Add linux-api for review of API/ABI changes
>>
>> This makes it more likely that patch submitters will CC API/ABI changes
>> to the linux-api list, and tools like get_maintainer.pl will do so
>> automatically.
>>
>> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
>> ---
>>  MAINTAINERS | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7578deb..52a282b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -210,6 +210,13 @@ S:	Supported
>>  F:	Documentation/scsi/aacraid.txt
>>  F:	drivers/scsi/aacraid/
>>  
>> +ABI/API
>> +L:	linux-api@vger.kernel.org
>> +F:	Documentation/ABI/
>> +F:	include/linux/syscalls.h
>> +F:	include/uapi/
>> +F:	kernel/sys_ni.c
>> +
> 
> It'd be nice to have also sysfs changes covered as well if this is not 
> just about API, but also ABI; I am not sure whether this could be covered 
> by MAINTAINERS pattern, but at least mentioning this list in 
> Documentation/ABI/README seems appropriate.
> 

If sysfs is in, what about cgroupfs? Which has been under dramatic changes.

> (I am also wondering who is actually subsribed to the linux-api@ 
> mailinglist, given the low general awareness of its existance :) ).
> 

Maybe we should force all core deveopers to subscribe linux-api?

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Jiri Kosina]

On Thu, 8 May 2014, Li Zefan wrote:

> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 7578deb..52a282b 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -210,6 +210,13 @@ S:	Supported
> >>  F:	Documentation/scsi/aacraid.txt
> >>  F:	drivers/scsi/aacraid/
> >>  
> >> +ABI/API
> >> +L:	linux-api@vger.kernel.org
> >> +F:	Documentation/ABI/
> >> +F:	include/linux/syscalls.h
> >> +F:	include/uapi/
> >> +F:	kernel/sys_ni.c
> >> +
> > 
> > It'd be nice to have also sysfs changes covered as well if this is not 
> > just about API, but also ABI; I am not sure whether this could be covered 
> > by MAINTAINERS pattern, but at least mentioning this list in 
> > Documentation/ABI/README seems appropriate.
> 
> If sysfs is in, 

Well, sysfs definitely is considered kernel ABI.

> what about cgroupfs? Which has been under dramatic changes.

This is of course interesting (and important) question per se -- the 
amount of interfaces between userspace and kernelspace is growing at a 
rapid pace (especially a lot of crap gets added into debugfs I guess), and 
it's not always clearly defined what is considered proper ABI and thus 
should be maintained stable.

Is "if we ever get a report about userspace regression because of kernel 
interface change" enough well-defined criteria? What if there is userspace 
depending on the contents of the kernel ringbuffer? Etc etc ...

-- 
Jiri Kosina
SUSE Labs

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Michael Kerrisk (man-pages)]

On 05/06/2014 09:00 PM, Greg KH wrote:

> We do have linux-api, which should be cc:ed for new api additions.  But
> it usually isn't :(

As I've just said in reply to Josh Triplett's MAINTAINERS patch, I think 
they key here is persistence. Every time you see something that changes 
the API/ABI, ask the submitter to CC linux-api (and CC linux-api on 
that request). I try to do this now and then, but I catch just a few
drops from the LKML firehose. If enough people do this often enough,
I think there's a chance to bring this discipline to the culture.

CCing linux-api@ really would help a lot of people:
https://www.kernel.org/doc/man-pages/linux-api-ml.html

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Dan Carpenter]

In olden times we used to have LKML for this sorts of discussion.  Last
year we decided that LKML was officially useless in terms of anyone
reading it.  We decided we would re-use the kernel-summit list for this
kind of API discussion.

The problem with LKML is that everyone CCs it for everything.  The rule
for the new list was going to be that you couldn't cross post or CC any
other list for the discussion.

In the end the list never got off the ground.

Part of the reason it failed is that no one is going to CC there own
patches to the list for extra scrutiny.  Why ask for punishment?

regards,
dan carpenter

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Randy Dunlap]

On 05/06/2014 12:57 PM, Dan Carpenter wrote:
> In olden times we used to have LKML for this sorts of discussion.  Last
> year we decided that LKML was officially useless in terms of anyone
> reading it.  We decided we would re-use the kernel-summit list for this
> kind of API discussion.

That seems weird to me since I had never heard about that quirk^W information.
I wouldn't expect ksummit-discuss to be used for API discussions.

> The problem with LKML is that everyone CCs it for everything.  The rule
> for the new list was going to be that you couldn't cross post or CC any
> other list for the discussion.
> 
> In the end the list never got off the ground.
> 
> Part of the reason it failed is that no one is going to CC there own
> patches to the list for extra scrutiny.  Why ask for punishment?


-- 
~Randy

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Jeff Layton]

On Tue, 6 May 2014 10:45:05 -0700
Andy Lutomirski <luto@amacapital.net> wrote:

> There doesn't currently seem to be any real process for reviewing new
> core APIs for sanity of design, appropriateness to solve the problem
> they're targetting, or correctness of implementation.  Some examples
> that come to mind recently:
> 
>  - A lot of people seem to think that O_TMPFILE is a terrible
> interface, despite the fact that the functionality it provides is very
> useful.  It was also rather badly broken until -rc8 or so.
> 
>  - The x86 32-bit vdso clock functions almost made it in with a
> questionable symbol version.
> 
>  - 3.15 is currently slated to include an unfortunate ABI glitch in
> the MIPS seccomp filter interface.  There's a patch.
> 
>  - There are some aspects of the keyring API that seem to me to be quite bad.
> 
>  - An impressive number of new APIs are missing -EINVAL returns if
> reserved parameters are set.
> 
> (I'm not trying to point a finger at anyone with these examples;
> they're just things that I was involved in to some extent.)
> 
> The current process is confused.   For example, I currently plan on
> trying to remember to ask Linus to revert the MIPS seccomp stuff or
> fix it sometime around -rc6 if the patch hasn't landed, but this
> sucks.
> 
> I think that the process could be improved.  I think that there are
> people who are willing to spend time to read API docs and thinking
> about these kinds of issues.  (I am, and Michael Kerrisk seems to do a
> fair amount of it.)
> 
> I would like to discuss what we could change to make this work better
> in the future.  In an ideal world, I would like to see every core API
> change come with documentation, tests (possibly simple ones), and a
> post, with acks, to a list that covers just API changes.  This might
> be tough, but it could add a lot of value.
> 
> I've occasionally thought that API docs should live in the kernel tree
> in some reasonably well organized place so that patch sets can include
> doc patches.  I realize that this is contentious.
> 
> I'm sure that other people have other suggestions here.
> 
> --Andy
> 
> P.S.  I'm not on the invitation nominee list, so if people like this
> topic, I'd appreciate a nomination :)


I'd be interested in discussing this as well.

I just went through a bunch of gyrations with the file-private -> OFD
file lock renaming. I originally posted these patches over several
months, but it wasn't until it was merged that people started speaking
out over the name. Perhaps if I had known about linux-api@ and had
cc'ed it on those patches, we might have gotten that squared away
earlier?

Also, I'm still interested in getting this driven back into POSIX spec,
so having a more well-defined way to interact with the POSIX folks
would be helpful.

There are some open questions here too, notably: How are we defining
API/ABI? Clearly a new syscall is a change that needs to be carefully
vetted. What about a new mount option? Does that qualify?

While I think having a lot of eyes on userland-visible changes is a
good thing, we need to take heed not to make such a process too formal
or it'll become painful to deal with.

-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI

[Michael Kerrisk (man-pages)]

On Fri, May 9, 2014 at 1:33 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> On Tue, 6 May 2014 10:45:05 -0700
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> There doesn't currently seem to be any real process for reviewing new
>> core APIs for sanity of design, appropriateness to solve the problem
>> they're targetting, or correctness of implementation.  Some examples
>> that come to mind recently:
>>
>>  - A lot of people seem to think that O_TMPFILE is a terrible
>> interface, despite the fact that the functionality it provides is very
>> useful.  It was also rather badly broken until -rc8 or so.
>>
>>  - The x86 32-bit vdso clock functions almost made it in with a
>> questionable symbol version.
>>
>>  - 3.15 is currently slated to include an unfortunate ABI glitch in
>> the MIPS seccomp filter interface.  There's a patch.
>>
>>  - There are some aspects of the keyring API that seem to me to be quite bad.
>>
>>  - An impressive number of new APIs are missing -EINVAL returns if
>> reserved parameters are set.
>>
>> (I'm not trying to point a finger at anyone with these examples;
>> they're just things that I was involved in to some extent.)
>>
>> The current process is confused.   For example, I currently plan on
>> trying to remember to ask Linus to revert the MIPS seccomp stuff or
>> fix it sometime around -rc6 if the patch hasn't landed, but this
>> sucks.
>>
>> I think that the process could be improved.  I think that there are
>> people who are willing to spend time to read API docs and thinking
>> about these kinds of issues.  (I am, and Michael Kerrisk seems to do a
>> fair amount of it.)
>>
>> I would like to discuss what we could change to make this work better
>> in the future.  In an ideal world, I would like to see every core API
>> change come with documentation, tests (possibly simple ones), and a
>> post, with acks, to a list that covers just API changes.  This might
>> be tough, but it could add a lot of value.
>>
>> I've occasionally thought that API docs should live in the kernel tree
>> in some reasonably well organized place so that patch sets can include
>> doc patches.  I realize that this is contentious.
>>
>> I'm sure that other people have other suggestions here.
>>
>> --Andy
>>
>> P.S.  I'm not on the invitation nominee list, so if people like this
>> topic, I'd appreciate a nomination :)
>
>
> I'd be interested in discussing this as well.
>
> I just went through a bunch of gyrations with the file-private -> OFD
> file lock renaming. I originally posted these patches over several
> months, but it wasn't until it was merged that people started speaking
> out over the name. Perhaps if I had known about linux-api@ and had
> cc'ed it on those patches, we might have gotten that squared away
> earlier?

Maybe. I think there's also the problem that the number of people
looking at this stuff closely is fairly small. I mean, right now I'm
looking at documenting sched_setattr(), and I've just spotted a second
API bug (Peter Z, you'll get another mail soon ;-)), without too much
effort. That says to me that there probably wasn't anyone who, with
some distance from the implementation, looked very closely at the
details of the API. And that's a loop I've gone though countless times
when writing man pages.

> Also, I'm still interested in getting this driven back into POSIX spec,
> so having a more well-defined way to interact with the POSIX folks
> would be helpful.

Well, your RH colleague Eric Blake would seem to be a good "in"--he's
regularly in on the Austin Group meetings, AFAICT.

> There are some open questions here too, notably: How are we defining
> API/ABI? Clearly a new syscall is a change that needs to be carefully
> vetted. What about a new mount option? Does that qualify?

As in a new MS_* flag to mount(2)? I'd have said yes.

> While I think having a lot of eyes on userland-visible changes is a
> good thing, we need to take heed not to make such a process too formal
> or it'll become painful to deal with.

With my hat off to you, because you did most everything right, I have
to say that when I hear a kernel developer say this, it sounds to me
suspiciously like: "I want someone else to bear the pain". That
someone else is typically a large number of user-space programmers who
end up dealing with bugs and broken APIs for a long time. I don't know
how formal would be too formal, but I think we're a long way from that
point and a away from where we should be in terms of rigor when it
comes to API/ABI changes.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/