Re: [PATCH] bitops: add _local bitops

From: Michael S. Tsirkin
Date: Wed May 09 2012 - 12:55:40 EST


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.
--
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/