[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Cleaning up kernel's headers for userspace



On Mon, 2006-04-24 at 17:48 +0000, Tim Yamin wrote:
> So, I might be misunderstanding but what that does right now is just
> grok the headers and strips out anything inside __KERNEL__. Yeah, that
> works. But I think it's a bit redundant. If it's inside #ifdef
> __KERNEL__ it shouldn't be a problem anyway because thanks to the wonders
> of a pre-processor your user-app just never even sees it

That's true, but the idea is that the 'make headers_install' output
should be easy to read and process _without_ all that extra noise in it.

You should be able to do a diff between the output from one kernel
version and the next, and see only the real user-visible changes. We
should be able to read through the exported files and _see_ the stuff
which isn't guarded with __KERNEL__ but should be, and the namespace
pollution. It should jump out at us and make the kernel janitors itch to
clean it up.

At the moment, anyone can make the headers worse for userspace without
it even being noticeable. With 'make headers_check' such offences would
be noticed at source.

>  and IMO one should keep things in the headers for completeness
> (problem solving is easier that way).

I'm not sure I agree. Keeping stuff like asm/atomic.h might be
'complete' but it's broken if people actually try to _use_ it. And you
get some muppets actually defining __KERNEL__ just to get at stuff they
shouldn't.

> The reason we go with this approach is transparency; rather than
> stripping things out the "Gentoo way" of doing them is to play with the
> jigsaw pieces. Some things need to be __KERNEL__ that aren't. For
> others it's the opposite. Here and there we do namespace collision
> fixes some of which happen via automated sed-fu (e.g. u8 -> __u8).

Those fixes need to go upstream, surely?. Are you _opposed_ to us
striving for a situation where you can just ship what the kernel spits
out?

> The other problem is the vastness of our architecture support - we
> do from x86 to m68k and all those headers need to work -- hence
> hand-picking really never was seen as the viable option as it means
> a *lot* of work between upstream releases. Our current way you just
> fix a few conflicts with the patches have when you move from say 2.6.16
> to 2.6.17, and run test-cases to see if any new gremlins have
> appeared.

The idea is that this all happens upstream; no such merges would be
required. Once we've got it sorted out, the architecture maintainers are
responsible for the output of 'make headers_install' on their
architecture, and 'make headers_check' exists for automated checking of
the results. If you have more checks which we should apply during the
'make headers_check' stage, that would be useful.

> Personally I think the approach is problematic since you'll end up
> with the same problem as that with linux-libc-headers: basically, you
> get a header tree and the diff between that and "raw" tree is pretty huge
> and it needs a lot of work to maintain (for example, if the architecture
> maintainers decide to add asm-foo/blah.h which requires asm-foo/baz.h you
> now need to include both and make sure things play nice...)

I don't think I've communicated the idea correctly then, since this is
the _opposite_ of where I'm going. No changes would be made except in
the upstream kernel tree. We'd just use the output of the 'make
headers_install' step. Never patch the exported files -- only the source
files.

We want everything to be upstream, and the results of 'make
headers_install' from the kernel to be shippable as-is. If the arch
maintainer adds asm-foo/blah.h which requires asm-foo/baz.h, then they
add both of them to asm-foo/Kbuild and all is well. Or if they forget,
and just start including asm-foo/baz.h from the existing asm/blah.h
without touching Kbuild, then the automatic checks mean they get a
kicking for it fairly quickly thereafter.

Yes, I agree that anything which involves _external_ patches and
scripting is going to be problematic. It's got to be upstream, and we've
got to expect individual developers who are working in ioctls, sockopts,
etc. to actually pay some attention to the resulting user-visible
headers. And that's why I wanted 'headers_install' and 'headers_check'
-- it's just a tool to make the existing mess more obvious and help us
to identify and fix the problems.

Ideally, I'd like to reach a point where 'headers_install' is nothing
more than a recursive copy, with no picking of files or unifdef. That's
a way off yet though.

> Just exporting all the headers raw but properly fixing the #ifdef
> __KERNEL__ logic is the easiest solution in my opinion, after all, that's
> what the whole __KERNEL__ system is supposed to be there for ;) -- and
> when things do break one can just fix that specific problem.

That hasn't happened, though. And a large part of that is because the
problems aren't _obvious_. Hence the tools which make the problems
obvious by removing the __KERNEL__ and the unneeded files.

> > Note that I'd rather not have a discussion about moving user-visible
> > headers into a separate directory right now. As long as we mark them for
> > export as we do at the moment, that's the interesting part. Linus has
> > already vetoed a suggestion that we move the public headers into a kabi/
> > directory, and I'd like to go back to him with that suggestion _after_
> > we've cleaned it all up and hopefully abolished the use of '#ifdef
> > __KERNEL__', in the hope that he'll change his mind. We'll have a
> > clearer idea of how we might want to organise things under kabi/ once
> > we've cleaned it all up and we know what we have left, anyway.
> 
> I agree with Linus' decision here -- having two sets of headers is a real
> pain to maintain. You've got to add things to two files and it means double
> the work. The undef solution fixes this somewhat but you still end up with
> the same end-result: a filtered tree which is quite different from the "raw"
> tree.

Again I think there's some confusion. You wouldn't have two sets of
headers. You'd have _shared_ structures and definitions in one directory
which are suitable for userspace, and you've have _private_ structures
and definitions in another directory. You'd include the former from the
latter.

Imagine taking a bunch of messy header files, finding the public parts
of them and splitting them out into separate files -- so that we have
none of those horrid ifdefs that everyone seems to hate in C code, while
conveniently overlooking them in headers. Then move the public headers
into a separate directory instead of leaving them mixed in with the
private headers.

That describes what I did with include/linux/mtd -> include/mtd a while
back. And it's a _lot_ nicer.

> Look back at 2.4 -- everything worked with the raw set of headers because
> maintainers took an active interest in fixing headers for userspace with
> one easy-to-maintain set. What I think that needs doing for 2.6 is starting
> to go through the all the crap that's built up over the months and making
> the "#ifdef __KERNEL__"'s do their job properly.

Yes, I agree. And that's what 'headers_install' is designed to provoke.
It's designed to let us _see_ the result with the __KERNEL__ taken out,
and designed to help us take diffs from one version to the next. And to
let Andrew reject patches which introduce regressions. Etc. 

It's just a tool to help us to make (and retain) the improvements that
we all agree that we need. It's not a substitute for those improvements.

> Anyway, my $0.02 -- if I sound like the grumpy gnome then just ignore me
> but this solution wouldn't really float our boat as things stand :)

Thanks for your feedback. Hopefully, I've managed to persuade you that
this is just a tool which helps us to get to the same place you seem to
want to get to -- the upstream header files being cleaned up; with or
without __KERNEL__. If not, it would be interesting to hear if you have
a better solution to propose. I'm not particularly wedded to this
approach -- it just seemed like a good way to get people involved and to
hold people accountable for user-visible stuff in the future. If you've
got better ideas, I'm all ears...

-- 
dwmw2



Reply to: