Re: [PATCH 1/2] KVM: Remove unused KVM_REQ_KICK to save a bit in vcpu->requests

[Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>]

On 2016/01/07 21:37, Paolo Bonzini wrote:

> On 07/01/2016 12:43, Takuya Yoshikawa wrote:
>> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
>> ---
>>   include/linux/kvm_host.h | 45 ++++++++++++++++++++++-----------------------
>>   1 file changed, 22 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 61c3e6c..ca9b93e 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -122,29 +122,28 @@ static inline bool is_error_page(struct page *page)
>>   #define KVM_REQ_UNHALT             6
>>   #define KVM_REQ_MMU_SYNC           7
>>   #define KVM_REQ_CLOCK_UPDATE       8
>> -#define KVM_REQ_KICK               9
>
> I'd prefer to leave just an /* unused: 9 */ here.

Since some request handlers can make other requests which need to be
checked in the following path, this young number is valuable. In this
sense, I agree. Your patch looks good to me.

> This patch can go in for 4.5.  Regarding the other patch,
> KVM_REQ_MCLOCK_INPROGRESS is indeed not really necessary, see
> http://www.spinics.net/lists/kvm/msg95944.html and the follow-up.  Were
> you thinking of the same?  If so I would prefer to have some comments.

I did not notice that discussion.

What I have in mind is:
   http://www.spinics.net/lists/kvm/msg80477.html

We have more requests than that time and the overhead of
requests checking has become much worse: Hyper-V code may
still add some more if-branches, which will always be false
for many guests.

To find a solution, I am doing some experiments and trying
to re-implement Avi's for_each_set_bit based solution.
The problem is that for_each_set_bit is too generic and
involves too many function calls, which can actually make
the code slower.

But since vcpu->requests is unsigned long, we can directly
use __ffs as kvm_mmu_write_protect_pt_masked() does. This is
the reason why I object to expanding it to 64-bit size now.

I still need to think carefully about the dependencies between
a few request handlers and am not sure what I am thinking now
will really work.

Anyway, your patches make enough room for now, so I do not need
to remove KVM_REQ_MCLOCK_INPROGRESS dummy request for that. It
depends just on our preference.

   Takuya