[Xen-devel] Re: [PATCH 5/5] xen: events: use per-cpu variable for cpu_evtchn_mask

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Tue, 2010-10-26 at 15:36 +0100, Konrad Rzeszutek Wilk wrote:
> On Mon, Oct 25, 2010 at 05:23:33PM +0100, Ian Campbell wrote:
> > I can't see any reason why it isn't already.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >  drivers/xen/events.c |   31 +++++++++++--------------------
> >  1 files changed, 11 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index 9b58505..144ff72 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -110,19 +110,9 @@ static int *pirq_to_irq;
> >  static int nr_pirqs;
> >  
> >  static int *evtchn_to_irq;
> > -struct cpu_evtchn_s {
> > -   unsigned long bits[NR_EVENT_CHANNELS/BITS_PER_LONG];
> > -};
> >  
> > -static __initdata struct cpu_evtchn_s init_evtchn_mask = {
> > -   .bits[0 ... (NR_EVENT_CHANNELS/BITS_PER_LONG)-1] = ~0ul,
> > -};
> > -static struct cpu_evtchn_s *cpu_evtchn_mask_p = &init_evtchn_mask;
> > -
> > -static inline unsigned long *cpu_evtchn_mask(int cpu)
> > -{
> > -   return cpu_evtchn_mask_p[cpu].bits;
> > -}
> > +static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
> > +                 cpu_evtchn_mask);
> >  
> >  /* Xen will never allocate port zero for any purpose. */
> >  #define VALID_EVTCHN(chn)  ((chn) != 0)
> > @@ -301,7 +291,7 @@ static inline unsigned long active_evtchns(unsigned int 
> > cpu,
> >                                        unsigned int idx)
> >  {
> >     return (sh->evtchn_pending[idx] &
> > -           cpu_evtchn_mask(cpu)[idx] &
> > +           per_cpu(cpu_evtchn_mask, cpu)[idx] &
> >             ~sh->evtchn_mask[idx]);
> >  }
> >  
> > @@ -314,8 +304,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, 
> > unsigned int cpu)
> >     cpumask_copy(irq_to_desc(irq)->affinity, cpumask_of(cpu));
> >  #endif
> >  
> > -   __clear_bit(chn, cpu_evtchn_mask(cpu_from_irq(irq)));
> > -   __set_bit(chn, cpu_evtchn_mask(cpu));
> > +   __clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
> > +   __set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
> >  
> >     info_for_irq(irq)->cpu = cpu;
> >  }
> > @@ -332,7 +322,11 @@ static void init_evtchn_cpu_bindings(void)
> >     }
> >  #endif
> >  
> > -   memset(cpu_evtchn_mask(0), ~0, sizeof(struct cpu_evtchn_s));
> > +   printk(KERN_CRIT "%s: CPU0 at %p size %zd\n", __func__,
> 
> If this is per_cpu, wouldn't the comment 'CPU0 at' be improper?

Oops, that was just debug anyway.

> Hmm, so we use percpu structure, but all of the event channel and such
> are set for CPU0. So what is the benefit of this, when the interrupts
> are _only_ happening on CPU0?

All event channels start off bound to VCPU0 so we need to initialise
cpu_evtchn_mask(0) to have all ones and leave all the other CPU's
bitmaps with all zeros.

> Oh, right, there is also the rebind cpu function somewhere so that the
> spinlock, timers, etc are allocated on other CPUs, right?

All evtchn's start off bound to VCPU 0 but the kernel can rebind as
necessary.

Ian.


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