* [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI @ 2014-05-06 17:45 Andy Lutomirski 2014-05-06 17:58 ` josh ` (3 more replies) 0 siblings, 4 replies; 31+ messages in thread From: Andy Lutomirski @ 2014-05-06 17:45 UTC (permalink / raw) To: ksummit-discuss, Michael Kerrisk-manpages 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 :) ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 17:45 [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI Andy Lutomirski @ 2014-05-06 17:58 ` josh 2014-05-06 19:12 ` Shuah Khan ` (2 more replies) 2014-05-06 19:00 ` Greg KH ` (2 subsequent siblings) 3 siblings, 3 replies; 31+ messages in thread From: josh @ 2014-05-06 17:58 UTC (permalink / raw) To: Andy Lutomirski; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 17:58 ` josh @ 2014-05-06 19:12 ` Shuah Khan 2014-05-06 19:16 ` Andy Lutomirski 2014-05-06 19:21 ` Johannes Berg 2014-05-07 17:48 ` Michael Kerrisk (man-pages) 2 siblings, 1 reply; 31+ messages in thread From: Shuah Khan @ 2014-05-06 19:12 UTC (permalink / raw) To: josh; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:12 ` Shuah Khan @ 2014-05-06 19:16 ` Andy Lutomirski 2014-05-06 19:37 ` Shuah Khan 0 siblings, 1 reply; 31+ messages in thread From: Andy Lutomirski @ 2014-05-06 19:16 UTC (permalink / raw) To: Shuah Khan; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:16 ` Andy Lutomirski @ 2014-05-06 19:37 ` Shuah Khan 0 siblings, 0 replies; 31+ messages in thread From: Shuah Khan @ 2014-05-06 19:37 UTC (permalink / raw) To: Andy Lutomirski; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 17:58 ` josh 2014-05-06 19:12 ` Shuah Khan @ 2014-05-06 19:21 ` Johannes Berg 2014-05-06 19:43 ` Andy Lutomirski ` (3 more replies) 2014-05-07 17:48 ` Michael Kerrisk (man-pages) 2 siblings, 4 replies; 31+ messages in thread From: Johannes Berg @ 2014-05-06 19:21 UTC (permalink / raw) To: josh; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:21 ` Johannes Berg @ 2014-05-06 19:43 ` Andy Lutomirski 2014-05-06 19:48 ` Johannes Berg 2014-05-06 19:45 ` josh ` (2 subsequent siblings) 3 siblings, 1 reply; 31+ messages in thread From: Andy Lutomirski @ 2014-05-06 19:43 UTC (permalink / raw) To: Johannes Berg; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:43 ` Andy Lutomirski @ 2014-05-06 19:48 ` Johannes Berg 2014-05-06 19:51 ` Andy Lutomirski 0 siblings, 1 reply; 31+ messages in thread From: Johannes Berg @ 2014-05-06 19:48 UTC (permalink / raw) To: Andy Lutomirski; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:48 ` Johannes Berg @ 2014-05-06 19:51 ` Andy Lutomirski 0 siblings, 0 replies; 31+ messages in thread From: Andy Lutomirski @ 2014-05-06 19:51 UTC (permalink / raw) To: Johannes Berg; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:21 ` Johannes Berg 2014-05-06 19:43 ` Andy Lutomirski @ 2014-05-06 19:45 ` josh 2014-05-06 20:10 ` Daniel Vetter 2014-05-07 10:12 ` Laurent Pinchart 3 siblings, 0 replies; 31+ messages in thread From: josh @ 2014-05-06 19:45 UTC (permalink / raw) To: Johannes Berg; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:21 ` Johannes Berg 2014-05-06 19:43 ` Andy Lutomirski 2014-05-06 19:45 ` josh @ 2014-05-06 20:10 ` Daniel Vetter 2014-05-06 20:13 ` Andy Lutomirski 2014-05-07 10:12 ` Laurent Pinchart 3 siblings, 1 reply; 31+ messages in thread From: Daniel Vetter @ 2014-05-06 20:10 UTC (permalink / raw) To: Johannes Berg; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 20:10 ` Daniel Vetter @ 2014-05-06 20:13 ` Andy Lutomirski 0 siblings, 0 replies; 31+ messages in thread From: Andy Lutomirski @ 2014-05-06 20:13 UTC (permalink / raw) To: Daniel Vetter; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:21 ` Johannes Berg ` (2 preceding siblings ...) 2014-05-06 20:10 ` Daniel Vetter @ 2014-05-07 10:12 ` Laurent Pinchart 2014-05-07 12:36 ` Daniel Vetter 3 siblings, 1 reply; 31+ messages in thread From: Laurent Pinchart @ 2014-05-07 10:12 UTC (permalink / raw) To: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-07 10:12 ` Laurent Pinchart @ 2014-05-07 12:36 ` Daniel Vetter 2014-05-07 13:30 ` Laurent Pinchart 2014-05-12 14:15 ` Wolfram Sang 0 siblings, 2 replies; 31+ messages in thread From: Daniel Vetter @ 2014-05-07 12:36 UTC (permalink / raw) To: Laurent Pinchart; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-07 12:36 ` Daniel Vetter @ 2014-05-07 13:30 ` Laurent Pinchart 2014-05-07 13:50 ` Hans Verkuil 2014-05-12 14:15 ` Wolfram Sang 1 sibling, 1 reply; 31+ messages in thread From: Laurent Pinchart @ 2014-05-07 13:30 UTC (permalink / raw) To: Daniel Vetter; +Cc: Hans Verkuil, ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-07 13:30 ` Laurent Pinchart @ 2014-05-07 13:50 ` Hans Verkuil 0 siblings, 0 replies; 31+ messages in thread From: Hans Verkuil @ 2014-05-07 13:50 UTC (permalink / raw) To: Laurent Pinchart, Daniel Vetter; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-07 12:36 ` Daniel Vetter 2014-05-07 13:30 ` Laurent Pinchart @ 2014-05-12 14:15 ` Wolfram Sang 1 sibling, 0 replies; 31+ messages in thread From: Wolfram Sang @ 2014-05-12 14:15 UTC (permalink / raw) To: Daniel Vetter; +Cc: ksummit-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 17:58 ` josh 2014-05-06 19:12 ` Shuah Khan 2014-05-06 19:21 ` Johannes Berg @ 2014-05-07 17:48 ` Michael Kerrisk (man-pages) 2 siblings, 0 replies; 31+ messages in thread From: Michael Kerrisk (man-pages) @ 2014-05-07 17:48 UTC (permalink / raw) To: josh, Andy Lutomirski; +Cc: ksummit-discuss 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/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 17:45 [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI Andy Lutomirski 2014-05-06 17:58 ` josh @ 2014-05-06 19:00 ` Greg KH 2014-05-06 20:07 ` Steven Rostedt 2014-05-07 6:27 ` Michael Kerrisk (man-pages) 2014-05-06 19:57 ` Dan Carpenter 2014-05-09 11:33 ` Jeff Layton 3 siblings, 2 replies; 31+ messages in thread From: Greg KH @ 2014-05-06 19:00 UTC (permalink / raw) To: Andy Lutomirski; +Cc: ksummit-discuss 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 :( ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:00 ` Greg KH @ 2014-05-06 20:07 ` Steven Rostedt 2014-05-06 20:34 ` Josh Triplett 2014-05-07 6:27 ` Michael Kerrisk (man-pages) 1 sibling, 1 reply; 31+ messages in thread From: Steven Rostedt @ 2014-05-06 20:07 UTC (permalink / raw) To: Greg KH; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 20:07 ` Steven Rostedt @ 2014-05-06 20:34 ` Josh Triplett 2014-05-06 20:42 ` Steven Rostedt 2014-05-07 11:48 ` Jiri Kosina 0 siblings, 2 replies; 31+ messages in thread From: Josh Triplett @ 2014-05-06 20:34 UTC (permalink / raw) To: Steven Rostedt; +Cc: ksummit-discuss 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 20:34 ` Josh Triplett @ 2014-05-06 20:42 ` Steven Rostedt 2014-05-06 21:00 ` josh 2014-05-07 11:48 ` Jiri Kosina 1 sibling, 1 reply; 31+ messages in thread From: Steven Rostedt @ 2014-05-06 20:42 UTC (permalink / raw) To: Josh Triplett; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 20:42 ` Steven Rostedt @ 2014-05-06 21:00 ` josh 0 siblings, 0 replies; 31+ messages in thread From: josh @ 2014-05-06 21:00 UTC (permalink / raw) To: Steven Rostedt; +Cc: ksummit-discuss 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 20:34 ` Josh Triplett 2014-05-06 20:42 ` Steven Rostedt @ 2014-05-07 11:48 ` Jiri Kosina 2014-05-08 6:35 ` Li Zefan 1 sibling, 1 reply; 31+ messages in thread From: Jiri Kosina @ 2014-05-07 11:48 UTC (permalink / raw) To: Josh Triplett; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-07 11:48 ` Jiri Kosina @ 2014-05-08 6:35 ` Li Zefan 2014-05-12 6:37 ` Jiri Kosina 0 siblings, 1 reply; 31+ messages in thread From: Li Zefan @ 2014-05-08 6:35 UTC (permalink / raw) To: Jiri Kosina; +Cc: ksummit-discuss 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? ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-08 6:35 ` Li Zefan @ 2014-05-12 6:37 ` Jiri Kosina 0 siblings, 0 replies; 31+ messages in thread From: Jiri Kosina @ 2014-05-12 6:37 UTC (permalink / raw) To: Li Zefan; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:00 ` Greg KH 2014-05-06 20:07 ` Steven Rostedt @ 2014-05-07 6:27 ` Michael Kerrisk (man-pages) 1 sibling, 0 replies; 31+ messages in thread From: Michael Kerrisk (man-pages) @ 2014-05-07 6:27 UTC (permalink / raw) To: Greg KH, Andy Lutomirski; +Cc: ksummit-discuss 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/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 17:45 [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI Andy Lutomirski 2014-05-06 17:58 ` josh 2014-05-06 19:00 ` Greg KH @ 2014-05-06 19:57 ` Dan Carpenter 2014-05-08 18:15 ` Randy Dunlap 2014-05-09 11:33 ` Jeff Layton 3 siblings, 1 reply; 31+ messages in thread From: Dan Carpenter @ 2014-05-06 19:57 UTC (permalink / raw) To: Andy Lutomirski; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 19:57 ` Dan Carpenter @ 2014-05-08 18:15 ` Randy Dunlap 0 siblings, 0 replies; 31+ messages in thread From: Randy Dunlap @ 2014-05-08 18:15 UTC (permalink / raw) To: Dan Carpenter, Andy Lutomirski; +Cc: ksummit-discuss 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-06 17:45 [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI Andy Lutomirski ` (2 preceding siblings ...) 2014-05-06 19:57 ` Dan Carpenter @ 2014-05-09 11:33 ` Jeff Layton 2014-05-09 11:50 ` Michael Kerrisk (man-pages) 3 siblings, 1 reply; 31+ messages in thread From: Jeff Layton @ 2014-05-09 11:33 UTC (permalink / raw) To: Andy Lutomirski; +Cc: ksummit-discuss 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> ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI 2014-05-09 11:33 ` Jeff Layton @ 2014-05-09 11:50 ` Michael Kerrisk (man-pages) 0 siblings, 0 replies; 31+ messages in thread From: Michael Kerrisk (man-pages) @ 2014-05-09 11:50 UTC (permalink / raw) To: Jeff Layton; +Cc: ksummit-discuss 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/ ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2014-05-12 14:15 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-06 17:45 [Ksummit-discuss] [CORE TOPIC] Reviewing new API/ABI Andy Lutomirski 2014-05-06 17:58 ` josh 2014-05-06 19:12 ` Shuah Khan 2014-05-06 19:16 ` Andy Lutomirski 2014-05-06 19:37 ` Shuah Khan 2014-05-06 19:21 ` Johannes Berg 2014-05-06 19:43 ` Andy Lutomirski 2014-05-06 19:48 ` Johannes Berg 2014-05-06 19:51 ` Andy Lutomirski 2014-05-06 19:45 ` josh 2014-05-06 20:10 ` Daniel Vetter 2014-05-06 20:13 ` Andy Lutomirski 2014-05-07 10:12 ` Laurent Pinchart 2014-05-07 12:36 ` Daniel Vetter 2014-05-07 13:30 ` Laurent Pinchart 2014-05-07 13:50 ` Hans Verkuil 2014-05-12 14:15 ` Wolfram Sang 2014-05-07 17:48 ` Michael Kerrisk (man-pages) 2014-05-06 19:00 ` Greg KH 2014-05-06 20:07 ` Steven Rostedt 2014-05-06 20:34 ` Josh Triplett 2014-05-06 20:42 ` Steven Rostedt 2014-05-06 21:00 ` josh 2014-05-07 11:48 ` Jiri Kosina 2014-05-08 6:35 ` Li Zefan 2014-05-12 6:37 ` Jiri Kosina 2014-05-07 6:27 ` Michael Kerrisk (man-pages) 2014-05-06 19:57 ` Dan Carpenter 2014-05-08 18:15 ` Randy Dunlap 2014-05-09 11:33 ` Jeff Layton 2014-05-09 11:50 ` Michael Kerrisk (man-pages)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.