Re: + stupid-hack-to-make-mainline-build.patch added to -mm tree

[Jeremy Fitzhardinge]

Thomas Gleixner wrote:
> Still there is a difference between using existing kernel interfaces and
> abusing them in a way which makes modifications to the core kernel code
> hard and unmaintainable. See below.
>   

I completely agree.  "Using the kernel interfaces" doesn't mean "this
random hack happens to work", it means "use the interface as intended as
a fully-fledged client".  If the interface doesn't work for our use,
then we can negotiate with the appropriate people on how to extend it
properly.


> On the other hand we yet see things like:
>
>         /* We use normal irq0 handler on cpu0. */
>         time_init_hook();
>
> Which is just reaching into the kernel code directly and does not handle
> the clock event interrupt self contained. clockevents is not bound to
> IRQ0 and this kind of hackery is exactly what we need to avoid in order
> to get this maintainable.
>   

Yes, I'm definitely not arguing with you about this.  I think the first
cut vmi time code was pretty questionable, but I have confidence they'll
fix it up before submission.

The point is that when you put the xen and vmi implementations next to
each other you find that 1) in each case there's a pretty small
abstraction distance between the clock interface and the hypercall
interface, and 2) there's very little code which can be shared between
the two.  Which means that adding another layer of abstraction to
protect the clock code from paravirtualized time devices is just going
to add fat without much benefit.

> Yes, if they are used in a sane and self contained way without reaching
> all over the place and expecting that those functions, which are not
> part of the paravirt interfaces will work for ever.
>   

100% agree.  If the interfaces change, then we'll change the code using
them like any other kernel code would.  If the new interfaces are hard
to make work then that's a problem, but one would hope that would get
shaken out as part of the normal kernel development process.

The point is that this code under and around the paravirt_ops interface
is just normal Linux code, and we expect to participate in the normal
kernel development process, with all the usual
discussions/arguments/negotiations over interface changes.  If the code
loses all its maintainers and becomes orphaned, unresponsive to
interface changes, then it's like any other dead driver: mark it
CONFIG_BROKEN and wait for someone to fix it.  But for now and the
foreseeable future these are going to be actively supported and
maintained pieces of code.

> You are not increasing the entanglement with the rest of the system,
> when you use a self contained device on top of an existing core kernel
> infrastructure, which has a paravirt backend. Quite the contrary, you
> have one piece of virtual hardware which is connected to the kernel and
> interacts with the various incarnations on the other side, which can as
> well live inside the kernel code. Granted it is another level of
> indirection, but I'd be happy to have only to deal with one of those
> beasts.
>   

Right.  But at that point the interface doesn't really have much of a
technical basis.  It's really a political border at which you can hand
off responsibility and make it ours.  I quite understand your
motivation, but I think you're solving a problem that hasn't happened
yet, and one that we'd all like to avoid.

I know the vmi time code has coloured your view here, but I surely hope
it can be got into a better state before posting.  I'm biased of course,
but I would rather hope that all these drivers we're talking about will
be as stylistically clean as the Xen time code (which has room for
improvement, of course).

There is, however, a median solution which keeps the number of clock
drivers down but also doesn't involve extending pv_ops.  We can just
create paravirt_clocksource/paravirt_clockevent helper wrappers, with
their own internal interfaces to act as a facade for the
hypervisor-specific code.  I don't think there's much point in doing
this now, but maybe it will become appealing once we start dealing with
things like stolen time.

> No it's not an absolute blocker, as long as we can take care, that the
> number of incarnations is 
>
> - designed to be shareable between hypervisors which have the same time
> model
> - common code like the 128 bit math is in a shared library
> - self contained and not reaching out into core kernel code for no good
> reason

Yep.

    J
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/