From mboxrd@z Thu Jan  1 00:00:00 1970
From: Carsten Otte <cotte@de.ibm.com>
Subject: Re: [patch 2/2] [PATCH] kvm-s390: pseudo page fault support
Date: Thu, 17 Nov 2011 14:38:39 +0100
Message-ID: <4EC50E5F.6040202@de.ibm.com>
References: <20111117111954.130341385@de.ibm.com> <20111117112248.205248622@de.ibm.com> <4EC5098B.70101@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tossati <mtosatti@redhat.com>, borntrae@linux.vnet.ibm.com,
	heicars2@linux.vnet.ibm.com, mschwid2@linux.vnet.ibm.com,
	huckc@linux.vnet.ibm.com, KVM <kvm@vger.kernel.org>
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mtagate7.uk.ibm.com ([194.196.100.167]:48788 "EHLO
	mtagate7.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754339Ab1KQNjE (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 17 Nov 2011 08:39:04 -0500
Received: from d06nrmr1806.portsmouth.uk.ibm.com (d06nrmr1806.portsmouth.uk.ibm.com [9.149.39.193])
	by mtagate7.uk.ibm.com (8.13.1/8.13.1) with ESMTP id pAHDd24r005112
	for <kvm@vger.kernel.org>; Thu, 17 Nov 2011 13:39:02 GMT
Received: from d06av06.portsmouth.uk.ibm.com (d06av06.portsmouth.uk.ibm.com [9.149.37.217])
	by d06nrmr1806.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pAHDd21B2080892
	for <kvm@vger.kernel.org>; Thu, 17 Nov 2011 13:39:02 GMT
Received: from d06av06.portsmouth.uk.ibm.com (loopback [127.0.0.1])
	by d06av06.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pAHKd2H4009944
	for <kvm@vger.kernel.org>; Thu, 17 Nov 2011 13:39:02 -0700
In-Reply-To: <4EC5098B.70101@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.