Re: [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode

Nicholas Piggin
2020-07-16T04:42:49Z

Excerpts from Nicholas Piggin's message of July 16, 2020 2:15 pm:
> Excerpts from Mathieu Desnoyers's message of July 14, 2020 12:13 am:
>> ----- On Jul 13, 2020, at 9:47 AM, Nicholas Piggin npiggin@gmail.com wrote:
>>
>>> Excerpts from Nicholas Piggin's message of July 13, 2020 2:45 pm:
>>>> Excerpts from Andy Lutomirski's message of July 11, 2020 3:04 am:
>>>>> Also, as it stands, I can easily see in_irq() ceasing to promise to
>>>>> serialize. There are older kernels for which it does not promise to
>>>>> serialize. And I have plans to make it stop serializing in the
>>>>> nearish future.
>>>>
>>>> You mean x86's return from interrupt? Sounds fun... you'll konw where to
>>>> update the membarrier sync code, at least :)
>>>
>>> Oh, I should actually say Mathieu recently clarified a return from
>>> interrupt doesn't fundamentally need to serialize in order to support
>>> membarrier sync core.
>>
>> Clarification to your statement:
>>
>> Return from interrupt to kernel code does not need to be context serializing
>> as long as kernel serializes before returning to user-space.
>>
>> However, return from interrupt to user-space needs to be context serializing.
>
> Hmm, I'm not sure it's enough even with the sync in the exit_lazy_tlb
> in the right places.
>
> A kernel thread does a use_mm, then it blocks and the user process with
> the same mm runs on that CPU, and then it calls into the kernel, blocks,
> the kernel thread runs again, another CPU issues a membarrier which does
> not IPI this one because it's running a kthread, and then the kthread
> switches back to the user process (still without having unused the mm),
> and then the user process returns from syscall without having done a
> core synchronising instruction.
>
> The cause of the problem is you want to avoid IPI'ing kthreads. Why?
> I'm guessing it really only matters as an optimisation in case of idle
> threads. Idle thread is easy (well, easier) because it won't use_mm, so
> you could check for rq->curr == rq->idle in your loop (in a suitable
> sched accessor function).
>
> But... I'm not really liking this subtlety in the scheduler for all this
> (the scheduler still needs the barriers when switching out of idle).
>
> Can it be improved somehow? Let me forget x86 core sync problem for now
> (that _may_ be a bit harder), and step back and look at what we're doing.
> The memory barrier case would actually suffer from the same problem as
> core sync, because in the same situation it has no implicit mmdrop in
> the scheduler switch code either.
>
> So what are we doing with membarrier? We want any activity caused by the
> set of CPUs/threads specified that can be observed by this thread before
> calling membarrier is appropriately fenced from activity that can be
> observed to happen after the call returns.
>
> CPU0 CPU1
> 1. user stuff
> a. membarrier() 2. enter kernel
> b. read rq->curr 3. rq->curr switched to kthread
> c. is kthread, skip IPI 4. switch_to kthread
> d. return to user 5. rq->curr switched to user thread
> 6. switch_to user thread
> 7. exit kernel
> 8. more user stuff
>
> As far as I can see, the problem is CPU1 might reorder step 5 and step
> 8, so you have mmdrop of lazy mm be a mb after step 6.
>
> But why? The membarrier call only cares that there is a full barrier
> between 1 and 8, right? Which it will get from the previous context
> switch to the kthread.

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

What you're really ordering is a, g vs 1, 9 right?

In other words, 9 must see a if it sees g, g must see 1 if it saw 9,
etc.

Userspace does not care where the barriers are exactly or what kernel
memory accesses might be being ordered by them, so long as there is a
mb somewhere between a and g, and 1 and 9. Right?