thr3ads.net - Xen devel - [Xen-devel] unfair servicing of DomU vbd requests [Mar 2011]

If this information is useful, please help other people find it:
Share via:

James Harper

2011-Mar-03 02:25 UTC

[Xen-devel] unfair servicing of DomU vbd requests

A user of GPLPV (see thread "blue screen in windows balloon driver")
is
getting a bug check in Windows under extremely high memory usage and
swapfile thrashing tests across multiple DomU''s. Responses to my query
on the ntdev mailing list say that this would happen if an IO request is
not completed after 70 seconds during high memory/pagefile pressure,
which is what is happening.

It appears that Dom0 is not servicing vbd requests from DomU''s fairly
so
one or two end up getting stalled while the others are mostly okay. How
are vbd requests supposed to be serviced? Is there potential for one to
be overlooked for a long period of time? Is there some settings that
could be changed to avoid this happening?

Thanks

James

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

Keir Fraser

2011-Mar-03 07:29 UTC

head link

Re: [Xen-devel] unfair servicing of DomU vbd requests

On 03/03/2011 02:25, "James Harper"
<james.harper@bendigoit.com.au> wrote:
> A user of GPLPV (see thread "blue screen in windows balloon
driver") is
> getting a bug check in Windows under extremely high memory usage and
> swapfile thrashing tests across multiple DomU''s. Responses to my
query
> on the ntdev mailing list say that this would happen if an IO request is
> not completed after 70 seconds during high memory/pagefile pressure,
> which is what is happening.
> 
> It appears that Dom0 is not servicing vbd requests from DomU''s
fairly so
> one or two end up getting stalled while the others are mostly okay. How
> are vbd requests supposed to be serviced? Is there potential for one to
> be overlooked for a long period of time? Is there some settings that
> could be changed to avoid this happening?
Dom0 does round-robin scanning of pending event channels these days, which
helps fairness a fair bit. When a pending event is found, the corresponding
blkfront has a batch of requests pulled down and submitted into Linux''s
block subsystem at which point we have no more control over scheduling (it
could generally be configured though -- Linux has an admin mechanism for
that).

 -- Keir
> Thanks
> 
> James
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


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

Ian Campbell

2011-Mar-03 08:22 UTC

head link

Re: [Xen-devel] unfair servicing of DomU vbd requests

On Thu, 2011-03-03 at 07:29 +0000, Keir Fraser wrote:> On 03/03/2011 02:25, "James Harper"
<james.harper@bendigoit.com.au> wrote:
> 
> > A user of GPLPV (see thread "blue screen in windows balloon
driver") is
> > getting a bug check in Windows under extremely high memory usage and
> > swapfile thrashing tests across multiple DomU''s. Responses to
my query
> > on the ntdev mailing list say that this would happen if an IO request
is
> > not completed after 70 seconds during high memory/pagefile pressure,
> > which is what is happening.
> > 
> > It appears that Dom0 is not servicing vbd requests from
DomU''s fairly so
> > one or two end up getting stalled while the others are mostly okay.
How
> > are vbd requests supposed to be serviced? Is there potential for one
to
> > be overlooked for a long period of time? Is there some settings that
> > could be changed to avoid this happening?
> 
> Dom0 does round-robin scanning of pending event channels these days, which
> helps fairness a fair bit.
I have a feeling this isn''t true of pvops kernels...

looks like we need to pull 324:7fe1c6d02a2b (and subsequent fixes) out
of 2.6.18-xen.hg into the pvops world.

I''ll take a look shortly if no one beats me to it.
>  When a pending event is found, the corresponding
> blkfront has a batch of requests pulled down and submitted into
Linux''s
> block subsystem at which point we have no more control over scheduling (it
> could generally be configured though -- Linux has an admin mechanism for
> that).
> 
>  -- Keir
> 
> > Thanks
> > 
> > James
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


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

James Harper

2011-Mar-03 08:28 UTC

head link

RE: [Xen-devel] unfair servicing of DomU vbd requests

> > > It appears that Dom0 is not servicing vbd requests from
DomU's fairly so
> > > one or two end up getting stalled while the others are mostly
okay. How
> > > are vbd requests supposed to be serviced? Is there potential for
one to
> > > be overlooked for a long period of time? Is there some settings
that
> > > could be changed to avoid this happening?
> >
> > Dom0 does round-robin scanning of pending event channels these days,
which
> > helps fairness a fair bit.
> 
> I have a feeling this isn't true of pvops kernels...
> 
> looks like we need to pull 324:7fe1c6d02a2b (and subsequent fixes) out
> of 2.6.18-xen.hg into the pvops world.
> 
I hope that's true and easy to fix. It would certainly explain why one DomU
can starve enough to the point where it's IO doesn't get serviced for
>70 seconds.

Thanks for looking at this.

James

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

Keir Fraser

2011-Mar-03 08:30 UTC

head link

Re: [Xen-devel] unfair servicing of DomU vbd requests

On 03/03/2011 08:28, "James Harper"
<james.harper@bendigoit.com.au> wrote:
>>>> It appears that Dom0 is not servicing vbd requests from
DomU''s fairly so
>>>> one or two end up getting stalled while the others are mostly
okay. How
>>>> are vbd requests supposed to be serviced? Is there potential
for one to
>>>> be overlooked for a long period of time? Is there some settings
that
>>>> could be changed to avoid this happening?
>>> 
>>> Dom0 does round-robin scanning of pending event channels these
days, which
>>> helps fairness a fair bit.
>> 
>> I have a feeling this isn''t true of pvops kernels...
>> 
>> looks like we need to pull 324:7fe1c6d02a2b (and subsequent fixes) out
>> of 2.6.18-xen.hg into the pvops world.
>> 
> 
> I hope that''s true and easy to fix. It would certainly explain why
one DomU
> can starve enough to the point where it''s IO doesn''t get
serviced for >70
> seconds.
Without the round-robin servicing, unfairness to the point of starvation is
a distinct possibility.

 -- Keir
> Thanks for looking at this.
> 
> James


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

Ian Campbell

2011-Mar-03 17:09 UTC

head link

[GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

On Thu, 2011-03-03 at 08:30 +0000, Keir Fraser wrote: > Without the round-robin servicing, unfairness to the point of starvation is
> a distinct possibility.
Indeed. The following is a forward port of these patches from
2.6.18-xen.hg: 
      * 324:7fe1c6d02a2b 
      * 325:b2768401db94 
      * 988:c88a02a22a05 
      * 990:427276ac595d 
      * 991:9ba6d9f3fbc0

Hopefully I didn''t miss any.

The switch in pvops from "l1"/"l2" naming convention to
"word"/"bit"
made the conflict resolution a bit prone to thinkos but hopefully I
didn''t b0rk it too badly. Review with that in mind greatly appreciated.

boots a dom0 + a pv guest.

Pull request on top of konrad/stable/irq.cleanup:

The following changes since commit c5ae07bb307b658c8458f29ca77d237aec0f9327:
  Ian Campbell (1):
        xen: events: remove dom0 specific xen_create_msi_irq

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/linux-2.6.git irq-fairness

Ian Campbell (1):
      xen: events: Make last processed event channel a per-cpu variable.

Keir Fraser (3):
      xen: events: Clean up round-robin evtchn scan.
      xen: events: Make round-robin scan fairer by snapshotting each l2 word
      xen: events: Remove redundant clear of l2i at end of round-robin loop

Scott Rixner (1):
      xen: events: Process event channels notifications in round-robin order.

 drivers/xen/events.c |   80 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 72 insertions(+), 8 deletions(-)



Ian.





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

Ian Campbell

2011-Mar-03 17:10 UTC

head link

[Xen-devel] [PATCH 1/5] xen: events: Process event channels notifications in round-robin order.

From: Scott Rixner <rixner@rice.edu>

Avoids fairness issue resulting from domain 0 processing lowest
numbered event channel first.

Bugzilla #1115 "Event channel port scanning unfair".

Signed-off-by: Ian Campbell <ian.campbell@xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
[ijc: forward ported from linux-2.6.18-xen.hg 324:7fe1c6d02a2b
 various variables have different names in this tree:
  l1  -> pending_words
  l2  -> pending_bits
  l1i -> word_idx
  l2i -> bit_idx
]
---
 drivers/xen/events.c |   72 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 6befe62..75cc6f5 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1028,6 +1028,11 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
 static DEFINE_PER_CPU(unsigned, xed_nesting_count);
 
 /*
+ * Mask out the i least significant bits of w
+ */
+#define MASK_LSBS(w, i) (w & ((~0UL) << i))
+
+/*
  * Search the CPUs pending events bitmasks.  For each one found, map
  * the event number to an irq, and feed it into do_IRQ() for
  * handling.
@@ -1038,6 +1043,9 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
  */
 static void __xen_evtchn_do_upcall(void)
 {
+	static unsigned int last_word_idx = BITS_PER_LONG - 1;
+	static unsigned int last_bit_idx = BITS_PER_LONG - 1;
+	int word_idx, bit_idx;
 	int cpu = get_cpu();
 	struct shared_info *s = HYPERVISOR_shared_info;
 	struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
@@ -1056,17 +1064,50 @@ static void __xen_evtchn_do_upcall(void)
 		wmb();
 #endif
 		pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
+
+		word_idx = last_word_idx;
+		bit_idx = last_bit_idx;
+
 		while (pending_words != 0) {
 			unsigned long pending_bits;
-			int word_idx = __ffs(pending_words);
-			pending_words &= ~(1UL << word_idx);
+			unsigned long words;
+
+			word_idx = (word_idx + 1) % BITS_PER_LONG;
+			words = MASK_LSBS(pending_words, word_idx);
+
+			/*
+			 * If we masked out all events, wrap around to the
+			 * beginning.
+			 */
+			if (words == 0) {
+				word_idx = BITS_PER_LONG - 1;
+				bit_idx = BITS_PER_LONG - 1;
+				continue;
+			}
+			word_idx = __ffs(words);
 
-			while ((pending_bits = active_evtchns(cpu, s, word_idx)) != 0) {
-				int bit_idx = __ffs(pending_bits);
-				int port = (word_idx * BITS_PER_LONG) + bit_idx;
-				int irq = evtchn_to_irq[port];
+			do {
+				unsigned long bits;
+				int port, irq;
 				struct irq_desc *desc;
 
+				pending_bits = active_evtchns(cpu, s, word_idx);
+
+				bit_idx = (bit_idx + 1) % BITS_PER_LONG;
+				bits = MASK_LSBS(pending_bits, bit_idx);
+
+				/* If we masked out all events, move on. */
+				if (bits == 0) {
+					bit_idx = BITS_PER_LONG - 1;
+					break;
+				}
+
+				bit_idx = __ffs(bits);
+
+				/* Process port. */
+				port = (word_idx * BITS_PER_LONG) + bit_idx;
+				irq = evtchn_to_irq[port];
+
 				mask_evtchn(port);
 				clear_evtchn(port);
 
@@ -1075,7 +1116,24 @@ static void __xen_evtchn_do_upcall(void)
 					if (desc)
 						generic_handle_irq_desc(irq, desc);
 				}
-			}
+
+				/*
+				 * If this is the final port processed, we''ll
+				 * pick up here+1 next time.
+				 */
+				last_word_idx = word_idx;
+				last_bit_idx = bit_idx;
+
+			} while (bit_idx != BITS_PER_LONG - 1);
+
+			pending_bits = active_evtchns(cpu, s, word_idx);
+
+			/*
+			 * We handled all ports, so we can clear the
+			 * selector bit.
+			 */
+			if (pending_bits == 0)
+				pending_words &= ~(1UL << word_idx);
 		}
 
 		BUG_ON(!irqs_disabled());
-- 
1.5.6.5


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

Ian Campbell

2011-Mar-03 17:10 UTC

head link

[Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

Signed-off-by: Ian Campbell <ian.campbell@xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
[ijc: forward ported from linux-2.6.18-xen.hg 325:b2768401db94]
---
 drivers/xen/events.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 75cc6f5..1fc3192 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1026,6 +1026,8 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
 }
 
 static DEFINE_PER_CPU(unsigned, xed_nesting_count);
+static DEFINE_PER_CPU(unsigned int, last_word_idx) = { BITS_PER_LONG - 1 };
+static DEFINE_PER_CPU(unsigned int, last_bit_idx) = { BITS_PER_LONG - 1 };
 
 /*
  * Mask out the i least significant bits of w
@@ -1043,8 +1045,6 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
  */
 static void __xen_evtchn_do_upcall(void)
 {
-	static unsigned int last_word_idx = BITS_PER_LONG - 1;
-	static unsigned int last_bit_idx = BITS_PER_LONG - 1;
 	int word_idx, bit_idx;
 	int cpu = get_cpu();
 	struct shared_info *s = HYPERVISOR_shared_info;
@@ -1065,8 +1065,8 @@ static void __xen_evtchn_do_upcall(void)
 #endif
 		pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
 
-		word_idx = last_word_idx;
-		bit_idx = last_bit_idx;
+		word_idx = __this_cpu_read(last_word_idx);
+		bit_idx = __this_cpu_read(last_bit_idx);
 
 		while (pending_words != 0) {
 			unsigned long pending_bits;
@@ -1121,9 +1121,8 @@ static void __xen_evtchn_do_upcall(void)
 				 * If this is the final port processed, we''ll
 				 * pick up here+1 next time.
 				 */
-				last_word_idx = word_idx;
-				last_bit_idx = bit_idx;
-
+				__this_cpu_write(last_word_idx, word_idx);
+				__this_cpu_write(last_bit_idx, bit_idx);
 			} while (bit_idx != BITS_PER_LONG - 1);
 
 			pending_bits = active_evtchns(cpu, s, word_idx);
-- 
1.5.6.5


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

Ian Campbell

2011-Mar-03 17:10 UTC

head link

[Xen-devel] [PATCH 3/5] xen: events: Clean up round-robin evtchn scan.

From: Keir Fraser <keir.fraser@citrix.com>

Also fixes a couple of boundary cases.

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
[ijc: forward ported from linux-2.6.18-xen.hg 988:c88a02a22a05]
---
 drivers/xen/events.c |   44 ++++++++++++++++++++------------------------
 1 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 1fc3192..c49cb6d 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1026,8 +1026,8 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
 }
 
 static DEFINE_PER_CPU(unsigned, xed_nesting_count);
-static DEFINE_PER_CPU(unsigned int, last_word_idx) = { BITS_PER_LONG - 1 };
-static DEFINE_PER_CPU(unsigned int, last_bit_idx) = { BITS_PER_LONG - 1 };
+static DEFINE_PER_CPU(unsigned int, current_word_idx);
+static DEFINE_PER_CPU(unsigned int, current_bit_idx);
 
 /*
  * Mask out the i least significant bits of w
@@ -1065,23 +1065,21 @@ static void __xen_evtchn_do_upcall(void)
 #endif
 		pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
 
-		word_idx = __this_cpu_read(last_word_idx);
-		bit_idx = __this_cpu_read(last_bit_idx);
+		word_idx = __this_cpu_read(current_word_idx);
+		bit_idx = __this_cpu_read(current_bit_idx);
 
 		while (pending_words != 0) {
 			unsigned long pending_bits;
 			unsigned long words;
 
-			word_idx = (word_idx + 1) % BITS_PER_LONG;
 			words = MASK_LSBS(pending_words, word_idx);
 
 			/*
-			 * If we masked out all events, wrap around to the
-			 * beginning.
+			 * If we masked out all events, wrap to beginning.
 			 */
 			if (words == 0) {
-				word_idx = BITS_PER_LONG - 1;
-				bit_idx = BITS_PER_LONG - 1;
+				word_idx = 0;
+				bit_idx = 0;
 				continue;
 			}
 			word_idx = __ffs(words);
@@ -1093,14 +1091,11 @@ static void __xen_evtchn_do_upcall(void)
 
 				pending_bits = active_evtchns(cpu, s, word_idx);
 
-				bit_idx = (bit_idx + 1) % BITS_PER_LONG;
 				bits = MASK_LSBS(pending_bits, bit_idx);
 
 				/* If we masked out all events, move on. */
-				if (bits == 0) {
-					bit_idx = BITS_PER_LONG - 1;
+				if (bits == 0)
 					break;
-				}
 
 				bit_idx = __ffs(bits);
 
@@ -1117,22 +1112,23 @@ static void __xen_evtchn_do_upcall(void)
 						generic_handle_irq_desc(irq, desc);
 				}
 
-				/*
-				 * If this is the final port processed, we''ll
-				 * pick up here+1 next time.
-				 */
-				__this_cpu_write(last_word_idx, word_idx);
-				__this_cpu_write(last_bit_idx, bit_idx);
-			} while (bit_idx != BITS_PER_LONG - 1);
+				bit_idx = (bit_idx + 1) % BITS_PER_LONG;
+
+				/* Next caller starts at last processed + 1 */
+				__this_cpu_write(current_word_idx,
+						 bit_idx ? word_idx :
+						 (word_idx+1) % BITS_PER_LONG);
+				__this_cpu_write(current_bit_idx, bit_idx);
+			} while (bit_idx != 0);
 
 			pending_bits = active_evtchns(cpu, s, word_idx);
 
-			/*
-			 * We handled all ports, so we can clear the
-			 * selector bit.
-			 */
+			/* If we handled all ports, clear the selector bit. */
 			if (pending_bits == 0)
 				pending_words &= ~(1UL << word_idx);
+
+			word_idx = (word_idx + 1) % BITS_PER_LONG;
+			bit_idx = 0;
 		}
 
 		BUG_ON(!irqs_disabled());
-- 
1.5.6.5


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

Ian Campbell

2011-Mar-03 17:10 UTC

head link

[Xen-devel] [PATCH 4/5] xen: events: Make round-robin scan fairer by snapshotting each l2 word

From: Keir Fraser <keir.fraser@citrix.com>

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
[ijc: forward ported from linux-2.6.18-xen.hg 990:427276ac595d]
---
 drivers/xen/events.c |   30 +++++++++++++++++++++---------
 1 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index c49cb6d..0c3f17b 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1045,7 +1045,9 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
  */
 static void __xen_evtchn_do_upcall(void)
 {
+	int start_word_idx, start_bit_idx;
 	int word_idx, bit_idx;
+	int i;
 	int cpu = get_cpu();
 	struct shared_info *s = HYPERVISOR_shared_info;
 	struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
@@ -1065,10 +1067,12 @@ static void __xen_evtchn_do_upcall(void)
 #endif
 		pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
 
-		word_idx = __this_cpu_read(current_word_idx);
-		bit_idx = __this_cpu_read(current_bit_idx);
+		start_word_idx = __this_cpu_read(current_word_idx);
+		start_bit_idx = __this_cpu_read(current_bit_idx);
+
+		word_idx = start_word_idx;
 
-		while (pending_words != 0) {
+		for (i = 0; pending_words != 0; i++) {
 			unsigned long pending_bits;
 			unsigned long words;
 
@@ -1084,13 +1088,23 @@ static void __xen_evtchn_do_upcall(void)
 			}
 			word_idx = __ffs(words);
 
+			pending_bits = active_evtchns(cpu, s, word_idx);
+			bit_idx = 0; /* usually scan entire word from start */
+			if (word_idx == start_word_idx) {
+				/* We scan the starting word in two parts */
+				if (i == 0)
+					/* 1st time: start in the middle */
+					bit_idx = start_bit_idx;
+				else
+					/* 2nd time: mask bits done already */
+					bit_idx &= (1UL << start_bit_idx) - 1;
+			}
+
 			do {
 				unsigned long bits;
 				int port, irq;
 				struct irq_desc *desc;
 
-				pending_bits = active_evtchns(cpu, s, word_idx);
-
 				bits = MASK_LSBS(pending_bits, bit_idx);
 
 				/* If we masked out all events, move on. */
@@ -1121,10 +1135,8 @@ static void __xen_evtchn_do_upcall(void)
 				__this_cpu_write(current_bit_idx, bit_idx);
 			} while (bit_idx != 0);
 
-			pending_bits = active_evtchns(cpu, s, word_idx);
-
-			/* If we handled all ports, clear the selector bit. */
-			if (pending_bits == 0)
+			/* Scan start_l1i twice; all others once. */
+			if ((word_idx != start_word_idx) || (i != 0))
 				pending_words &= ~(1UL << word_idx);
 
 			word_idx = (word_idx + 1) % BITS_PER_LONG;
-- 
1.5.6.5


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

Ian Campbell

2011-Mar-03 17:10 UTC

head link

[Xen-devel] [PATCH 5/5] xen: events: Remove redundant clear of l2i at end of round-robin loop

From: Keir Fraser <keir.fraser@citrix.com>

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
[ijc: forward ported from linux-2.6.18-xen.hg 991:9ba6d9f3fbc0]
---
 drivers/xen/events.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 0c3f17b..8786e06 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1140,7 +1140,6 @@ static void __xen_evtchn_do_upcall(void)
 				pending_words &= ~(1UL << word_idx);
 
 			word_idx = (word_idx + 1) % BITS_PER_LONG;
-			bit_idx = 0;
 		}
 
 		BUG_ON(!irqs_disabled());
-- 
1.5.6.5


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

John Weekes

2011-Mar-04 08:40 UTC

head link

Re: [GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

On 3/3/2011 9:09 AM, Ian Campbell wrote:> On Thu, 2011-03-03 at 08:30 +0000, Keir Fraser wrote:
>> >  Without the round-robin servicing, unfairness to the point of
starvation is
>> >  a distinct possibility.
> Indeed. The following is a forward port of these patches from
> 2.6.18-xen.hg:
Thanks for working on this, Ian.

I think that this has affected me several times, as I''ve noticed these 
symptoms before (heavy disk access by a few domains, followed by several 
Windows domUs mysteriously rebooting) -- as recently as Wednesday, in 
fact. The I/O from rebooting domUs, unfortunately, makes everything even 
worse, often leading to further Windows domUs crashing.

I look forward to this being merged into stable-2.6.32. In the meantime, 
I''ll monkey around with git as you described and try to get a local 
version working. I''m itching to test it out, since this also might 
explain some of the occasional the longish disk delays that I see on 
machines with a medium I/O load (in which the dom0 still responds 
normally, but one of the domUs frequently stalls waiting for I/O).

-John

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

Ian Campbell

2011-Mar-04 09:15 UTC

head link

Re: [GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

On Fri, 2011-03-04 at 08:40 +0000, John Weekes wrote:> On 3/3/2011 9:09 AM, Ian Campbell wrote:
> > On Thu, 2011-03-03 at 08:30 +0000, Keir Fraser wrote:
> >> >  Without the round-robin servicing, unfairness to the point
of starvation is
> >> >  a distinct possibility.
> > Indeed. The following is a forward port of these patches from
> > 2.6.18-xen.hg:
> 
> Thanks for working on this, Ian.
> 
> I think that this has affected me several times, as I''ve noticed
these
> symptoms before (heavy disk access by a few domains, followed by several 
> Windows domUs mysteriously rebooting) -- as recently as Wednesday, in 
> fact. The I/O from rebooting domUs, unfortunately, makes everything even 
> worse, often leading to further Windows domUs crashing.
> 
> I look forward to this being merged into stable-2.6.32. In the meantime, 
> I''ll monkey around with git as you described and try to get a
local
> version working. I''m itching to test it out, since this also might
> explain some of the occasional the longish disk delays that I see on 
> machines with a medium I/O load (in which the dom0 still responds 
> normally, but one of the domUs frequently stalls waiting for I/O).
This series was for more recent kernels. It seems to backport with
minimum fuss (switch from __this_cpu_(read|write) to __get_cpu_var).

I only compile tested this one file and haven''t even booted it but the
result is available on top of xen.git xen/next-2.6.32 at:

The following changes since commit 892d2f052e979cf1916647c752b94cf62ec1c6dc:
  Jeremy Fitzhardinge (1):
        Merge commit ''v2.6.32.28'' into xen/next-2.6.32

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/linux-2.6.git irq-fairness-2.6.32

Ian Campbell (1):
      xen: events: Make last processed event channel a per-cpu variable.

Keir Fraser (3):
      xen: events: Clean up round-robin evtchn scan.
      xen: events: Make round-robin scan fairer by snapshotting each l2 word
      xen: events: Remove redundant clear of l2i at end of round-robin loop

Scott Rixner (1):
      xen: events: Process event channels notifications in round-robin order.

 drivers/xen/events.c |   80 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 72 insertions(+), 8 deletions(-)

Ian.



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

John Weekes

2011-Mar-07 19:33 UTC

head link

Re: [GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

On 3/4/2011 1:15 AM, Ian Campbell wrote:> This series was for more recent kernels. It seems to backport with
> minimum fuss (switch from __this_cpu_(read|write) to __get_cpu_var).
>
> I only compile tested this one file and haven''t even booted it but
the
> result is available on top of xen.git xen/next-2.6.32 at:
> ...
>    git://xenbits.xen.org/people/ianc/linux-2.6.git irq-fairness-2.6.32
Thanks for the rebase and for the earlier instructions on how to merge 
your git topic branch, which were clear and easy to follow.

I''ve have this code running on a lightly-used production machine right 
now with no problems observed so far. I will continue to monitor and 
roll it out to a higher-usage machine that has had problems without the 
RR bits within the next few days, if all goes well.

-John

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

Konrad Rzeszutek Wilk

2011-Mar-09 20:32 UTC

head link

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

On Thu, Mar 03, 2011 at 05:10:12PM +0000, Ian Campbell
wrote:> Signed-off-by: Ian Campbell <ian.campbell@xensource.com>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
You cloned yourself?
> [ijc: forward ported from linux-2.6.18-xen.hg 325:b2768401db94]
> ---
>  drivers/xen/events.c |   13 ++++++-------
>  1 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 75cc6f5..1fc3192 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -1026,6 +1026,8 @@ irqreturn_t xen_debug_interrupt(int irq, void
*dev_id)
>  }
>  
>  static DEFINE_PER_CPU(unsigned, xed_nesting_count);
> +static DEFINE_PER_CPU(unsigned int, last_word_idx) = { BITS_PER_LONG - 1
};
> +static DEFINE_PER_CPU(unsigned int, last_bit_idx) = { BITS_PER_LONG - 1 };
>  
>  /*
>   * Mask out the i least significant bits of w
> @@ -1043,8 +1045,6 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
>   */
>  static void __xen_evtchn_do_upcall(void)
>  {
> -	static unsigned int last_word_idx = BITS_PER_LONG - 1;
> -	static unsigned int last_bit_idx = BITS_PER_LONG - 1;
>  	int word_idx, bit_idx;
>  	int cpu = get_cpu();
>  	struct shared_info *s = HYPERVISOR_shared_info;
> @@ -1065,8 +1065,8 @@ static void __xen_evtchn_do_upcall(void)
>  #endif
>  		pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
>  
> -		word_idx = last_word_idx;
> -		bit_idx = last_bit_idx;
> +		word_idx = __this_cpu_read(last_word_idx);
> +		bit_idx = __this_cpu_read(last_bit_idx);
>  
>  		while (pending_words != 0) {
>  			unsigned long pending_bits;
> @@ -1121,9 +1121,8 @@ static void __xen_evtchn_do_upcall(void)
>  				 * If this is the final port processed, we''ll
>  				 * pick up here+1 next time.
>  				 */
> -				last_word_idx = word_idx;
> -				last_bit_idx = bit_idx;
> -
> +				__this_cpu_write(last_word_idx, word_idx);
> +				__this_cpu_write(last_bit_idx, bit_idx);
>  			} while (bit_idx != BITS_PER_LONG - 1);
>  
>  			pending_bits = active_evtchns(cpu, s, word_idx);
> -- 
> 1.5.6.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ian Campbell

2011-Mar-09 20:40 UTC

head link

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

On Wed, 2011-03-09 at 20:32 +0000, Konrad Rzeszutek Wilk
wrote:> On Thu, Mar 03, 2011 at 05:10:12PM +0000, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@xensource.com>
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> You cloned yourself?
The first is from the original linux-2.6.18-xen.hg commit.

The second is from today as I forward the change upstream.

I wasn''t really sure what to do with the historical signed-off-by so I
decided to leave it, just as I would have done if it had been someone
else''s.

Ian.
> 
> > [ijc: forward ported from linux-2.6.18-xen.hg 325:b2768401db94]
> > ---
> >  drivers/xen/events.c |   13 ++++++-------
> >  1 files changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index 75cc6f5..1fc3192 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -1026,6 +1026,8 @@ irqreturn_t xen_debug_interrupt(int irq, void
*dev_id)
> >  }
> >  
> >  static DEFINE_PER_CPU(unsigned, xed_nesting_count);
> > +static DEFINE_PER_CPU(unsigned int, last_word_idx) = { BITS_PER_LONG
- 1 };
> > +static DEFINE_PER_CPU(unsigned int, last_bit_idx) = { BITS_PER_LONG -
1 };
> >  
> >  /*
> >   * Mask out the i least significant bits of w
> > @@ -1043,8 +1045,6 @@ static DEFINE_PER_CPU(unsigned,
xed_nesting_count);
> >   */
> >  static void __xen_evtchn_do_upcall(void)
> >  {
> > -	static unsigned int last_word_idx = BITS_PER_LONG - 1;
> > -	static unsigned int last_bit_idx = BITS_PER_LONG - 1;
> >  	int word_idx, bit_idx;
> >  	int cpu = get_cpu();
> >  	struct shared_info *s = HYPERVISOR_shared_info;
> > @@ -1065,8 +1065,8 @@ static void __xen_evtchn_do_upcall(void)
> >  #endif
> >  		pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> >  
> > -		word_idx = last_word_idx;
> > -		bit_idx = last_bit_idx;
> > +		word_idx = __this_cpu_read(last_word_idx);
> > +		bit_idx = __this_cpu_read(last_bit_idx);
> >  
> >  		while (pending_words != 0) {
> >  			unsigned long pending_bits;
> > @@ -1121,9 +1121,8 @@ static void __xen_evtchn_do_upcall(void)
> >  				 * If this is the final port processed, we''ll
> >  				 * pick up here+1 next time.
> >  				 */
> > -				last_word_idx = word_idx;
> > -				last_bit_idx = bit_idx;
> > -
> > +				__this_cpu_write(last_word_idx, word_idx);
> > +				__this_cpu_write(last_bit_idx, bit_idx);
> >  			} while (bit_idx != BITS_PER_LONG - 1);
> >  
> >  			pending_bits = active_evtchns(cpu, s, word_idx);
> > -- 
> > 1.5.6.5
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel


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

Jeremy Fitzhardinge

2011-Mar-09 20:47 UTC

head link

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

On 03/09/2011 12:40 PM, Ian Campbell wrote:> On Wed, 2011-03-09 at 20:32 +0000, Konrad Rzeszutek Wilk wrote:
>> On Thu, Mar 03, 2011 at 05:10:12PM +0000, Ian Campbell wrote:
>>> Signed-off-by: Ian Campbell <ian.campbell@xensource.com>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> You cloned yourself?
> The first is from the original linux-2.6.18-xen.hg commit.
>
> The second is from today as I forward the change upstream.
>
> I wasn''t really sure what to do with the historical signed-off-by
so I
> decided to leave it, just as I would have done if it had been someone
> else''s.
Since Citrix and Xensource are logically the same entity, just the
Citrix one should be fine.

    J

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

Ian Campbell

2011-Mar-10 08:30 UTC

head link

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

On Wed, 2011-03-09 at 20:47 +0000, Jeremy Fitzhardinge
wrote:> On 03/09/2011 12:40 PM, Ian Campbell wrote:
> > On Wed, 2011-03-09 at 20:32 +0000, Konrad Rzeszutek Wilk wrote:
> >> On Thu, Mar 03, 2011 at 05:10:12PM +0000, Ian Campbell wrote:
> >>> Signed-off-by: Ian Campbell <ian.campbell@xensource.com>
> >>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> >> You cloned yourself?
> > The first is from the original linux-2.6.18-xen.hg commit.
> >
> > The second is from today as I forward the change upstream.
> >
> > I wasn''t really sure what to do with the historical
signed-off-by so I
> > decided to leave it, just as I would have done if it had been someone
> > else''s.
> 
> Since Citrix and Xensource are logically the same entity, just the
> Citrix one should be fine.
You are almost certainly right, but the xensource one is harmless and
accurately describes the provenance of the patch.

More importantly I don''t think it''s a big enough deal to be
worth
respinning the series

Ian.


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

Jeremy Fitzhardinge

2011-Mar-11 17:46 UTC

head link

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

On 03/10/2011 12:30 AM, Ian Campbell wrote:> More importantly I don''t think it''s a big enough deal to
be worth
> respinning the series
Definitely not.

    J


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

Xen devel - Mar 2011 - unfair servicing of DomU vbd requests

[Xen-devel] unfair servicing of DomU vbd requests

Re: [Xen-devel] unfair servicing of DomU vbd requests

Re: [Xen-devel] unfair servicing of DomU vbd requests

RE: [Xen-devel] unfair servicing of DomU vbd requests

Re: [Xen-devel] unfair servicing of DomU vbd requests

[GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

[Xen-devel] [PATCH 1/5] xen: events: Process event channels notifications in round-robin order.

[Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

[Xen-devel] [PATCH 3/5] xen: events: Clean up round-robin evtchn scan.

[Xen-devel] [PATCH 4/5] xen: events: Make round-robin scan fairer by snapshotting each l2 word

[Xen-devel] [PATCH 5/5] xen: events: Remove redundant clear of l2i at end of round-robin loop

Re: [GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

Re: [GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

Re: [GIT/PATCH 0/5] Re: [Xen-devel] unfair servicing of DomU vbd requests

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.

Re: [Xen-devel] [PATCH 2/5] xen: events: Make last processed event channel a per-cpu variable.