From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Xen4.2 S3 regression? Date: Thu, 23 Aug 2012 19:54:48 +0100 Message-ID: <50367C78.80608@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------000503060106030301020801" Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ben Guthro Cc: Jan Beulich , Konrad Rzeszutek Wilk , John Baboval , Thomas Goetz , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --------------000503060106030301020801 Content-Type: multipart/alternative; boundary="------------020109070509040601030307" --------------020109070509040601030307 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit > On 23/08/12 19:03, Ben Guthro wrote: >> I did some more bisecting here, and I came up with another changeset >> that seems to be problematic, Re: IRQs >> >> After bisecting the problem discussed earlier in this thread to the >> changeset below, >> http://xenbits.xen.org/hg/xen-unstable.hg/rev/0695a5cdcb42 >> >> >> I worked past that issue by the following hack: >> >> --- a/xen/common/event_channel.c >> +++ b/xen/common/event_channel.c >> @@ -1103,7 +1103,7 @@ void evtchn_destroy_final(struct domain *d) >> void evtchn_move_pirqs(struct vcpu *v) >> { >> struct domain *d = v->domain; >> - const cpumask_t *mask = cpumask_of(v->processor); >> + //const cpumask_t *mask = cpumask_of(v->processor); >> unsigned int port; >> struct evtchn *chn; >> >> @@ -1111,7 +1111,9 @@ void evtchn_move_pirqs(struct vcpu *v) >> for ( port = v->pirq_evtchn_head; port; port = chn->u.pirq.next_port ) >> { >> chn = evtchn_from_port(d, port); >> +#if 0 >> pirq_set_affinity(d, chn->u.pirq.irq, mask); >> +#endif >> } >> spin_unlock(&d->event_lock); >> } >> >> >> This seemed to work for this rather old changeset, but it was not >> sufficient to fix it against the 4.1, or unstable trees. >> >> I further bisected, in combination with this hack, and found the >> following changeset to also be problematic: >> >> http://xenbits.xen.org/hg/xen-unstable.hg/rev/c2cb776a5365 >> >> >> That is, before this change I could resume reliably (with the hack >> above) - and after I could not. >> This was surprising to me, as this change also looks rather innocuous. > And by the looks of that changeset, the logic in fixup_irqs() in irq.c > was changed. > > Jan: The commit message says "simplify operations [in] a few cases". > Was the change in fixup_irqs() deliberate? > > ~Andrew Ben: Could you test the attached patch? It is for unstable and undoes the logical change to fixup_irqs() ~Andrew > >> Naturally, backing out this change seems to be non-trivial against the >> tip, since so much around it has changed. >> > -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 > (0)1223 225 900, http://www.citrix.com > _______________________________________________ Xen-devel mailing list > Xen-devel@lists.xen.org http://lists.xen.org/xen-devel -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com --------------020109070509040601030307 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit 
On 23/08/12 19:03, Ben Guthro wrote:

I did some more bisecting here, and I came up with another changeset
that seems to be problematic, Re: IRQs

After bisecting the problem discussed earlier in this thread to the
changeset below,
http://xenbits.xen.org/hg/xen-unstable.hg/rev/0695a5cdcb42


I worked past that issue by the following hack:

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1103,7 +1103,7 @@ void evtchn_destroy_final(struct domain *d)
 void evtchn_move_pirqs(struct vcpu *v)
 {
     struct domain *d = v->domain;
-    const cpumask_t *mask = cpumask_of(v->processor);
+    //const cpumask_t *mask = cpumask_of(v->processor);
     unsigned int port;
     struct evtchn *chn;

@@ -1111,7 +1111,9 @@ void evtchn_move_pirqs(struct vcpu *v)
     for ( port = v->pirq_evtchn_head; port; port = chn->u.pirq.next_port )
     {
         chn = evtchn_from_port(d, port);
+#if 0
         pirq_set_affinity(d, chn->u.pirq.irq, mask);
+#endif
     }
     spin_unlock(&d->event_lock);
 }


This seemed to work for this rather old changeset, but it was not
sufficient to fix it against the 4.1, or unstable trees.

I further bisected, in combination with this hack, and found the
following changeset to also be problematic:

http://xenbits.xen.org/hg/xen-unstable.hg/rev/c2cb776a5365


That is, before this change I could resume reliably (with the hack
above) - and after I could not.
This was surprising to me, as this change also looks rather innocuous.

And by the looks of that changeset, the logic in fixup_irqs() in irq.c
was changed.

Jan: The commit message says "simplify operations [in] a few cases". 
Was the change in fixup_irqs() deliberate?

~Andrew

Ben: Could you test the attached patch?  It is for unstable and undoes the logical change to fixup_irqs()

~Andrew



Naturally, backing out this change seems to be non-trivial against the
tip, since so much around it has changed.


-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com
--------------020109070509040601030307-- --------------000503060106030301020801 Content-Type: text/x-patch; name="s3-revert-fixup_irqs.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="s3-revert-fixup_irqs.patch" # HG changeset patch # Parent b02ac80ff6899e98b4089842843104fd8572a7cd diff -r b02ac80ff689 xen/arch/x86/irq.c --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -2151,7 +2151,7 @@ void fixup_irqs(void) spin_lock(&desc->lock); cpumask_copy(&affinity, desc->affinity); - if ( !desc->action || cpumask_subset(&affinity, &cpu_online_map) ) + if ( !desc->action || cpumask_equal(&affinity, &cpu_online_map) ) { spin_unlock(&desc->lock); continue; --------------000503060106030301020801 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------000503060106030301020801--