linux-kernel - Re: [PATCH] kvm: add halt_poll

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <54D8DBFE.1070508@redhat.com>
Date:	Mon, 09 Feb 2015 17:10:38 +0100
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Radim Krčmář <rkrcmar@...hat.com>
CC:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org, riel@...hat.com,
	mtosatti@...hat.com, jan.kiszka@...mens.com, dmatlack@...gle.com
Subject: Re: [PATCH] kvm: add halt_poll_ns module parameter



On 09/02/2015 16:21, Radim Krčmář wrote:
> 2015-02-06 13:48+0100, Paolo Bonzini:
> [...]
>> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
>> ---
> 
> Reviewed-by: Radim Krčmář <rkrcmar@...hat.com>
> 
> Noticed changes since RFC:
>  - polling is used in more situations
>  - new tracepoint
>  - module parameter in nanoseconds
>  - properly handled time
>  - no polling with overcommit

Yup, pretty much what came in from Marcelo and David.

>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> @@ -148,6 +148,7 @@ struct kvm_vm_stat {
>>  };
>>  
>>  struct kvm_vcpu_stat {
>> +	u32 halt_successful_poll;
>>  	u32 halt_wakeup;
>>  };
> 
> We don't expose it in arch/arm/kvm/guest.c,
>   struct kvm_stats_debugfs_item debugfs_entries[] = {
>   	{ NULL }
>   };

Yes.  Too late for 3.20.

>> +TRACE_EVENT(kvm_vcpu_wakeup,
>> +	    TP_PROTO(__u64 ns, bool waited),
> 
> (__u64 is preferred here?)

Preferred to what?

>> @@ -1813,29 +1816,60 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
>>  void kvm_vcpu_block(struct kvm_vcpu *vcpu)
>>  {
>> +	ktime_t start, cur;
>>  	DEFINE_WAIT(wait);
>> +	bool waited = false;
>> +
>> +	start = cur = ktime_get();
>> +	if (halt_poll_ns) {
>> +		ktime_t stop = ktime_add_ns(ktime_get(), halt_poll_ns);
>> +		do {
>> +			/*
>> +			 * This sets KVM_REQ_UNHALT if an interrupt
>> +			 * arrives.
>> +			 */
>> +			if (kvm_vcpu_check_block(vcpu) < 0) {
>> +				++vcpu->stat.halt_successful_poll;
>> +				goto out;
>> +			}
>> +			cur = ktime_get();
>> +		} while (single_task_running() && ktime_before(cur, stop));
> 
> After reading a bunch of code, I'm still not sure ...
>  - need_resched() can't be true when single_task_running()?
>    (I think it could happen -- balancing comes into mind.)

Single_task_running is per-CPU; for a task to relinquish control to
another task, you first need to have multiple tasks running.  In other
words, I think it cannot.

>  - is it ok to ignore need_resched() when single_task_running()?
>    (Most likely not.)

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