mixture of atomic and non-atomic operations on CPU masks

mixture of atomic and non-atomic operations on CPU masks

[Jan Beulich]

All,

the inconsistency here has been puzzling me for a while, and I think
we ought to do something about it (perhaps not for 4.5 anymore,
but then right after): cpumask_(test_and_)?(set|clear)_cpu all use
the respective atomic bitops, in contrast to all other operations on
cpumask_t. A good part of the users don't require the atomicity at
all (in particular any of those acting on function scope variables).
Does anyone know of reasons why the default shouldn't be non-
atomic ops across the board, with atomic special cases being made
available for the few cases where they're actually needed?

Jan

Re: mixture of atomic and non-atomic operations on CPU masks

[Ian Campbell]

On Wed, 2014-09-10 at 07:46 +0100, Jan Beulich wrote:
> the inconsistency here has been puzzling me for a while, and I think
> we ought to do something about it (perhaps not for 4.5 anymore,
> but then right after): cpumask_(test_and_)?(set|clear)_cpu all use
> the respective atomic bitops, in contrast to all other operations on
> cpumask_t.

Did that just happen by accident because the bitops are sync by default
with a non-sync variant?

>  A good part of the users don't require the atomicity at
> all (in particular any of those acting on function scope variables).
> Does anyone know of reasons why the default shouldn't be non-
> atomic ops across the board, with atomic special cases being made
> available for the few cases where they're actually needed?

I'm fine with that.

Ian.

Re: mixture of atomic and non-atomic operations on CPU masks

[Jan Beulich]

>>> On 10.09.14 at 12:03, <Ian.Campbell@eu.citrix.com> wrote:
> On Wed, 2014-09-10 at 07:46 +0100, Jan Beulich wrote:
>> the inconsistency here has been puzzling me for a while, and I think
>> we ought to do something about it (perhaps not for 4.5 anymore,
>> but then right after): cpumask_(test_and_)?(set|clear)_cpu all use
>> the respective atomic bitops, in contrast to all other operations on
>> cpumask_t.
> 
> Did that just happen by accident because the bitops are sync by default
> with a non-sync variant?

I'm afraid not - in some cases the 1-bit manipulation functions are
indeed expected to be usable in racy situations. I.e. I think Linux
(from where we got all this) has this deliberately this way, not
paying attention to the inconsistency and the often pointless
extra overhead.

Jan

Re: mixture of atomic and non-atomic operations on CPU masks

[Andrew Cooper]

On 10/09/14 07:46, Jan Beulich wrote:
> All,
>
> the inconsistency here has been puzzling me for a while, and I think
> we ought to do something about it (perhaps not for 4.5 anymore,
> but then right after): cpumask_(test_and_)?(set|clear)_cpu all use
> the respective atomic bitops, in contrast to all other operations on
> cpumask_t. A good part of the users don't require the atomicity at
> all (in particular any of those acting on function scope variables).
> Does anyone know of reasons why the default shouldn't be non-
> atomic ops across the board, with atomic special cases being made
> available for the few cases where they're actually needed?

Its entirely possible that the first users of test_and_? required
atomicity, and the first users which didn't require atomicity copied the
atomic ops as they already existed.

Defaulting to non-atomic seems reasonable, as I suspect that there are
fairly few cases where atomicity is required.  (At least atomic
operations on the local stack is not especially bad to other cpus; I
really hope other cpus don't have writable cache lines of our stack.)

~Andrew