[ovs-dev] [PATCH v4 03/12] classifier: Support table versioning

[Ben Pfaff]

On Wed, Jun 10, 2015 at 03:51:05PM -0700, Ben Pfaff wrote:
> On Tue, Jun 09, 2015 at 05:24:10PM -0700, Jarno Rajahalme wrote:
> > This patch allows classifier rules to become visible and invisible in
> > specific versions.  A 'version' is defined as a positive monotonically
> > increasing integer, which never wraps around.
> > 
> > The new 'visibility' attribute replaces the prior 'to_be_removed' and
> > 'visible' attributes.
> > 
> > When versioning is not used, the 'version' parameter should be passed
> > as 'CLS_MIN_VERSION' when creating rules, and 'CLS_MAX_VERSION' when
> > looking up flows.
> > 
> > This feature enables the support for atomic OpenFlow bundles without
> > significant performance penalty on 64-bit systems. There is a
> > performance decrease in 32-bit systems due to 64-bit atomics used.
> > 
> > Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> The two comments
> 
>     /* clang does not want to read from a const atomic. */
> 
> stand out to me a little, because they imply that this is a special
> property of Clang.  I don't think that's true; rather, it's a
> consequence of the C standard definition, which defines atomic "load"
> operations as taking plain types, not const-qualified ones.  I think
> that goes back, historically, to the definition of a "side effect" in C:
> 
>     Accessing a volatile object, modifying an object, modifying a file,
>     or calling a function that does any of those operations are all side
>     effects, which are changes in the state of the execution
>     environment.
> 
> That is, *any* access to a volatile object (not just a write) is a side
> effect (and thus cannot be optimized out by the compiler).  I get the
> impression that a lot of behavior for atomic types in C11 is related to
> volatile types.
> 
> Anyway, personally I'd either drop the comments or just s/clang/C11/.
> 
> Acked-by: Ben Pfaff <blp at nicira.com>

This patch adds an XXX, is that removed in some later patch?