| Message ID | 1474991845-27962-5-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 27.09.16 at 17:56, <roger.pau@citrix.com> wrote: > --- a/xen/arch/x86/mm/hap/hap.c > +++ b/xen/arch/x86/mm/hap/hap.c > @@ -379,7 +379,9 @@ hap_set_allocation(struct domain *d, unsigned long pages, int *preempted) > break; > > /* Check to see if we need to yield and try again */ > - if ( preempted && hypercall_preempt_check() ) > + if ( preempted && > + (is_idle_vcpu(current) ? softirq_pending(smp_processor_id()) : > + hypercall_preempt_check()) ) So what is the supposed action for the caller to take in this case? I think this should at least be spelled out in the commit message. Jan
On Thu, Sep 29, 2016 at 04:43:07AM -0600, Jan Beulich wrote: > >>> On 27.09.16 at 17:56, <roger.pau@citrix.com> wrote: > > --- a/xen/arch/x86/mm/hap/hap.c > > +++ b/xen/arch/x86/mm/hap/hap.c > > @@ -379,7 +379,9 @@ hap_set_allocation(struct domain *d, unsigned long pages, int *preempted) > > break; > > > > /* Check to see if we need to yield and try again */ > > - if ( preempted && hypercall_preempt_check() ) > > + if ( preempted && > > + (is_idle_vcpu(current) ? softirq_pending(smp_processor_id()) : > > + hypercall_preempt_check()) ) > > So what is the supposed action for the caller to take in this case? > I think this should at least be spelled out in the commit message. I'm not sure I follow, but I assume you mean in the idle vcpu case? In that case the caller should call process_pending_softirqs. I will modify the commit message to include it. Roger.
>>> On 29.09.16 at 16:37, <roger.pau@citrix.com> wrote: > On Thu, Sep 29, 2016 at 04:43:07AM -0600, Jan Beulich wrote: >> >>> On 27.09.16 at 17:56, <roger.pau@citrix.com> wrote: >> > --- a/xen/arch/x86/mm/hap/hap.c >> > +++ b/xen/arch/x86/mm/hap/hap.c >> > @@ -379,7 +379,9 @@ hap_set_allocation(struct domain *d, unsigned long > pages, int *preempted) >> > break; >> > >> > /* Check to see if we need to yield and try again */ >> > - if ( preempted && hypercall_preempt_check() ) >> > + if ( preempted && >> > + (is_idle_vcpu(current) ? softirq_pending(smp_processor_id()) > : >> > + hypercall_preempt_check()) ) >> >> So what is the supposed action for the caller to take in this case? >> I think this should at least be spelled out in the commit message. > > I'm not sure I follow, but I assume you mean in the idle vcpu case? Yes. Right now it is expected to schedule a hypercall continuation. > In that case the caller should call process_pending_softirqs. I will modify > the commit message to include it. Thanks. Jan
On 27/09/16 16:56, Roger Pau Monne wrote: > ... and using the "preempted" parameter. The solution relies on just calling > softirq_pending if the current domain is the idle domain. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> You probably also want to add something to the effect of: "This allows us to call *_set_allocation() when building domain 0." Someone doing archeology doesn't want to dig out this series to figure out how it would be possible to call hap_set_allocation() while idle. :-) -George
On 30/09/16 17:56, George Dunlap wrote: > On 27/09/16 16:56, Roger Pau Monne wrote: >> ... and using the "preempted" parameter. The solution relies on just calling >> softirq_pending if the current domain is the idle domain. >> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > You probably also want to add something to the effect of: > > "This allows us to call *_set_allocation() when building domain 0." > > Someone doing archeology doesn't want to dig out this series to figure > out how it would be possible to call hap_set_allocation() while idle. :-) Oh, but when the comments have been addressed, you can add: Acked-by: George Dunlap <george.dunlap@citrix.com>
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c index b6d2c61..2dc82f5 100644 --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -379,7 +379,9 @@ hap_set_allocation(struct domain *d, unsigned long pages, int *preempted) break; /* Check to see if we need to yield and try again */ - if ( preempted && hypercall_preempt_check() ) + if ( preempted && + (is_idle_vcpu(current) ? softirq_pending(smp_processor_id()) : + hypercall_preempt_check()) ) { *preempted = 1; return 0;
... and using the "preempted" parameter. The solution relies on just calling softirq_pending if the current domain is the idle domain. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/mm/hap/hap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)