WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 3/6] eliminate cpu_set()

To: Jan Beulich <JBeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 3/6] eliminate cpu_set()
From: Keir Fraser <keir@xxxxxxx>
Date: Mon, 07 Nov 2011 14:31:59 +0000
Cc:
Delivery-date: Mon, 07 Nov 2011 08:27:04 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:user-agent:date:subject:from:to:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; bh=nsDc0qUBG3UerErcg1UE04YY2LueCAgu+CtrkHzW6Bw=; b=ZJWUVGeg3K62Q968D2iiHxivFjCfTF+r+4K+B0Aw7uoVXoIbMybyaAIl3PG5n8crd3 XgUKnFFCI29/HzcllrKJCPJY19v4mZ8XYLB8D5AKMA8DLLxQtf7fUI8DBiP2XQFgIHqa XO7GiTDAnKeCYp5e4FD3VZXzttGYdzxjDAkq8=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4EB7B997020000780005F4AC@xxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcydWgHRB0a3JnfCQkaCBIeeKDQsIA==
Thread-topic: [Xen-devel] [PATCH 3/6] eliminate cpu_set()
User-agent: Microsoft-Entourage/12.30.0.110427
On 07/11/2011 09:57, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I don't like set_cpu_{present,online} taking a boolean clear/set flag. There
is no caller that can both set and clear a flag, so it is always hardcoded
as 0 or 1. And then the reader has to make a (probably not hard) guess what
that means.

If you must add an abstraction interface here, better to define four of
them: {set,clear}_cpu_{present,online}.

Apart from this, all patches are:
Acked-by: Keir Fraser <keir@xxxxxxx>

 -- Keir

> --- a/xen/arch/ia64/linux-xen/acpi.c
> +++ b/xen/arch/ia64/linux-xen/acpi.c
> @@ -557,7 +557,7 @@ acpi_numa_processor_affinity_init(struct
>    (pa->apic_id << 8) | (pa->local_sapic_eid);
> /* nid should be overridden as logical node id later */
> node_cpuid[srat_num_cpus].nid = pxm;
> - cpu_set(srat_num_cpus, early_cpu_possible_map);
> + cpumask_set_cpu(srat_num_cpus, &early_cpu_possible_map);
> srat_num_cpus++;
>  }
>  
> @@ -917,7 +917,7 @@ __init void prefill_possible_map(void)
> possible, max((possible - available_cpus), 0));
>  
> for (i = 0; i < possible; i++)
> -  cpu_set(i, cpu_possible_map);
> +  cpumask_set_cpu(i, &cpu_possible_map);
>  }
>  
>  #ifndef XEN
> --- a/xen/arch/ia64/linux-xen/setup.c
> +++ b/xen/arch/ia64/linux-xen/setup.c
> @@ -463,7 +463,7 @@ mark_bsp_online (void)
>  {
>  #ifdef CONFIG_SMP
> /* If we register an early console, allow CPU 0 to printk */
> - cpu_set(smp_processor_id(), cpu_online_map);
> + set_cpu_online(smp_processor_id(), 1);
>  #endif
>  }
>  
> --- a/xen/arch/ia64/linux-xen/smpboot.c
> +++ b/xen/arch/ia64/linux-xen/smpboot.c
> @@ -392,7 +392,7 @@ smp_callin (void)
>  #else
> lock_ipi_calllock();
>  #endif
> - cpu_set(cpuid, cpu_online_map);
> + set_cpu_online(cpuid, 1);
>  #ifdef XEN
> unlock_ipi_calllock(flags);
>  #else
> @@ -437,7 +437,7 @@ smp_callin (void)
> /*
> * Allow the master to continue.
> */
> - cpu_set(cpuid, cpu_callin_map);
> + cpumask_set_cpu(cpuid, &cpu_callin_map);
> Dprintk("Stack on CPU %d at about %p\n",cpuid, &cpuid);
>  }
>  
> @@ -625,8 +625,8 @@ smp_prepare_cpus (unsigned int max_cpus)
> /*
> * We have the boot CPU online for sure.
> */
> - cpu_set(0, cpu_online_map);
> - cpu_set(0, cpu_callin_map);
> + set_cpu_online(0, 1);
> + cpumask_set_cpu(0, &cpu_callin_map);
>  
> local_cpu_data->loops_per_jiffy = loops_per_jiffy;
> ia64_cpu_to_sapicid[0] = boot_cpu_id;
> @@ -652,8 +652,8 @@ smp_prepare_cpus (unsigned int max_cpus)
>  
>  void __devinit smp_prepare_boot_cpu(void)
>  {
> - cpu_set(smp_processor_id(), cpu_online_map);
> - cpu_set(smp_processor_id(), cpu_callin_map);
> + set_cpu_online(smp_processor_id(), 1);
> + cpumask_set_cpu(smp_processor_id(), &cpu_callin_map);
> per_cpu(cpu_state, smp_processor_id()) = CPU_ONLINE;
>  }
>  
> --- a/xen/arch/ia64/linux-xen/sn/kernel/sn2_smp.c
> +++ b/xen/arch/ia64/linux-xen/sn/kernel/sn2_smp.c
> @@ -214,7 +214,7 @@ sn2_global_tlb_purge(unsigned long start
> for_each_possible_cpu(cpu) {
> cnode = cpu_to_node(cpu);
> if (!node_isset(cnode, nodes_flushed)) {
> -   cpu_set(cpu, selected_cpus);
> +   cpumask_set_cpu(cpu, &selected_cpus);
> i++;
> }
> node_set(cnode, nodes_flushed);
> --- a/xen/arch/ia64/xen/mm_init.c
> +++ b/xen/arch/ia64/xen/mm_init.c
> @@ -38,7 +38,7 @@ ia64_mmu_init (void *my_cpu_data)
> ia64_set_psr(psr);
> ia64_srlz_i();
>  #ifdef XEN
> - cpu_set(cpu, percpu_set);
> + cpumask_set_cpu(cpu, &percpu_set);
>  #endif
>  
> /*
> --- a/xen/arch/ia64/xen/tlb_track.c
> +++ b/xen/arch/ia64/xen/tlb_track.c
> @@ -389,7 +389,7 @@ tlb_track_insert_or_dirty(struct tlb_tra
>  
>   found:
>      BUG_ON(v->processor >= NR_CPUS);
> -    cpu_set(v->processor, entry->pcpu_dirty_mask);
> +    cpumask_set_cpu(v->processor, &entry->pcpu_dirty_mask);
>      BUG_ON(v->vcpu_id >= NR_CPUS);
>      vcpu_set(v->vcpu_id, entry->vcpu_dirty_mask);
>      perfc_incr(tlb_track_iod_dirtied);
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -271,9 +271,9 @@ static void mwait_idle_with_hints(unsign
>       */
>      if ( expires > NOW() || expires == 0 )
>      {
> -        cpu_set(cpu, cpuidle_mwait_flags);
> +        cpumask_set_cpu(cpu, &cpuidle_mwait_flags);
>          __mwait(eax, ecx);
> -        cpu_clear(cpu, cpuidle_mwait_flags);
> +        cpumask_clear_cpu(cpu, &cpuidle_mwait_flags);
>      }
>  
>      if ( expires <= NOW() && expires > 0 )
> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c
> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
> @@ -828,7 +828,7 @@ static void intel_machine_check(struct c
>               * (the MSRs are sticky)
>               */
>              if (bs.pcc || !bs.recoverable)
> -                cpu_set(smp_processor_id(), mce_fatal_cpus);
> +                cpumask_set_cpu(smp_processor_id(), &mce_fatal_cpus);
>          } else {
>              if (mctc != NULL)
>                  mctelem_commit(mctc);
> @@ -849,7 +849,7 @@ static void intel_machine_check(struct c
>  
>      mce_barrier_enter(&mce_trap_bar);
>      if ( mctc != NULL && mce_urgent_action(regs, mctc))
> -        cpu_set(smp_processor_id(), mce_fatal_cpus);
> +        cpumask_set_cpu(smp_processor_id(), &mce_fatal_cpus);
>      mce_barrier_exit(&mce_trap_bar);
>      /*
>       * Wait until everybody has processed the trap.
> --- a/xen/arch/x86/mpparse.c
> +++ b/xen/arch/x86/mpparse.c
> @@ -161,7 +161,7 @@ static int __devinit MP_processor_info_x
> return cpu;
> }
> x86_cpu_to_apicid[cpu] = apicid;
> -  cpu_set(cpu, cpu_present_map);
> +  set_cpu_present(cpu, 1);
> }
>  
> if (++num_processors > 8) {
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -288,7 +288,7 @@ void __init numa_initmem_init(unsigned l
>  
>  __cpuinit void numa_add_cpu(int cpu)
>  {
> - cpu_set(cpu, node_to_cpumask[cpu_to_node(cpu)]);
> + cpumask_set_cpu(cpu, &node_to_cpumask[cpu_to_node(cpu)]);
>  } 
>  
>  void __cpuinit numa_set_node(int cpu, int node)
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -244,7 +244,7 @@ static void set_cpu_sibling_map(int cpu)
>      int i;
>      struct cpuinfo_x86 *c = cpu_data;
>  
> -    cpu_set(cpu, cpu_sibling_setup_map);
> +    cpumask_set_cpu(cpu, &cpu_sibling_setup_map);
>  
>      if ( c[cpu].x86_num_siblings > 1 )
>      {
> @@ -380,7 +380,7 @@ void start_secondary(void *unused)
>       */
>      lock_vector_lock();
>      __setup_vector_irq(cpu);
> -    cpu_set(cpu, cpu_online_map);
> +    set_cpu_online(cpu, 1);
>      unlock_vector_lock();
>  
>      init_percpu_time();
> @@ -804,8 +804,8 @@ void __init smp_prepare_cpus(unsigned in
>  
>  void __init smp_prepare_boot_cpu(void)
>  {
> -    cpu_set(smp_processor_id(), cpu_online_map);
> -    cpu_set(smp_processor_id(), cpu_present_map);
> +    set_cpu_online(smp_processor_id(), 1);
> +    set_cpu_present(smp_processor_id(), 1);
>  }
>  
>  static void
> @@ -933,7 +933,7 @@ int cpu_add(uint32_t apic_id, uint32_t a
>                     "break assumed cross-CPU TSC coherency.\n"
>                     " ** Consider using boot parameter \"tsc=skewed\" "
>                     "which forces TSC emulation where appropriate.\n", cpu);
> -        cpu_set(cpu, tsc_sync_cpu_mask);
> +        cpumask_set_cpu(cpu, &tsc_sync_cpu_mask);
>      }
>  
>      srat_detect_node(cpu);
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1573,7 +1573,7 @@ __initcall(disable_pit_irq);
>  
>  void pit_broadcast_enter(void)
>  {
> -    cpu_set(smp_processor_id(), pit_broadcast_mask);
> +    cpumask_set_cpu(smp_processor_id(), &pit_broadcast_mask);
>  }
>  
>  void pit_broadcast_exit(void)
> --- a/xen/common/cpupool.c
> +++ b/xen/common/cpupool.c
> @@ -253,7 +253,7 @@ static long cpupool_unassign_cpu_helper(
>  
>      spin_lock(&cpupool_lock);
>      ret = cpu_disable_scheduler(cpu);
> -    cpu_set(cpu, cpupool_free_cpus);
> +    cpumask_set_cpu(cpu, &cpupool_free_cpus);
>      if ( !ret )
>      {
>          ret = schedule_cpu_switch(cpu, NULL);
> @@ -409,8 +409,8 @@ void cpupool_rm_domain(struct domain *d)
>  static void cpupool_cpu_add(unsigned int cpu)
>  {
>      spin_lock(&cpupool_lock);
> -    cpu_clear(cpu, cpupool_locked_cpus);
> -    cpu_set(cpu, cpupool_free_cpus);
> +    cpumask_clear_cpu(cpu, &cpupool_locked_cpus);
> +    cpumask_set_cpu(cpu, &cpupool_free_cpus);
>      cpupool_assign_cpu_locked(cpupool0, cpu);
>      spin_unlock(&cpupool_lock);
>  }
> @@ -428,7 +428,7 @@ static int cpupool_cpu_remove(unsigned i
>      if ( !cpumask_test_cpu(cpu, cpupool0->cpu_valid))
>          ret = -EBUSY;
>      else
> -        cpu_set(cpu, cpupool_locked_cpus);
> +        cpumask_set_cpu(cpu, &cpupool_locked_cpus);
>      spin_unlock(&cpupool_lock);
>  
>      return ret;
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -1725,7 +1725,7 @@ csched_schedule(
>      {
>          /* Update the idle mask if necessary */
>          if ( !cpumask_test_cpu(cpu, &rqd->idle) )
> -            cpu_set(cpu, rqd->idle);
> +            cpumask_set_cpu(cpu, &rqd->idle);
>          /* Make sure avgload gets updated periodically even
>           * if there's no activity */
>          update_load(ops, rqd, NULL, 0, now);
> @@ -1860,7 +1860,7 @@ static void activate_runqueue(struct csc
>      INIT_LIST_HEAD(&rqd->runq);
>      spin_lock_init(&rqd->lock);
>  
> -    cpu_set(rqi, prv->active_queues);
> +    cpumask_set_cpu(rqi, &prv->active_queues);
>  }
>  
>  static void deactivate_runqueue(struct csched_private *prv, int rqi)
> @@ -1927,12 +1927,12 @@ static void init_pcpu(const struct sched
>      /* Set the runqueue map */
>      prv->runq_map[cpu]=rqi;
>      
> -    cpu_set(cpu, rqd->idle);
> -    cpu_set(cpu, rqd->active);
> +    cpumask_set_cpu(cpu, &rqd->idle);
> +    cpumask_set_cpu(cpu, &rqd->active);
>  
>      spin_unlock(old_lock);
>  
> -    cpu_set(cpu, prv->initialized);
> +    cpumask_set_cpu(cpu, &prv->initialized);
>  
>      spin_unlock_irqrestore(&prv->lock, flags);
>  
> --- a/xen/include/asm-ia64/linux-xen/asm/acpi.h
> +++ b/xen/include/asm-ia64/linux-xen/asm/acpi.h
> @@ -153,7 +153,7 @@ static inline void per_cpu_scan_finalize
> high_cpu = min(high_cpu + reserve_cpus, NR_CPUS);
>  
> for (cpu = low_cpu; cpu < high_cpu; cpu++) {
> -  cpu_set(cpu, early_cpu_possible_map);
> +  cpumask_set_cpu(cpu, &early_cpu_possible_map);
> if (node_cpuid[cpu].nid == NUMA_NO_NODE) {
> node_cpuid[cpu].nid = next_nid;
> next_nid++;
> --- a/xen/include/xen/cpumask.h
> +++ b/xen/include/xen/cpumask.h
> @@ -97,7 +97,6 @@ static inline unsigned int cpumask_check
> return cpu;
>  }
>  
> -#define cpu_set(cpu, dst) cpumask_set_cpu(cpu, &(dst))
>  static inline void cpumask_set_cpu(int cpu, volatile cpumask_t *dstp)
>  {
> set_bit(cpumask_check(cpu), dstp->bits);
> @@ -452,6 +451,14 @@ extern cpumask_t cpu_present_map;
>  #define cpu_present(cpu) ((cpu) == 0)
>  #endif
>  
> +#define set_cpu_online(cpu, online) \
> +    ((online) ? cpumask_set_cpu(cpu, &cpu_online_map) \
> +              : cpumask_clear_cpu(cpu, &cpu_online_map))
> +
> +#define set_cpu_present(cpu, present) \
> +    ((present) ? cpumask_set_cpu(cpu, &cpu_present_map) \
> +               : cpumask_clear_cpu(cpu, &cpu_present_map))
> +
>  #define any_online_cpu(mask)   \
>  ({      \
> int cpu;    \
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



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