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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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}()


> > +                                     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