Messages in this thread Patch in this message | | | Subject | Re: [PATCH 3/6] x86/apic: Fix ugly casting and branching in cpu_mask_to_apicid_and() | From | Joe Perches <> | Date | Thu, 14 Jun 2012 06:00:09 -0700 |
| |
On Thu, 2012-06-14 at 09:49 +0200, Alexander Gordeev wrote: [] > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c [] > @@ -2127,19 +2127,19 @@ int default_cpu_mask_to_apicid_and(const struct cpumask *cpumask, > const struct cpumask *andmask, > unsigned int *apicid) > { > - int cpu; > + unsigned int cpu; > > for_each_cpu_and(cpu, cpumask, andmask) { > if (cpumask_test_cpu(cpu, cpu_online_mask)) > break; > } > > - if (likely((unsigned int)cpu < nr_cpu_ids)) { > + if (likely(cpu < nr_cpu_ids)) { > *apicid = per_cpu(x86_cpu_to_apicid, cpu); > return 0; > - } else { > - return -EINVAL; > } > + > + return -EINVAL;
I think you should reverse the test and make the expected common case the normal non-indented return.
if (unlikely(cpu >= nr_cpu_ids)) return -EINVAL;
*apicid = per_cpu(x86_cpu_to_apicid, cpu); return 0; }
Perhaps the unlikely isn't necessary.
> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c [] > @@ -285,12 +285,12 @@ uv_cpu_mask_to_apicid_and(const struct cpumask *cpumask, > break; > } > > - if (likely((unsigned int)cpu < nr_cpu_ids)) { > + if (likely(cpu < nr_cpu_ids)) { > *apicid = per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits; > return 0; > - } else { > - return -EINVAL; > } > + > + return -EINVAL;
here too
| |