Re: [RFC PATCH v6 16/92] kvm: introspection: handle events and event replies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 13 Aug 2019 10:55:21 +0200, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> On 09/08/19 17:59, Adalbert Lazăr wrote:
> > 
> > +			 reply->padding2);
> > +
> > +	ivcpu->reply_waiting = false;
> > +	return expected->error;
> > +}
> > +
> >  /*
> 
> Is this missing a wakeup?
> 
> >  
> > +static bool need_to_wait(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvmi_vcpu *ivcpu = IVCPU(vcpu);
> > +
> > +	return ivcpu->reply_waiting;
> > +}
> > +
> 
> Do you actually need this function?  It seems to me that everywhere you
> call it you already have an ivcpu, so you can just access the field.
> 
> Also, "reply_waiting" means "there is a reply that is waiting".  What
> you mean is "waiting_for_reply".

In an older version, handle_event_reply() was executed from the receiving
thread (having another name) and it contained a wakeup function. Now,
indeed, 'waiting_for_reply' is the right name.
 
> The overall structure of the jobs code is confusing.  The same function
> kvm_run_jobs_and_wait is an infinite loop before and gets a "break"
> later.  It is also not clear why kvmi_job_wait is called through a job.
>  Can you have instead just kvm_run_jobs in KVM_REQ_INTROSPECTION, and
> something like this instead when sending an event:
> 
> int kvmi_wait_for_reply(struct kvm_vcpu *vcpu)
> {
> 	struct kvmi_vcpu *ivcpu = IVCPU(vcpu);
> 
> 	while (ivcpu->waiting_for_reply) {
> 		kvmi_run_jobs(vcpu);
> 
> 		err = swait_event_killable(*wq,
> 				!ivcpu->waiting_for_reply ||
> 				!list_empty(&ivcpu->job_list));
> 
> 		if (err)
> 			return -EINTR;
> 	}
> 
> 	return 0;
> }
> 
> ?
> 
> Paolo

Much better :) Thank you.






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux