Re: [PATCH] bitops: add _local bitops

Michael S. Tsirkin
2012-05-09T16:55:41Z

On Wed, May 09, 2012 at 09:45:57AM -0700, H. Peter Anvin wrote:
> On 05/09/2012 09:36 AM, Michael S. Tsirkin wrote:
> >
> > Well it talks about a memory barrier, not an
> > optimization barrier.
> >
>
> Same thing.

I see. So it really should say 'any barrier', right?
Documentation/atomic_ops.txt goes to great length
to distinguish between the two and we probably
should not confuse things.

> > If compiler reorders code, changes will appear in
> > the wrong order on the current processor,
> > not just on other processors, no?
>
> Yes.

So this seems to contradict what the comment says:

clear_bit() is atomic and may not be reordered.
and you say compiler *can* reorder it, and below

you should call smp_mb__before_clear_bit() and/or * smp_mb__after_clear_bit()
in order to ensure changes are visible on other processors.

and in fact this is not enough, you also need to call
barrier() to ensure changes are visible on the same
processor in the correct order.

> For your _local I would just copy the atomic bitops but remote the locks
> in most cases.
>
> -hpa

Right, I sent v2 that does exactly this.

My question about documentation for change_bit
is an unrelated one: to me, it looks like the documentation for
change_bit does not match the implementation, or at least is somewhat
confusing.

> --
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel. I don't speak on their behalf.