> On Jul 15, 2020, at 9:15 PM, Nicholas Piggin <npiggin@xxxxxxxxx> wrote: > > Excerpts from Mathieu Desnoyers's message of July 14, 2020 12:13 am: >> ----- On Jul 13, 2020, at 9:47 AM, Nicholas Piggin npiggin@xxxxxxxxx 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 must say the memory barrier comments in membarrier could be improved > a bit (unless I'm missing where the main comment is). It's fine to know > what barriers pair with one another, but we need to know which exact > memory accesses it is ordering > > /* > * Matches memory barriers around rq->curr modification in > * scheduler. > */ > > Sure, but it doesn't say what else is being ordered. I think it's just > the user memory accesses, but would be nice to make that a bit more > explicit. If we had such comments then we might know this case is safe. > > I think the funny powerpc barrier is a similar case of this. If we > ever see remote_rq->curr->flags & PF_KTHREAD, then we _know_ that > CPU has or will have issued a memory barrier between running user > code. > > So AFAIKS all this membarrier stuff in kernel/sched/core.c could > just go away. Except x86 because thread switch doesn't imply core > sync, so CPU1 between 1 and 8 may never issue a core sync instruction > the same way a context switch must be a full mb. > > Before getting to x86 -- Am I right, or way off track here? I find it hard to believe that this is x86 only. Why would thread switch imply core sync on any architecture? Is x86 unique in having a stupid expensive core sync that is heavier than smp_mb()? But I’m wondering if all this deferred sync stuff is wrong. In the brave new world of io_uring and such, perhaps kernel access matter too. Heck, even: int a[2]; Thread A: a[0] = 1; a[1] = 2: Thread B: write(fd, a, sizeof(a)); Doesn’t do what thread A is expecting. Admittedly this particular example is nonsense, but maybe there are sensible cases that matter to someone. —Andy > > Thanks, > Nick