Kaushik Kumar Ram
2010-Jan-29 06:02 UTC
[Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
# HG changeset patch
# User kaushik
# Date 1264744731 21600
# Node ID 12c3dcae53baf45665266614d4b589875c819355
# Parent 91224343eeee460c9aafdaadc1bdedab54e92256
Search a snapshot of level 2 bits for pending upcalls.
Using only a snapshot of level 1 bits can lead to unfairness
in some cases. Consider the case where two upcalls are marked
pending after a snapshot of level 1 bits is taken. One of these
two upcalls can still be processed if the corresponding level 1
bit was already set.
This is not a perfect solution since its still possible for a level 2
bit to be set before a level 2 snapshot is taken (unless both
snapshots are taken atomically).
Signed-off-by: Kaushik Kumar Ram <kaushik@rice.edu>
diff -r 91224343eeee -r 12c3dcae53ba drivers/xen/core/evtchn.c
--- a/drivers/xen/core/evtchn.c Thu Jan 21 15:05:02 2010 +0000
+++ b/drivers/xen/core/evtchn.c Thu Jan 28 23:58:51 2010 -0600
@@ -243,6 +243,8 @@ asmlinkage void evtchn_do_upcall(struct
unsigned int cpu = smp_processor_id();
shared_info_t *s = HYPERVISOR_shared_info;
vcpu_info_t *vcpu_info = &s->vcpu_info[cpu];
+ int i;
+ unsigned long local_l2[sizeof(unsigned long) * 8];
exit_idle();
irq_enter();
@@ -260,6 +262,10 @@ asmlinkage void evtchn_do_upcall(struct
wmb();
#endif
l1 = xchg(&vcpu_info->evtchn_pending_sel, 0);
+
+ for(i = 0; i < BITS_PER_LONG; i++) {
+ local_l2[i] = active_evtchns(cpu, s, i);
+ }
l1i = per_cpu(last_processed_l1i, cpu);
l2i = per_cpu(last_processed_l2i, cpu);
@@ -277,7 +283,7 @@ asmlinkage void evtchn_do_upcall(struct
l1i = __ffs(masked_l1);
do {
- l2 = active_evtchns(cpu, s, l1i);
+ l2 = local_l2[l1i];
l2i = (l2i + 1) % BITS_PER_LONG;
masked_l2 = l2 & ((~0UL) << l2i);
@@ -296,13 +302,15 @@ asmlinkage void evtchn_do_upcall(struct
else
evtchn_device_upcall(port);
+ clear_bit(l2i, &local_l2[l1i]);
+
/* if this is the final port processed,
we''ll pick up here+1 next time */
per_cpu(last_processed_l1i, cpu) = l1i;
per_cpu(last_processed_l2i, cpu) = l2i;
} while (l2i != BITS_PER_LONG - 1);
- l2 = active_evtchns(cpu, s, l1i);
+ l2 = local_l2[l1i];
if (l2 == 0) /* we handled all ports, so we can clear
the selector bit */
l1 &= ~(1UL << l1i);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jan-29 08:46 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On 29/01/2010 06:02, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:> Search a snapshot of level 2 bits for pending upcalls. > > Using only a snapshot of level 1 bits can lead to unfairness > in some cases. Consider the case where two upcalls are marked > pending after a snapshot of level 1 bits is taken. One of these > two upcalls can still be processed if the corresponding level 1 > bit was already set. > > This is not a perfect solution since its still possible for a level 2 > bit to be set before a level 2 snapshot is taken (unless both > snapshots are taken atomically).When I look at this I see 512 extra bytes on the stack, and a possibly theoretical fairness improvement. Is the improvement measurable? Even if it is, I wonder how much of the unfairness comes from only clearing bit l1i in l1 if active_evtchns(l1i) is zero? I understand something like that is needed to deal with when we start scanning from halfway through an l2, but clearly the potential impact of that check-before-the-clear is wider ranging. If you''ve measured a fairness/performance win, you might get all or most of it by making the check-before-clear more sophisticated, and at no extra cost in stack space. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kaushik Kumar Ram
2010-Jan-30 02:19 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On Jan 29, 2010, at 2:46 AM, Keir Fraser wrote:> On 29/01/2010 06:02, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >> Search a snapshot of level 2 bits for pending upcalls. >> >> Using only a snapshot of level 1 bits can lead to unfairness >> in some cases. Consider the case where two upcalls are marked >> pending after a snapshot of level 1 bits is taken. One of these >> two upcalls can still be processed if the corresponding level 1 >> bit was already set. >> >> This is not a perfect solution since its still possible for a level 2 >> bit to be set before a level 2 snapshot is taken (unless both >> snapshots are taken atomically). > > When I look at this I see 512 extra bytes on the stack, and a possibly > theoretical fairness improvement. Is the improvement measurable?We did observe measurable improvement during network I/O (fair bandwidth allocation with up to 8 guests).> Even if it > is, I wonder how much of the unfairness comes from only clearing bit l1i in > l1 if active_evtchns(l1i) is zero? I understand something like that is > needed to deal with when we start scanning from halfway through an l2, but > clearly the potential impact of that check-before-the-clear is wider > ranging. If you''ve measured a fairness/performance win, you might get all or > most of it by making the check-before-clear more sophisticated, and at no > extra cost in stack space.I understand your concern about the extra bytes in the stack. But I don''t follow your other arguments here. Can you explain a bit more? At the very least, I think the l2 bits should not be copied in every iteration of the inner loop. In other words, the l2 bits have to copied first (once) and then checked in the inner loop for pending upcalls. -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jan-30 08:19 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On 30/01/2010 02:19, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>> When I look at this I see 512 extra bytes on the stack, and a possibly >> theoretical fairness improvement. Is the improvement measurable? > > We did observe measurable improvement during network I/O (fair bandwidth > allocation with up to 8 guests).That''s good.> I understand your concern about the extra bytes in the stack. But I don''t > follow your other arguments here. Can you explain a bit more?Well, following your suggestion below, I agree it would be very good to pull the read of active_evtchns(l1i) out of the inner loop. But that is somewhat defeated if you continue to read active_evtchns(l1i) after the loop, and make clearing l1i in l1 mask conditional on it being zero: that means you will definitely come back to this l1i before resampling the active l1 mask and finding potential new l1i''s to scan. So how about making the clear of l1i in the l1 mask unconditional? I think that would be better, but I wasn''t sure it is safe, since the first l1i you scan you may start halfway through, and thus legitimately have more work to do on that l1i on a later iteration of the outer loop. But I think that is the only case it is good to leave the l1 unmasked? Also, even better, on that second scan of that l1i, you would preferably want to scan only those bits in the l2 mask that you didn''t scan on the first iteration of the outer loop! Does that all make sense? -- Keir> At the very least, I think the l2 bits should not be copied in every iteration > of the inner loop. > In other words, the l2 bits have to copied first (once) and then checked in > the inner loop > for pending upcalls. > > -Kaushik_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kaushik Kumar Ram
2010-Jan-31 00:40 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On Jan 30, 2010, at 2:19 AM, Keir Fraser wrote:> On 30/01/2010 02:19, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >> I understand your concern about the extra bytes in the stack. But I don''t >> follow your other arguments here. Can you explain a bit more? > > Well, following your suggestion below, I agree it would be very good to pull > the read of active_evtchns(l1i) out of the inner loop. But that is somewhat > defeated if you continue to read active_evtchns(l1i) after the loop, and > make clearing l1i in l1 mask conditional on it being zero: that means you > will definitely come back to this l1i before resampling the active l1 mask > and finding potential new l1i''s to scan. > > So how about making the clear of l1i in the l1 mask unconditional? I think > that would be better, but I wasn''t sure it is safe, since the first l1i you > scan you may start halfway through, and thus legitimately have more work to > do on that l1i on a later iteration of the outer loop. But I think that is > the only case it is good to leave the l1 unmasked? Also, even better, on > that second scan of that l1i, you would preferably want to scan only those > bits in the l2 mask that you didn''t scan on the first iteration of the outer > loop!OK. I agree the following is a good compromise. - Unconditionally clear l1 bits except the first l1i (but only if l2 is scanned from halfway). - Remember where the scanning began (both l1i and l2i) and stop scanning at that point after wrapping around. - Read active_evtchns() once per l1i (except the first l1i where you might have to do it twice). -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jan-31 08:24 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On 31/01/2010 00:40, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>> So how about making the clear of l1i in the l1 mask unconditional? I think >> that would be better, but I wasn''t sure it is safe, since the first l1i you >> scan you may start halfway through, and thus legitimately have more work to >> do on that l1i on a later iteration of the outer loop. But I think that is >> the only case it is good to leave the l1 unmasked? Also, even better, on >> that second scan of that l1i, you would preferably want to scan only those >> bits in the l2 mask that you didn''t scan on the first iteration of the outer >> loop! > > OK. I agree the following is a good compromise. > - Unconditionally clear l1 bits except the first l1i (but only if l2 is > scanned from halfway). > - Remember where the scanning began (both l1i and l2i) and stop scanning at > that point after wrapping around. > - Read active_evtchns() once per l1i (except the first l1i where you might > have to do it twice).Yes, sounds good. Are you going to make the patch? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kaushik Kumar Ram
2010-Jan-31 08:33 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On Jan 31, 2010, at 2:24 AM, Keir Fraser wrote:> On 31/01/2010 00:40, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >>> So how about making the clear of l1i in the l1 mask unconditional? I think >>> that would be better, but I wasn''t sure it is safe, since the first l1i you >>> scan you may start halfway through, and thus legitimately have more work to >>> do on that l1i on a later iteration of the outer loop. But I think that is >>> the only case it is good to leave the l1 unmasked? Also, even better, on >>> that second scan of that l1i, you would preferably want to scan only those >>> bits in the l2 mask that you didn''t scan on the first iteration of the outer >>> loop! >> >> OK. I agree the following is a good compromise. >> - Unconditionally clear l1 bits except the first l1i (but only if l2 is >> scanned from halfway). >> - Remember where the scanning began (both l1i and l2i) and stop scanning at >> that point after wrapping around. >> - Read active_evtchns() once per l1i (except the first l1i where you might >> have to do it twice). > > Yes, sounds good. Are you going to make the patch?OK. I will give it a shot. -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kaushik Kumar Ram
2010-Feb-01 04:19 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On Jan 31, 2010, at 2:24 AM, Keir Fraser wrote:> On 31/01/2010 00:40, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >>> So how about making the clear of l1i in the l1 mask unconditional? I think >>> that would be better, but I wasn''t sure it is safe, since the first l1i you >>> scan you may start halfway through, and thus legitimately have more work to >>> do on that l1i on a later iteration of the outer loop. But I think that is >>> the only case it is good to leave the l1 unmasked? Also, even better, on >>> that second scan of that l1i, you would preferably want to scan only those >>> bits in the l2 mask that you didn''t scan on the first iteration of the outer >>> loop! >> >> OK. I agree the following is a good compromise. >> - Unconditionally clear l1 bits except the first l1i (but only if l2 is >> scanned from halfway). >> - Remember where the scanning began (both l1i and l2i) and stop scanning at >> that point after wrapping around. >> - Read active_evtchns() once per l1i (except the first l1i where you might >> have to do it twice). > > Yes, sounds good. Are you going to make the patch?Here is a first version of the patch. It was more complicated than I expected and it was also hard for me to decide if it was "efficient" enough. I can work on improving it, based on your feedback. # HG changeset patch # User kaushik@sp11.cs.rice.edu # Date 1264994160 21600 # Branch evtchn2 # Node ID e539a849a3348c3e87e8d50eba5998b7cdb9394d # Parent c88a02a22a057a632e6c21442e42e56e07904988 Fair processing of pending upcalls. Signed-off-by: Kaushik Kumar Ram <kaushik@rice.edu> diff -r c88a02a22a05 -r e539a849a334 drivers/xen/core/evtchn.c --- a/drivers/xen/core/evtchn.c Fri Jan 29 07:57:48 2010 +0000 +++ b/drivers/xen/core/evtchn.c Sun Jan 31 21:16:00 2010 -0600 @@ -236,9 +236,9 @@ static DEFINE_PER_CPU(unsigned int, curr /* NB. Interrupts are disabled on entry. */ asmlinkage void evtchn_do_upcall(struct pt_regs *regs) { - unsigned long l1, l2; + unsigned long l1, l2, l2_start = 0; unsigned long masked_l1, masked_l2; - unsigned int l1i, l2i, port, count; + unsigned int l1i, l2i = 0, port, count, l1i_start, l2i_start; int irq; unsigned int cpu = smp_processor_id(); shared_info_t *s = HYPERVISOR_shared_info; @@ -261,48 +261,96 @@ asmlinkage void evtchn_do_upcall(struct #endif l1 = xchg(&vcpu_info->evtchn_pending_sel, 0); - l1i = per_cpu(current_l1i, cpu); - l2i = per_cpu(current_l2i, cpu); + l1i = l1i_start = per_cpu(current_l1i, cpu); + l2i_start = per_cpu(current_l2i, cpu); + + if(l1 != 0) + { + masked_l1 = l1 & ((~0UL) << l1i); + if (masked_l1 != 0) { + l1i = __ffs(masked_l1); + if(l1i == l1i_start && l2i_start != 0) { + l2 = active_evtchns(cpu, s, l1i); + l2i = l2i_start; + l2_start = l2 & ((~0UL) >> (BITS_PER_LONG - l2i)); + } + } + else { + l1i = 0; + masked_l1 = l1 & ((~0UL) << l1i); + l1i = __ffs(masked_l1); + l2 = active_evtchns(cpu, s, l1i); + } - while (l1 != 0) { - masked_l1 = l1 & ((~0UL) << l1i); - /* If we masked out all events, wrap to beginning. */ - if (masked_l1 == 0) { - l1i = l2i = 0; - continue; - } - l1i = __ffs(masked_l1); + while (1) { - do { - l2 = active_evtchns(cpu, s, l1i); - masked_l2 = l2 & ((~0UL) << l2i); - if (masked_l2 == 0) - break; - l2i = __ffs(masked_l2); + do { + masked_l2 = l2 & ((~0UL) << l2i); + if (masked_l2 == 0) + break; + l2i = __ffs(masked_l2); - /* process port */ - port = (l1i * BITS_PER_LONG) + l2i; - if ((irq = evtchn_to_irq[port]) != -1) - do_IRQ(irq, regs); - else - evtchn_device_upcall(port); + /* process port */ + port = (l1i * BITS_PER_LONG) + l2i; + if ((irq = evtchn_to_irq[port]) != -1) + do_IRQ(irq, regs); + else + evtchn_device_upcall(port); - l2i = (l2i + 1) % BITS_PER_LONG; + l2i = (l2i + 1) % BITS_PER_LONG; - /* Next caller starts at last processed + 1 */ - per_cpu(current_l1i, cpu) - l2i ? l1i : (l1i + 1) % BITS_PER_LONG; - per_cpu(current_l2i, cpu) = l2i; + /* Next caller starts at last processed + 1 */ + per_cpu(current_l1i, cpu) + l2i ? l1i : (l1i + 1) % BITS_PER_LONG; + per_cpu(current_l2i, cpu) = l2i; - } while (l2i != 0); + } while (l2i != 0); - l2 = active_evtchns(cpu, s, l1i); - /* If we handled all ports, clear the selector bit. */ - if (l2 == 0) l1 &= ~(1UL << l1i); - l1i = (l1i + 1) % BITS_PER_LONG; - l2i = 0; + if(l1 == 0) + break; + + l1i = (l1i + 1) % BITS_PER_LONG; + + masked_l1 = l1 & ((~0UL) << l1i); + /* If we masked out all events, wrap to beginning. */ + if (masked_l1 == 0) { + l1i = 0; + masked_l1 = l1 & ((~0UL) << l1i); + } + l1i = __ffs(masked_l1); + l2i = 0; + l2 = active_evtchns(cpu, s, l1i); + } + + /* Check and process any pending events in the + * unprocessed portion of bits selected by l1i_start. + */ + if(l2_start != 0) { + l1i = l1i_start; + l2i = 0; + do { + masked_l2 = l2_start & ((~0UL) << l2i); + if (masked_l2 == 0) + break; + l2i = __ffs(masked_l2); + + /* process port */ + port = (l1i * BITS_PER_LONG) + l2i; + if ((irq = evtchn_to_irq[port]) != -1) + do_IRQ(irq, regs); + else + evtchn_device_upcall(port); + + l2i = (l2i + 1) % BITS_PER_LONG; + + /* Next caller starts at last processed + 1 */ + per_cpu(current_l1i, cpu) + l2i ? l1i : (l1i + 1) % BITS_PER_LONG; + per_cpu(current_l2i, cpu) = l2i; + }while(1); + } } /* If there were nested callbacks then we have more to do. */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kaushik Kumar Ram
2010-Feb-01 04:29 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On Jan 31, 2010, at 10:19 PM, Kaushik Kumar Ram wrote:> > On Jan 31, 2010, at 2:24 AM, Keir Fraser wrote: > >> On 31/01/2010 00:40, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: >> >>>> So how about making the clear of l1i in the l1 mask unconditional? I think >>>> that would be better, but I wasn''t sure it is safe, since the first l1i you >>>> scan you may start halfway through, and thus legitimately have more work to >>>> do on that l1i on a later iteration of the outer loop. But I think that is >>>> the only case it is good to leave the l1 unmasked? Also, even better, on >>>> that second scan of that l1i, you would preferably want to scan only those >>>> bits in the l2 mask that you didn''t scan on the first iteration of the outer >>>> loop! >>> >>> OK. I agree the following is a good compromise. >>> - Unconditionally clear l1 bits except the first l1i (but only if l2 is >>> scanned from halfway). >>> - Remember where the scanning began (both l1i and l2i) and stop scanning at >>> that point after wrapping around. >>> - Read active_evtchns() once per l1i (except the first l1i where you might >>> have to do it twice). >> >> Yes, sounds good. Are you going to make the patch? > > Here is a first version of the patch. It was more complicated than I expected > and it was also hard for me to decide if it was "efficient" enough. > I can work on improving it, based on your feedback.I had a to fix a minor bug, please consider this patch. # HG changeset patch # User kaushik@sp11.cs.rice.edu # Date 1264994160 21600 # Branch evtchn2 # Node ID e539a849a3348c3e87e8d50eba5998b7cdb9394d # Parent c88a02a22a057a632e6c21442e42e56e07904988 Fair processing of pending upcalls. Signed-off-by: Kaushik Kumar Ram <kaushik@rice.edu> diff -r c88a02a22a05 -r e539a849a334 drivers/xen/core/evtchn.c --- a/drivers/xen/core/evtchn.c Fri Jan 29 07:57:48 2010 +0000 +++ b/drivers/xen/core/evtchn.c Sun Jan 31 21:16:00 2010 -0600 @@ -236,9 +236,9 @@ static DEFINE_PER_CPU(unsigned int, curr /* NB. Interrupts are disabled on entry. */ asmlinkage void evtchn_do_upcall(struct pt_regs *regs) { - unsigned long l1, l2; + unsigned long l1, l2, l2_start = 0; unsigned long masked_l1, masked_l2; - unsigned int l1i, l2i, port, count; + unsigned int l1i, l2i = 0, port, count, l1i_start, l2i_start; int irq; unsigned int cpu = smp_processor_id(); shared_info_t *s = HYPERVISOR_shared_info; @@ -261,48 +261,96 @@ asmlinkage void evtchn_do_upcall(struct #endif l1 = xchg(&vcpu_info->evtchn_pending_sel, 0); - l1i = per_cpu(current_l1i, cpu); - l2i = per_cpu(current_l2i, cpu); + l1i = l1i_start = per_cpu(current_l1i, cpu); + l2i_start = per_cpu(current_l2i, cpu); + + if(l1 != 0) + { + masked_l1 = l1 & ((~0UL) << l1i); + if (masked_l1 != 0) { + l1i = __ffs(masked_l1); + l2 = active_evtchns(cpu, s, l1i); + if(l1i == l1i_start && l2i_start != 0) { + l2i = l2i_start; + l2_start = l2 & ((~0UL) >> (BITS_PER_LONG - l2i)); + } + } + else { + l1i = 0; + masked_l1 = l1 & ((~0UL) << l1i); + l1i = __ffs(masked_l1); + l2 = active_evtchns(cpu, s, l1i); + } - while (l1 != 0) { - masked_l1 = l1 & ((~0UL) << l1i); - /* If we masked out all events, wrap to beginning. */ - if (masked_l1 == 0) { - l1i = l2i = 0; - continue; - } - l1i = __ffs(masked_l1); + while (1) { - do { - l2 = active_evtchns(cpu, s, l1i); - masked_l2 = l2 & ((~0UL) << l2i); - if (masked_l2 == 0) - break; - l2i = __ffs(masked_l2); + do { + masked_l2 = l2 & ((~0UL) << l2i); + if (masked_l2 == 0) + break; + l2i = __ffs(masked_l2); - /* process port */ - port = (l1i * BITS_PER_LONG) + l2i; - if ((irq = evtchn_to_irq[port]) != -1) - do_IRQ(irq, regs); - else - evtchn_device_upcall(port); + /* process port */ + port = (l1i * BITS_PER_LONG) + l2i; + if ((irq = evtchn_to_irq[port]) != -1) + do_IRQ(irq, regs); + else + evtchn_device_upcall(port); - l2i = (l2i + 1) % BITS_PER_LONG; + l2i = (l2i + 1) % BITS_PER_LONG; - /* Next caller starts at last processed + 1 */ - per_cpu(current_l1i, cpu) - l2i ? l1i : (l1i + 1) % BITS_PER_LONG; - per_cpu(current_l2i, cpu) = l2i; + /* Next caller starts at last processed + 1 */ + per_cpu(current_l1i, cpu) + l2i ? l1i : (l1i + 1) % BITS_PER_LONG; + per_cpu(current_l2i, cpu) = l2i; - } while (l2i != 0); + } while (l2i != 0); - l2 = active_evtchns(cpu, s, l1i); - /* If we handled all ports, clear the selector bit. */ - if (l2 == 0) l1 &= ~(1UL << l1i); - l1i = (l1i + 1) % BITS_PER_LONG; - l2i = 0; + if(l1 == 0) + break; + + l1i = (l1i + 1) % BITS_PER_LONG; + + masked_l1 = l1 & ((~0UL) << l1i); + /* If we masked out all events, wrap to beginning. */ + if (masked_l1 == 0) { + l1i = 0; + masked_l1 = l1 & ((~0UL) << l1i); + } + l1i = __ffs(masked_l1); + l2i = 0; + l2 = active_evtchns(cpu, s, l1i); + } + + /* Check and process any pending events in the + * unprocessed portion of bits selected by l1i_start. + */ + if(l2_start != 0) { + l1i = l1i_start; + l2i = 0; + do { + masked_l2 = l2_start & ((~0UL) << l2i); + if (masked_l2 == 0) + break; + l2i = __ffs(masked_l2); + + /* process port */ + port = (l1i * BITS_PER_LONG) + l2i; + if ((irq = evtchn_to_irq[port]) != -1) + do_IRQ(irq, regs); + else + evtchn_device_upcall(port); + + l2i = (l2i + 1) % BITS_PER_LONG; + + /* Next caller starts at last processed + 1 */ + per_cpu(current_l1i, cpu) + l2i ? l1i : (l1i + 1) % BITS_PER_LONG; + per_cpu(current_l2i, cpu) = l2i; + }while(1); + } } /* If there were nested callbacks then we have more to do. */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Feb-01 13:59 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On 01/02/2010 04:29, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>>> Yes, sounds good. Are you going to make the patch? >> >> Here is a first version of the patch. It was more complicated than I expected >> and it was also hard for me to decide if it was "efficient" enough. >> I can work on improving it, based on your feedback. > > I had a to fix a minor bug, please consider this patch.Somehow the patch is mangled and does not apply. But also it is very big. How about the attached patch instead? I think it does all we agreed on, and is much smaller. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kaushik Kumar Ram
2010-Feb-01 19:24 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On Feb 1, 2010, at 7:59 AM, Keir Fraser wrote:> On 01/02/2010 04:29, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >>>> Yes, sounds good. Are you going to make the patch? >>> >>> Here is a first version of the patch. It was more complicated than I expected >>> and it was also hard for me to decide if it was "efficient" enough. >>> I can work on improving it, based on your feedback. >> >> I had a to fix a minor bug, please consider this patch. > > Somehow the patch is mangled and does not apply. But also it is very big. > How about the attached patch instead? I think it does all we agreed on, and > is much smaller.Well, I tried really hard to avoid the extra checks inside the loop. Maybe it doesn''t matter. There is one small issue in your patch, otherwise it is fine. When start_l1i is not set, l2i has to be made zero. Otherwise, we would start halfway through the next l2 whose l1i is set. I think this issue exists even in the original code. -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Feb-03 09:32 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On 01/02/2010 19:24, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>> Somehow the patch is mangled and does not apply. But also it is very big. >> How about the attached patch instead? I think it does all we agreed on, and >> is much smaller. > > Well, I tried really hard to avoid the extra checks inside the loop. > Maybe it doesn''t matter. > > There is one small issue in your patch, otherwise it is fine. When start_l1i > is not set, l2i has to be made zero. Otherwise, we would start halfway through > the next l2 whose l1i is set. I think this issue exists even in the original > code.Thanks Kaushik! I fixed that and checked in as linux-2.6.18-xen:990, which will soon appear in the main public repo. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kaushik Kumar Ram
2010-Feb-03 10:42 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On Feb 3, 2010, at 3:32 AM, Keir Fraser wrote:> On 01/02/2010 19:24, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >>> Somehow the patch is mangled and does not apply. But also it is very big. >>> How about the attached patch instead? I think it does all we agreed on, and >>> is much smaller. >> >> Well, I tried really hard to avoid the extra checks inside the loop. >> Maybe it doesn''t matter. >> >> There is one small issue in your patch, otherwise it is fine. When start_l1i >> is not set, l2i has to be made zero. Otherwise, we would start halfway through >> the next l2 whose l1i is set. I think this issue exists even in the original >> code. > > Thanks Kaushik! I fixed that and checked in as linux-2.6.18-xen:990, which > will soon appear in the main public repo.l2i=0 at the end of the outer loop seems redundant. Otherwise it is fine. Thanks. -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Feb-03 11:18 UTC
Re: [Xen-devel] [PATCH] evtchn_do_upcall: search a snapshot of level 2 bits for pending upcalls
On 03/02/2010 10:42, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>> Thanks Kaushik! I fixed that and checked in as linux-2.6.18-xen:990, which >> will soon appear in the main public repo. > > l2i=0 at the end of the outer loop seems redundant. Otherwise it is fine.I meant to remove it. :-( Thanks! K. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel