Hi Christoph,
Sorry for the delay in looking at this, I've been on holiday for a week.
Comments inline.
On Wed, Aug 28, 2013 at 08:48:23PM +0100, Christoph Lameter wrote:
> Transformations done to __get_cpu_var()
>
>
> 1. Determine the address of the percpu instance of the current processor.
>
> DEFINE_PER_CPU(int, y);
> int *x = &__get_cpu_var(y);
>
> Converts to
>
> int *x = this_cpu_ptr(&y);
>
>
> 2. Same as #1 but this time an array structure is involved.
>
> DEFINE_PER_CPU(int, y[20]);
> int *x = __get_cpu_var(y);
>
> Converts to
>
> int *x = this_cpu_ptr(y);
This is the flavour we have for ARM's hw_breakpoint code, where we have an
array of perf_event * instead of int...
> Index: linux/arch/arm/kernel/hw_breakpoint.c
> ===================================================================
> --- linux.orig/arch/arm/kernel/hw_breakpoint.c 2013-08-26 13:48:40.956794980 -0500
> +++ linux/arch/arm/kernel/hw_breakpoint.c 2013-08-26 13:48:40.952795024 -0500
> @@ -344,13 +344,13 @@ int arch_install_hw_breakpoint(struct pe
> /* Breakpoint */
> ctrl_base = ARM_BASE_BCR;
> val_base = ARM_BASE_BVR;
> - slots = (struct perf_event **)__get_cpu_var(bp_on_reg);
> + slots = (struct perf_event **)__this_cpu_read(bp_on_reg);
...so I don't think this is quite right, and indeed, we get a bunch of errors
from GCC:
arch/arm/kernel/hw_breakpoint.c: In function ‘arch_install_hw_breakpoint’:
arch/arm/kernel/hw_breakpoint.c:347:33: error: incompatible types when assigning to type ‘struct perf_event *[16]’ from type ‘struct perf_event **’
arch/arm/kernel/hw_breakpoint.c:347:1: error: incompatible types when assigning to type ‘struct perf_event *[16]’ from type ‘struct perf_event **’
arch/arm/kernel/hw_breakpoint.c:347:1: error: incompatible types when assigning to type ‘struct perf_event *[16]’ from type ‘struct perf_event **’
arch/arm/kernel/hw_breakpoint.c:347:1: error: incompatible types when assigning to type ‘struct perf_event *[16]’ from type ‘struct perf_event **’
changing to match your recipe still doesn't work, however:
arch/arm/kernel/hw_breakpoint.c: In function ‘arch_install_hw_breakpoint’:
arch/arm/kernel/hw_breakpoint.c:347:33: error: cast specifies array type
> Index: linux/arch/arm64/kernel/debug-monitors.c
> ===================================================================
> --- linux.orig/arch/arm64/kernel/debug-monitors.c 2013-08-26 13:48:40.956794980 -0500
> +++ linux/arch/arm64/kernel/debug-monitors.c 2013-08-26 13:48:40.952795024 -0500
> @@ -98,11 +98,11 @@ void enable_debug_monitors(enum debug_el
>
> WARN_ON(preemptible());
>
> - if (local_inc_return(&__get_cpu_var(mde_ref_count)) == 1)
> + if (this_cpu_inc_return(mde_ref_count) == 1)
> enable = DBG_MDSCR_MDE;
I'm not sure that this is safe. We rely on local_inc_return to be atomic
with respect to the current CPU, which will end up being a wrapper around
atomic64_inc_return. However, this_cpu_inc_return simply uses a lock, so
other people accessing the count in a different manner (local_dec_and_test
below) may break local atomicity unless we start disabling interrupts or
something horrible like that.
> if (el == DBG_ACTIVE_EL1 &&
> - local_inc_return(&__get_cpu_var(kde_ref_count)) == 1)
> + this_cpu_inc_return(kde_ref_count) == 1)
> enable |= DBG_MDSCR_KDE;
>
> if (enable && debug_enabled) {
> @@ -118,11 +118,11 @@ void disable_debug_monitors(enum debug_e
>
> WARN_ON(preemptible());
>
> - if (local_dec_and_test(&__get_cpu_var(mde_ref_count)))
> + if (local_dec_and_test(this_cpu_ptr(&mde_ref_count)))
> disable = ~DBG_MDSCR_MDE;
>
> if (el == DBG_ACTIVE_EL1 &&
> - local_dec_and_test(&__get_cpu_var(kde_ref_count)))
> + local_dec_and_test(this_cpu_ptr(&kde_ref_count)))
> disable &= ~DBG_MDSCR_KDE;
>
> if (disable) {
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
