Re: [patch 2/2] [PATCH] kvm-s390: pseudo page fault support

[Avi Kivity <avi@redhat.com>]

On 11/17/2011 03:38 PM, Carsten Otte wrote:
>
>>> +    init = kzalloc(sizeof(*init), GFP_ATOMIC);
>>> +    if (!init)
>>> +        return;
>>> +
>>> +    done = kzalloc(sizeof(*done), GFP_ATOMIC);
>>> +    if (!done)
>>> +        goto out_init;
>>> +
>>> +    event = kzalloc(sizeof(*event), GFP_ATOMIC);
>>> +    if (!event)
>>> +        goto out_done;
>>
>> Three allocs?  Maybe combine them?  Even if their lifetimes are not
>> exactly the same.
> The interrupt stack frees the interrupt info types for done and
> init in context of the target vcpu thread when delivering. Thus,
> as far as I can see, these cannot be easily converged.

I mean, if delivery is always the last thing that happens, just do a
single free there.

>> Is this duplicating virt/kvm/async_pf.c?
> We intend to be able to backport this to 2.6.32 for RHEL6. async_pf.c
> relies on asynchonous work items which is not available on RHEL6. Thus
> we decided to go forward with our own implementation first, and move
> towards an aproach similar to async_pf.c as soon as we find time for
> it. I think in the end both pathes can and should be converged. The
> implementation in async_pf.c clearly has advantages over this approach.

Backports to distros, even if you happened to name my favourite, do not
override upstream considerations.  "as soon as we find time" doesn't
inspire confidence either.  If async_pf.c is better, let's do that (btw
async_pf.c did not find itself in generic code by accident).

(for the backport, I imagine it will be possible to backport async_pf.c)

-- 
error compiling committee.c: too many arguments to function