On Fri, Jul 17, 2020 at 09:39:25AM -0400, Mathieu Desnoyers wrote:

----- On Jul 16, 2020, at 5:24 PM, Alan Stern stern@rowland.harvard.edu wrote:

quoted

On Thu, Jul 16, 2020 at 02:58:41PM -0400, Mathieu Desnoyers wrote:

quoted

----- On Jul 16, 2020, at 12:03 PM, Mathieu Desnoyers
mathieu.desnoyers@efficios.com wrote:

quoted

----- On Jul 16, 2020, at 11:46 AM, Mathieu Desnoyers
mathieu.desnoyers@efficios.com wrote:

quoted

----- On Jul 16, 2020, at 12:42 AM, Nicholas Piggin npiggin@gmail.com wrote:

quoted

I should be more complete here, especially since I was complaining
about unclear barrier comment :)


CPU0                     CPU1
a. user stuff            1. user stuff
b. membarrier()          2. enter kernel
c. smp_mb()              3. smp_mb__after_spinlock(); // in __schedule
d. read rq->curr         4. rq->curr switched to kthread
e. is kthread, skip IPI  5. switch_to kthread
f. return to user        6. rq->curr switched to user thread
g. user stuff            7. switch_to user thread
                        8. exit kernel
                        9. more user stuff

...

quoted

quoted

Requiring a memory barrier between update of rq->curr (back to current process's
thread) and following user-space memory accesses does not seem to guarantee
anything more than what the initial barrier at the beginning of __schedule
already
provides, because the guarantees are only about accesses to user-space memory.

...

quoted

Is it correct to say that the switch_to operations in 5 and 7 include
memory barriers?  If they do, then skipping the IPI should be okay.

The reason is as follows: The guarantee you need to enforce is that
anything written by CPU0 before the membarrier() will be visible to CPU1
after it returns to user mode.  Let's say that a writes to X and 9
reads from X.

Then we have an instance of the Store Buffer pattern:

CPU0			CPU1
a. Write X		6. Write rq->curr for user thread
c. smp_mb()		7. switch_to memory barrier
d. Read rq->curr	9. Read X

In this pattern, the memory barriers make it impossible for both reads
to miss their corresponding writes.  Since d does fail to read 6 (it
sees the earlier value stored by 4), 9 must read a.

The other guarantee you need is that g on CPU0 will observe anything
written by CPU1 in 1.  This is easier to see, using the fact that 3 is a
memory barrier and d reads from 4.

Right, and Nick's reply involving pairs of loads/stores on each side
clarifies the situation even further.

The key part of my reply was the question: "Is it correct to say that 
the switch_to operations in 5 and 7 include memory barriers?"  From the 
text quoted above and from Nick's reply, it seems clear that they do 
not.

I agree with Nick: A memory barrier is needed somewhere between the 
assignment at 6 and the return to user mode at 8.  Otherwise you end up 
with the Store Buffer pattern having a memory barrier on only one side, 
and it is well known that this arrangement does not guarantee any 
ordering.

One thing I don't understand about all this: Any context switch has to 
include a memory barrier somewhere, but both you and Nick seem to be 
saying that steps 6 and 7 don't include (or don't need) any memory 
barriers.  What am I missing?

Alan Stern