Re: [PATCHv5 2/2] memory barrier: adding smp_mb__after_lock

[Oleg Nesterov <oleg@redhat.com>]

On 07/07, Mathieu Desnoyers wrote:
>
> * Eric Dumazet (eric.dumazet@gmail.com) wrote:
> >
> > What would be __read_lock() ? I cant see how it could *not* use lock prefix
> > actually and or being cheaper...
> >
>
> (I'll use read_lock_noacquire() instead of __read_lock() because
> __read_lock() is already used for low-level primitives and will produce
> name clashes. But I recognise that noacquire is just an ugly name.)
>
> Here, a __read_lock_noacquire _must_ be followed by a
> smp__mb_after_lock(), and a __read_unlock_norelease() _must_ be
> preceded by a smp__mb_before_unlock().

Your point was, smp_mb__after_lock() adds more complexity to the
barriers/locking rules.

Do you really think __read_lock_noacquire() makes this all more
simple/understandable? And again, we need __read_lock_irq_noaquire/etc.

Personally, I disagree. In fact, I do not understand when/why
_noacquire can be used, but this is another story.

Let's look from the different angle. The first patch from Jiri fixes
the bug. Yes, it is not clear if this is possible to trigger this
bug in practice, but still nobody disagrees the bug does exist.
The second patch fixes the added pessimization.

So, if you do not agree with these patches, perhaps you can send
fixes on top of these changes?



Sadly, I already removed the previous emails so I can't add my
acked-by to Jiri's patches. I didn't do this before because I
thought I am in no position to ack these changes. But looking
at this discussion, I'd like to vote for both these patches
anyway ;)

Oleg.