Re: [PATCH 2/3] percpu counter: cast this_cpu_sub() adjustment

From: Tejun Heo
Date: Sun Oct 27 2013 - 07:23:07 EST


On Sun, Oct 27, 2013 at 12:44:35AM -0700, Greg Thelen wrote:
> this_cpu_sub() is implemented as negation and addition.
>
> This patch casts the adjustment to the counter type before negation to
> sign extend the adjustment. This helps in cases where the counter
> type is wider than an unsigned adjustment. An alternative to this
> patch is to declare such operations unsupported, but it seemed useful
> to avoid surprises.
>
> This patch specifically helps the following example:
> unsigned int delta = 1
> preempt_disable()
> this_cpu_write(long_counter, 0)
> this_cpu_sub(long_counter, delta)
> preempt_enable()
>
> Before this change long_counter on a 64 bit machine ends with value
> 0xffffffff, rather than 0xffffffffffffffff. This is because
> this_cpu_sub(pcp, delta) boils down to this_cpu_add(pcp, -delta),
> which is basically:
> long_counter = 0 + 0xffffffff
>
> Also apply the same cast to:
> __this_cpu_sub()
> this_cpu_sub_return()
> and __this_cpu_sub_return()
>
> All percpu_test.ko passes, especially the following cases which
> previously failed:
>
> l -= ui_one;
> __this_cpu_sub(long_counter, ui_one);
> CHECK(l, long_counter, -1);
>
> l -= ui_one;
> this_cpu_sub(long_counter, ui_one);
> CHECK(l, long_counter, -1);
> CHECK(l, long_counter, 0xffffffffffffffff);
>
> ul -= ui_one;
> __this_cpu_sub(ulong_counter, ui_one);
> CHECK(ul, ulong_counter, -1);
> CHECK(ul, ulong_counter, 0xffffffffffffffff);
>
> ul = this_cpu_sub_return(ulong_counter, ui_one);
> CHECK(ul, ulong_counter, 2);
>
> ul = __this_cpu_sub_return(ulong_counter, ui_one);
> CHECK(ul, ulong_counter, 1);
>
> Signed-off-by: Greg Thelen <gthelen@xxxxxxxxxx>

Ouch, nice catch.

Acked-by: Tejun Heo <tj@xxxxxxxxxx>

We probably want to cc stable for this and the next one. How should
these be routed? I can take these through percpu tree or mm works
too. Either way, it'd be best to route them together.

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/