Jan Beulich
2008-Jan-18 16:06 UTC
[Xen-devel] [PATCH] linux/x86: convert ''cpu'' (and a few other) variables to unsigned
.. producing better code for x86-64.
As usual, written and tested on 2.6.24-rc7 and made apply to the 2.6.18
tree without further testing.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Index: head-2007-12-11/arch/i386/kernel/time-xen.c
==================================================================---
head-2007-12-11.orig/arch/i386/kernel/time-xen.c 2007-12-18 15:28:14.000000000
+0100
+++ head-2007-12-11/arch/i386/kernel/time-xen.c 2007-12-18 13:39:39.000000000
+0100
@@ -301,7 +301,7 @@ static void update_wallclock(void)
* Reads a consistent set of time-base values from Xen, into a shadow data
* area.
*/
-static void get_time_values_from_xen(int cpu)
+static void get_time_values_from_xen(unsigned int cpu)
{
struct vcpu_time_info *src;
struct shadow_time_info *dst;
@@ -322,7 +322,7 @@ static void get_time_values_from_xen(int
dst->tsc_to_usec_mul = dst->tsc_to_nsec_mul / 1000;
}
-static inline int time_values_up_to_date(int cpu)
+static inline int time_values_up_to_date(unsigned int cpu)
{
struct vcpu_time_info *src;
struct shadow_time_info *dst;
@@ -513,7 +513,7 @@ static int set_rtc_mmss(unsigned long no
*/
unsigned long long monotonic_clock(void)
{
- int cpu = get_cpu();
+ unsigned int cpu = get_cpu();
struct shadow_time_info *shadow = &per_cpu(shadow_time, cpu);
u64 time;
u32 local_time_version;
@@ -579,7 +579,7 @@ irqreturn_t timer_interrupt(
{
s64 delta, delta_cpu, stolen, blocked;
u64 sched_time;
- int i, cpu = smp_processor_id();
+ unsigned int i, cpu = smp_processor_id();
struct shadow_time_info *shadow = &per_cpu(shadow_time, cpu);
struct vcpu_runstate_info *runstate = &per_cpu(runstate, cpu);
@@ -614,7 +614,7 @@ irqreturn_t timer_interrupt(int irq, voi
if ((unlikely(delta < -(s64)permitted_clock_jitter) ||
unlikely(delta_cpu < -(s64)permitted_clock_jitter))
&& printk_ratelimit()) {
- printk("Timer ISR/%d: Time went backwards: "
+ printk("Timer ISR/%u: Time went backwards: "
"delta=%lld delta_cpu=%lld shadow=%lld "
"off=%lld processed=%lld cpu_processed=%lld\n",
cpu, delta, delta_cpu, shadow->system_timestamp,
@@ -717,7 +717,7 @@ static struct clocksource clocksource_xe
return IRQ_HANDLED;
}
-static void init_missing_ticks_accounting(int cpu)
+static void init_missing_ticks_accounting(unsigned int cpu)
{
struct vcpu_register_runstate_memory_area area;
struct vcpu_runstate_info *runstate = &per_cpu(runstate, cpu);
@@ -965,7 +965,7 @@ int __cpuinit local_setup_timer(unsigned
init_missing_ticks_accounting(cpu);
} while (read_seqretry(&xtime_lock, seq));
- sprintf(timer_name[cpu], "timer%d", cpu);
+ sprintf(timer_name[cpu], "timer%u", cpu);
irq = bind_virq_to_irqhandler(VIRQ_TIMER,
cpu,
timer_interrupt,
Index: head-2007-12-11/arch/i386/mm/hypervisor.c
==================================================================---
head-2007-12-11.orig/arch/i386/mm/hypervisor.c 2007-10-19 17:18:37.000000000
+0200
+++ head-2007-12-11/arch/i386/mm/hypervisor.c 2007-12-18 15:34:13.000000000
+0100
@@ -250,9 +250,9 @@ int xen_create_contiguous_region(
unsigned long vstart, unsigned int order, unsigned int address_bits)
{
unsigned long *in_frames = discontig_frames, out_frame;
- unsigned long frame, i, flags;
- long rc;
- int success;
+ unsigned long frame, flags;
+ unsigned int i;
+ int rc, success;
struct xen_memory_exchange exchange = {
.in = {
.nr_extents = 1UL << order,
@@ -286,7 +286,7 @@ int xen_create_contiguous_region(
balloon_lock(flags);
/* 1. Zap current PTEs, remembering MFNs. */
- for (i = 0; i < (1UL<<order); i++) {
+ for (i = 0; i < (1U<<order); i++) {
in_frames[i] = pfn_to_mfn((__pa(vstart) >> PAGE_SHIFT) + i);
MULTI_update_va_mapping(cr_mcl + i, vstart + (i*PAGE_SIZE),
__pte_ma(0), 0);
@@ -312,7 +312,7 @@ int xen_create_contiguous_region(
&exchange.out) == 1);
if (!success) {
/* Couldn''t get special memory: fall back to normal. */
- for (i = 0; i < (1UL<<order); i++)
+ for (i = 0; i < (1U<<order); i++)
in_frames[i] = (__pa(vstart)>>PAGE_SHIFT) + i;
if (HYPERVISOR_memory_op(XENMEM_populate_physmap,
&exchange.in) != (1UL<<order))
@@ -322,7 +322,7 @@ int xen_create_contiguous_region(
#endif
/* 3. Map the new extent in place of old pages. */
- for (i = 0; i < (1UL<<order); i++) {
+ for (i = 0; i < (1U<<order); i++) {
frame = success ? (out_frame + i) : in_frames[i];
MULTI_update_va_mapping(cr_mcl + i, vstart + (i*PAGE_SIZE),
pfn_pte_ma(frame, PAGE_KERNEL), 0);
@@ -348,9 +348,9 @@ EXPORT_SYMBOL_GPL(xen_create_contiguous_
void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
{
unsigned long *out_frames = discontig_frames, in_frame;
- unsigned long frame, i, flags;
- long rc;
- int success;
+ unsigned long frame, flags;
+ unsigned int i;
+ int rc, success;
struct xen_memory_exchange exchange = {
.in = {
.nr_extents = 1,
@@ -384,7 +384,7 @@ void xen_destroy_contiguous_region(unsig
in_frame = pfn_to_mfn(__pa(vstart) >> PAGE_SHIFT);
/* 2. Zap current PTEs. */
- for (i = 0; i < (1UL<<order); i++) {
+ for (i = 0; i < (1U<<order); i++) {
MULTI_update_va_mapping(cr_mcl + i, vstart + (i*PAGE_SIZE),
__pte_ma(0), 0);
set_phys_to_machine((__pa(vstart)>>PAGE_SHIFT)+i,
@@ -413,7 +413,7 @@ void xen_destroy_contiguous_region(unsig
#endif
/* 4. Map new pages in place of old pages. */
- for (i = 0; i < (1UL<<order); i++) {
+ for (i = 0; i < (1U<<order); i++) {
frame = success ? out_frames[i] : (in_frame + i);
MULTI_update_va_mapping(cr_mcl + i, vstart + (i*PAGE_SIZE),
pfn_pte_ma(frame, PAGE_KERNEL), 0);
@@ -436,7 +436,8 @@ int xen_limit_pages_to_max_mfn(
unsigned long *in_frames = discontig_frames, *out_frames = limited_frames;
void *v;
struct page *page;
- int i, nr_mcl, rc, success;
+ unsigned int i, nr_mcl;
+ int rc, success;
struct xen_memory_exchange exchange = {
.in = {
@@ -481,7 +482,7 @@ int xen_limit_pages_to_max_mfn(
balloon_lock(flags);
/* 1. Zap current PTEs (if any), remembering MFNs. */
- for (i = 0, nr_mcl = 0; i < (1UL<<order); i++) {
+ for (i = 0, nr_mcl = 0; i < (1U<<order); i++) {
page = &pages[i];
out_frames[i] = page_to_pfn(page);
@@ -514,7 +515,7 @@ int xen_limit_pages_to_max_mfn(
#endif
/* 3. Map the new pages in place of old pages. */
- for (i = 0, nr_mcl = 0; i < (1UL<<order); i++) {
+ for (i = 0, nr_mcl = 0; i < (1U<<order); i++) {
unsigned long pfn;
page = &pages[i];
pfn = page_to_pfn(page);
Index: head-2007-12-11/include/asm-i386/mach-xen/asm/hypercall.h
==================================================================---
head-2007-12-11.orig/include/asm-i386/mach-xen/asm/hypercall.h 2007-12-18
15:28:14.000000000 +0100
+++ head-2007-12-11/include/asm-i386/mach-xen/asm/hypercall.h 2007-12-11
11:45:14.000000000 +0100
@@ -343,7 +343,7 @@ HYPERVISOR_vm_assist(
static inline int
HYPERVISOR_vcpu_op(
- int cmd, int vcpuid, void *extra_args)
+ int cmd, unsigned int vcpuid, void *extra_args)
{
return _hypercall3(int, vcpu_op, cmd, vcpuid, extra_args);
}
Index: head-2007-12-11/include/asm-x86_64/mach-xen/asm/hypercall.h
==================================================================---
head-2007-12-11.orig/include/asm-x86_64/mach-xen/asm/hypercall.h 2007-12-18
15:28:14.000000000 +0100
+++ head-2007-12-11/include/asm-x86_64/mach-xen/asm/hypercall.h 2007-12-11
11:45:14.000000000 +0100
@@ -337,7 +337,7 @@ HYPERVISOR_vm_assist(
static inline int
HYPERVISOR_vcpu_op(
- int cmd, int vcpuid, void *extra_args)
+ int cmd, unsigned int vcpuid, void *extra_args)
{
return _hypercall3(int, vcpu_op, cmd, vcpuid, extra_args);
}
Index: head-2007-12-11/drivers/xen/core/cpu_hotplug.c
==================================================================---
head-2007-12-11.orig/drivers/xen/core/cpu_hotplug.c 2007-12-18
15:28:14.000000000 +0100
+++ head-2007-12-11/drivers/xen/core/cpu_hotplug.c 2007-12-18 15:14:20.000000000
+0100
@@ -32,7 +32,7 @@ static void vcpu_hotplug(unsigned int cp
if ((cpu >= NR_CPUS) || !cpu_possible(cpu))
return;
- sprintf(dir, "cpu/%d", cpu);
+ sprintf(dir, "cpu/%u", cpu);
err = xenbus_scanf(XBT_NIL, dir, "availability", "%s",
state);
if (err != 1) {
printk(KERN_ERR "XENBUS: Unable to read cpu state\n");
@@ -54,12 +54,12 @@ static void vcpu_hotplug(unsigned int cp
static void handle_vcpu_hotplug_event(
struct xenbus_watch *watch, const char **vec, unsigned int len)
{
- int cpu;
+ unsigned int cpu;
char *cpustr;
const char *node = vec[XS_WATCH_PATH];
if ((cpustr = strstr(node, "cpu/")) != NULL) {
- sscanf(cpustr, "cpu/%d", &cpu);
+ sscanf(cpustr, "cpu/%u", &cpu);
vcpu_hotplug(cpu);
}
}
@@ -67,7 +67,7 @@ static void handle_vcpu_hotplug_event(
static int smpboot_cpu_notify(struct notifier_block *notifier,
unsigned long action, void *hcpu)
{
- int cpu = (long)hcpu;
+ unsigned int cpu = (long)hcpu;
/*
* We do this in a callback notifier rather than __cpu_disable()
@@ -83,7 +83,7 @@ static int smpboot_cpu_notify(struct not
static int setup_cpu_watcher(struct notifier_block *notifier,
unsigned long event, void *data)
{
- int i;
+ unsigned int i;
static struct xenbus_watch cpu_watch = {
.node = "cpu",
@@ -121,7 +121,8 @@ arch_initcall(setup_vcpu_hotplug_event);
int smp_suspend(void)
{
- int cpu, err;
+ unsigned int cpu;
+ int err;
for_each_online_cpu(cpu) {
if (cpu == 0)
@@ -141,7 +142,7 @@ int smp_suspend(void)
void smp_resume(void)
{
- int cpu;
+ unsigned int cpu;
for_each_possible_cpu(cpu)
vcpu_hotplug(cpu);
Index: head-2007-12-11/drivers/xen/core/evtchn.c
==================================================================---
head-2007-12-11.orig/drivers/xen/core/evtchn.c 2007-12-18 15:28:14.000000000
+0100
+++ head-2007-12-11/drivers/xen/core/evtchn.c 2007-12-18 15:11:34.000000000
+0100
@@ -230,7 +230,8 @@ asmlinkage void evtchn_do_upcall(struct
unsigned long l1, l2;
unsigned long masked_l1, masked_l2;
unsigned int l1i, l2i, port, count;
- int irq, cpu = smp_processor_id();
+ int irq;
+ unsigned int cpu = smp_processor_id();
shared_info_t *s = HYPERVISOR_shared_info;
vcpu_info_t *vcpu_info = &s->vcpu_info[cpu];
@@ -466,7 +467,8 @@ static int bind_ipi_to_irq(unsigned int
static void unbind_from_irq(unsigned int irq)
{
struct evtchn_close close;
- int cpu, evtchn = evtchn_from_irq(irq);
+ unsigned int cpu;
+ int evtchn = evtchn_from_irq(irq);
spin_lock(&irq_mapping_update_lock);
@@ -927,7 +929,7 @@ void disable_all_local_evtchn(void)
synch_set_bit(i, &s->evtchn_mask[0]);
}
-static void restore_cpu_virqs(int cpu)
+static void restore_cpu_virqs(unsigned int cpu)
{
struct evtchn_bind_virq bind_virq;
int virq, irq, evtchn;
@@ -956,7 +958,7 @@ static void restore_cpu_virqs(int cpu)
}
}
-static void restore_cpu_ipis(int cpu)
+static void restore_cpu_ipis(unsigned int cpu)
{
struct evtchn_bind_ipi bind_ipi;
int ipi, irq, evtchn;
@@ -987,7 +989,7 @@ static void restore_cpu_ipis(int cpu)
void irq_resume(void)
{
- int cpu, pirq, irq, evtchn;
+ unsigned int cpu, pirq, irq, evtchn;
init_evtchn_cpu_bindings();
@@ -1014,7 +1016,7 @@ void irq_resume(void)
void __init xen_init_IRQ(void)
{
- int i;
+ unsigned int i;
init_evtchn_cpu_bindings();
Index: head-2007-12-11/drivers/xen/core/smpboot.c
==================================================================---
head-2007-12-11.orig/drivers/xen/core/smpboot.c 2007-12-18 15:28:14.000000000
+0100
+++ head-2007-12-11/drivers/xen/core/smpboot.c 2007-12-18 12:10:12.000000000
+0100
@@ -88,7 +88,7 @@ void __init smp_alloc_memory(void)
}
static inline void
-set_cpu_sibling_map(int cpu)
+set_cpu_sibling_map(unsigned int cpu)
{
cpu_data[cpu].phys_proc_id = cpu;
cpu_data[cpu].cpu_core_id = 0;
@@ -100,7 +100,7 @@ set_cpu_sibling_map(int cpu)
}
static void
-remove_siblinginfo(int cpu)
+remove_siblinginfo(unsigned int cpu)
{
cpu_data[cpu].phys_proc_id = BAD_APICID;
cpu_data[cpu].cpu_core_id = BAD_APICID;
@@ -117,7 +117,7 @@ static int __cpuinit xen_smp_intr_init(u
per_cpu(resched_irq, cpu) = per_cpu(callfunc_irq, cpu) = -1;
- sprintf(resched_name[cpu], "resched%d", cpu);
+ sprintf(resched_name[cpu], "resched%u", cpu);
rc = bind_ipi_to_irqhandler(RESCHEDULE_VECTOR,
cpu,
smp_reschedule_interrupt,
@@ -128,7 +128,7 @@ static int __cpuinit xen_smp_intr_init(u
goto fail;
per_cpu(resched_irq, cpu) = rc;
- sprintf(callfunc_name[cpu], "callfunc%d", cpu);
+ sprintf(callfunc_name[cpu], "callfunc%u", cpu);
rc = bind_ipi_to_irqhandler(CALL_FUNCTION_VECTOR,
cpu,
smp_call_function_interrupt,
@@ -248,7 +248,7 @@ static void __cpuinit cpu_initialize_con
void __init smp_prepare_cpus(unsigned int max_cpus)
{
- int cpu;
+ unsigned int cpu;
struct task_struct *idle;
#ifdef __x86_64__
struct desc_ptr *gdt_descr;
@@ -369,7 +369,7 @@ core_initcall(initialize_cpu_present_map
int __cpu_disable(void)
{
cpumask_t map = cpu_online_map;
- int cpu = smp_processor_id();
+ unsigned int cpu = smp_processor_id();
if (cpu == 0)
return -EBUSY;
Index: head-2007-12-11/drivers/xen/xenoprof/xenoprofile.c
==================================================================---
head-2007-12-11.orig/drivers/xen/xenoprof/xenoprofile.c 2007-12-18
15:28:14.000000000 +0100
+++ head-2007-12-11/drivers/xen/xenoprof/xenoprofile.c 2007-12-18
15:06:08.000000000 +0100
@@ -198,11 +198,9 @@ static irqreturn_t
xenoprof_ovf_interrupt(int irq, void * dev_id)
{
struct xenoprof_buf * buf;
- int cpu;
static unsigned long flag;
- cpu = smp_processor_id();
- buf = xenoprof_buf[cpu];
+ buf = xenoprof_buf[smp_processor_id()];
xenoprof_add_pc(buf, 0);
@@ -218,7 +216,7 @@ xenoprof_ovf_interrupt(int irq, void * d
static void unbind_virq(void)
{
- int i;
+ unsigned int i;
for_each_online_cpu(i) {
if (ovf_irq[i] >= 0) {
@@ -231,7 +229,8 @@ static void unbind_virq(void)
static int bind_virq(void)
{
- int i, result;
+ unsigned int i;
+ int result;
for_each_online_cpu(i) {
result = bind_virq_to_irqhandler(VIRQ_XENOPROF,
@@ -429,7 +428,7 @@ static int xenoprof_set_passive(int * p_
unsigned int pdoms)
{
int ret;
- int i, j;
+ unsigned int i, j;
struct xenoprof_buf *buf;
if (!xenoprof_is_primary)
@@ -503,7 +502,8 @@ static int using_xenoprof;
int __init xenoprofile_init(struct oprofile_operations * ops)
{
struct xenoprof_init init;
- int ret, i;
+ unsigned int i;
+ int ret;
ret = HYPERVISOR_xenoprof_op(XENOPROF_init, &init);
if (!ret) {
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2008-Jan-18 16:16 UTC
Re: [Xen-devel] [PATCH] linux/x86: convert ''cpu'' (and a few other) variables to unsigned
Jan Beulich writes ("[Xen-devel] [PATCH] linux/x86: convert
''cpu'' (and a few other) variables to
unsigned"):> .. producing better code for x86-64.
>
> As usual, written and tested on 2.6.24-rc7 and made apply to the 2.6.18
> tree without further testing.
In what way does this produce better code for x86-64 ?
Normally it''s IMO best practice to avoid `unsigned'' other than
for
bitfields, clock arithmetic, and the like, because the behaviour of
unsigned in the presence of subtraction and comparision is often
surprising.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jan Beulich
2008-Jan-18 16:27 UTC
Re: [Xen-devel] [PATCH] linux/x86: convert ''cpu'' (and a few other)variables to unsigned
>>> Ian Jackson <Ian.Jackson@eu.citrix.com> 18.01.08 17:16 >>> >Jan Beulich writes ("[Xen-devel] [PATCH] linux/x86: convert ''cpu'' (and a few other) variables to unsigned"): >> .. producing better code for x86-64. >> >> As usual, written and tested on 2.6.24-rc7 and made apply to the 2.6.18 >> tree without further testing. > >In what way does this produce better code for x86-64 ?When signed int is used as array index or in pointer arithmetic, the compiler has to insert an explicit, unconditional sign extension operation, whereas in many cases zero extension is implicit by preceding operations or can be done with a 32-bit move (which doesn''t require a REX prefix and thus results in [statisitically] half a byte shorter instructions).>Normally it''s IMO best practice to avoid `unsigned'' other than for >bitfields, clock arithmetic, and the like, because the behaviour of >unsigned in the presence of subtraction and comparision is often >surprising.I would disagree here, but it''s certainly a personal thing what''s considered surprising. Regardless of this personal aspect, the x86 Linux maintainers are actively doing the same thing (or are respectively asking for it to be done). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2008-Jan-18 16:36 UTC
Re: [Xen-devel] [PATCH] linux/x86: convert ''cpu'' (and a few other)variables to unsigned
Jan Beulich writes ("Re: [Xen-devel] [PATCH] linux/x86: convert
''cpu'' (and a few other)variables to
unsigned"):> When signed int is used as array index or in pointer arithmetic, the
> compiler has to insert an explicit, unconditional sign extension
> operation, whereas in many cases zero extension is implicit by
> preceding operations or can be done with a 32-bit move (which
> doesn''t require a REX prefix and thus results in [statisitically]
> half a byte shorter instructions).
Oh dear. That really is very unfortunate.
> >Normally it''s IMO best practice to avoid `unsigned''
other than for
> >bitfields, clock arithmetic, and the like, because the behaviour of
> >unsigned in the presence of subtraction and comparision is often
> >surprising.
>
> I would disagree here, but it''s certainly a personal thing
what''s
> considered surprising.
Indeed.
> Regardless of this personal aspect, the x86 Linux maintainers are
> actively doing the same thing (or are respectively asking for it to
> be done).
I suppose so.
I''m just not looking forward to all of the bugs of the form
if (cpus-1 < some_number_of_cpus) ...
and the like.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel