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

[Daniel Vetter <daniel.vetter@ffwll.ch>]

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