Re: [Xen-devel] [PATCH v3 08/10] xen/nodemask: Introduce unlocked __nodemask_{set, clear}() helpers

[Jan Beulich <JBeulich@xxxxxxxx>]

On 30.07.2019 19:48, Andrew Cooper wrote:
> On 30/07/2019 12:26, Jan Beulich wrote:
>> On 29.07.2019 14:12, Andrew Cooper wrote:
>>> --- a/xen/include/xen/nodemask.h
>>> +++ b/xen/include/xen/nodemask.h
>>> @@ -11,7 +11,9 @@
>>>    * The available nodemask operations are:
>>>    *
>>>    * void node_set(node, mask)              turn on bit 'node' in mask
>>> + * void __nodemask_set(node, mask) turn on bit 'node' in mask (unlocked)
>>>    * void node_clear(node, mask)            turn off bit 'node' in mask
>>> + * void __nodemask_clear(node, mask)       turn off bit 'node' in mask 
>>> (unlocked)
>> To be honest I'm unhappy to see you introduce new name space
>> violations. I realize you want to have the node mask interfaces
>> match the CPU mask one as closely as possible, but I think we
>> should diverge here (and eventually make the CPU mask ones
>> follow whatever route we go here). As to naming, since these
>> are static inlines, a single leading underscore may be an
>> option (albeit I'd prefer to avoid this). Another option would
>> be to have double infix underscores (nodemask__set()). Yet
>> another option would be to express the non-atomicity in a
>> verbal way rather than by the number of underscores used. I'm
>> afraid I don't have a good naming suggestion in that case,
>> though.
> 
> I'm really not as concerned with namespace violations as you seem to
> be.  We are not a library having to cooperate with arbitrary others - we
> are a freestanding build with the compiler being the only other source
> of symbols in the namespace.  Our chances of colliding are very low to
> begin with, and fixing conflicts if they arise is trivial.

Trivial or not, such conflicts are problematic for branches in
security only maintenance mode. While we _can_ backport such
patches there, we really _shouldn't_.

> As you note, there is nothing obvious as an alternative.  The double
> infix is a no-go as far as I'm concerned - it's far too subtle in the code.

Well, the single-underscore-prefix variant is at least better
than the double one, even if still not ideal. I'd prefer if
you switched, but I don't mean to stand in the way of this
going in, so
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
even in its current shape.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel