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

[Carsten Otte <cotte@de.ibm.com>]

On 17.11.2011 14:18, Avi Kivity wrote:
>> +	copy_from_guest(vcpu,&pfault_token, vcpu->arch.pfault_token,
>> +			8);
>
> Missing error check?
Good catch, will fix.

>> +	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.

> 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.