Re: [Xen-devel] [PATCH 09/13] xen: implement 3-level event channel routines

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Tue, Feb 05, 2013 at 05:39:02PM +0000, Wei Liu wrote:
> On Tue, 2013-02-05 at 17:09 +0000, Konrad Rzeszutek Wilk wrote:
> > >
> > > +static irqreturn_t xen_debug_interrupt_l3(int irq, void *dev_id)
> > > +{
> > > +     int cpu = smp_processor_id();
> > > +     unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> > > +     unsigned long nr_elems = NR_EVENT_CHANNELS_L3 / BITS_PER_LONG;
> > > +     int i;
> > > +     struct vcpu_info *v;
> > > +
> > > +     v = per_cpu(xen_vcpu, cpu);
> > > +
> > > +     printk(KERN_DEBUG "\npending (only show words which have bits set 
> > > to 1):\n   ");
> > > +     for (i = nr_elems-1; i >= 0; i--)
> > > +             if (evtchn_pending[i] != 0UL) {
> > > +                     printk(KERN_DEBUG " word index %d %0*lx\n",
> > > +                            i,
> > > +                            (int)sizeof(evtchn_pending[0])*2,
> > > +                            evtchn_pending[i]);
> > > +             }
> > > +
> > > +     printk(KERN_DEBUG "\nglobal mask (only show words which have bits 
> > > set to 0):\n   ");
> > > +     for (i = nr_elems-1; i >= 0; i--)
> > > +             if (evtchn_mask[i] != ~0UL) {
> > > +                     printk(KERN_DEBUG " word index %d %0*lx\n",
> > > +                            i,
> > > +                            (int)sizeof(evtchn_mask[0])*2,
> > > +                            evtchn_mask[i]);
> > > +             }
> > > +
> > > +     printk(KERN_DEBUG "\nglobally unmasked (only show result words 
> > > which have bits set to 1):\n   ");
> > > +     for (i = nr_elems-1; i >= 0; i--)
> > > +             if ((evtchn_pending[i] & ~evtchn_mask[i]) != 0UL) {
> > > +                     printk(KERN_DEBUG " word index %d %0*lx\n",
> > > +                            i,
> > > +                            (int)(sizeof(evtchn_mask[0])*2),
> > > +                            evtchn_pending[i] & ~evtchn_mask[i]);
> > > +             }
> > > +
> > > +     printk(KERN_DEBUG "\nlocal cpu%d mask (only show words which have 
> > > bits set to 1):\n   ", cpu);
> > > +     for (i = (NR_EVENT_CHANNELS_L3/BITS_PER_LONG)-1; i >= 0; i--)
> > > +             if (cpu_evtchn[i] != 0UL) {
> > > +                     printk(KERN_DEBUG " word index %d %0*lx\n",
> > > +                            i,
> > > +                            (int)(sizeof(cpu_evtchn[0])*2),
> > > +                            cpu_evtchn[i]);
> > > +             }
> > > +
> > > +     printk(KERN_DEBUG "\nlocally unmasked (only show result words which 
> > > have bits set to 1):\n   ");
> > > +     for (i = nr_elems-1; i >= 0; i--) {
> > > +             unsigned long pending = evtchn_pending[i]
> > > +                     & ~evtchn_mask[i]
> > > +                     & cpu_evtchn[i];
> > > +             if (pending != 0UL) {
> > > +                     printk(KERN_DEBUG " word index %d %0*lx\n",
> > > +                            i,
> > > +                            (int)(sizeof(evtchn_mask[0])*2),
> > > +                            pending);
> > > +             }
> > > +     }
> > > +
> > > +     printk(KERN_DEBUG "\npending list:\n");
> > > +     for (i = 0; i < NR_EVENT_CHANNELS_L3; i++) {
> > > +             if (sync_test_bit(i, evtchn_pending)) {
> > > +                     int word_idx = i / (BITS_PER_LONG * BITS_PER_LONG);
> > > +                     int word_idx_l2 = i / BITS_PER_LONG;
> > > +                     printk(KERN_DEBUG "  %d: event %d -> irq 
> > > %d%s%s%s%s\n",
> > > +                            cpu_from_evtchn(i), i,
> > > +                            evtchn_to_irq[i],
> > > +                            !sync_test_bit(word_idx, 
> > > &v->evtchn_pending_sel)
> > > +                            ? "" : " l1-clear",
> > > +                            !sync_test_bit(word_idx_l2, 
> > > per_cpu(evtchn_sel_l2, cpu))
> > > +                            ? "" : " l2-clear",
> > > +                            sync_test_bit(i, evtchn_mask)
> > > +                            ? "" : " globally-masked",
> > > +                            sync_test_bit(i, cpu_evtchn)
> > > +                            ? "" : " locally-masked");
> > > +             }
> > > +     }
> > > +
> > > +     return IRQ_HANDLED;
> > 
> > Um, there has to be a way to fold the most common cases of the L2 and L3
> > of this function in one?
> > 
> 
> The only common part is the for-loop. I could try to move statements in
> for-loops to dedicated functions, then the file will be filled with 10+
> functions like:
> 
> void output_l{2,3}_{globally,locally}_{masked,unmasked}()
> 
> void output_l{2,3}_pending()
> 
> void print_heading_for_l{2,3}()

Can it be via macros? Like arch/x86/mm/dump_pagetables.c has it for the
pagetable walker?
> 
> 
> > > +                                     pending_words_l2 &= ~(1UL << 
> > > word_idx_l2);
> > > +
> > > +                             word_idx_l2 = (word_idx_l2 + 1) % 
> > > BITS_PER_LONG;
> > > +                     }
> > 
> > This is a bit of a complex code. Is there any way you can split this up
> > in smaller inline functions?
> 
> I will try.
> 
> 
> Wei.
> 
> 

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