On 10.08.20 22:00, Julien Grall wrote:
Hi Julien
@@ -2275,6 +2282,16 @@ static void check_for_vcpu_work(void)
*/
void leave_hypervisor_to_guest(void)
{
+#ifdef CONFIG_IOREQ_SERVER
+ /*
+ * XXX: Check the return. Shall we call that in
+ * continue_running and context_switch instead?
+ * The benefits would be to avoid calling
+ * handle_hvm_io_completion on every return.
+ */
Yeah, that could be a simple and good optimization
Well, it is not simple as it is sounds :).
handle_hvm_io_completion() is the function in charge to mark the
vCPU as waiting for I/O. So we would at least need to split the
function.
I wrote this TODO because I wasn't sure about the complexity of
handle_hvm_io_completion(current). Looking at it again, the main
complexity is the looping over the IOREQ servers.
I think it would be better to optimize handle_hvm_io_completion()
rather than trying to hack the context_switch() or continue_running().
Well, is the idea in proposed dirty test patch below close to what
you expect? Patch optimizes handle_hvm_io_completion() to avoid extra
actions if vcpu's domain doesn't have ioreq_server, alternatively
the check could be moved out of handle_hvm_io_completion() to avoid
calling that function at all.
This looks ok to me.
BTW, TODO also suggests checking the return value of
handle_hvm_io_completion(), but I am completely sure we can simply
just return from leave_hypervisor_to_guest() at this point. Could you
please share your opinion?
From my understanding, handle_hvm_io_completion() may return false if
there is pending I/O or a failure.
It seems, yes
In the former case, I think we want to call handle_hvm_io_completion()
later on. Possibly after we call do_softirq().
I am wondering whether check_for_vcpu_work() could return whether
there are more work todo on the behalf of the vCPU.
So we could have:
do
{
check_for_pcpu_work();
} while (check_for_vcpu_work())
The implementation of check_for_vcpu_work() would be:
if ( !handle_hvm_io_completion() )
return true;
/* Rest of the existing code */
return false;
Thank you, will give it a try.
Can we behave the same way for both "pending I/O" and "failure" or we
need to distinguish them?
Probably we need some sort of safe timeout/number attempts in order to
not spin forever?
--
Regards,
Oleksandr Tyshchenko
