----- On Nov 9, 2017, at 2:07 PM, Andy Lutomirski luto@xxxxxxxxxx wrote:
> On Wed, Nov 8, 2017 at 10:35 AM, Mathieu Desnoyers
> <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
>
>> +/*
>> + * x86-64 implements return to user-space through sysret, which is not a
>> + * core-serializing instruction. Therefore, we need an explicit core
>> + * serializing instruction after going from kernel thread back to
>> + * user-space thread (active_mm moved back to current mm).
>> + */
>> +static inline void membarrier_arch_mm_sync_core(struct mm_struct *mm)
>> +{
>> + if (likely(!(atomic_read(&mm->membarrier_state) &
>> + MEMBARRIER_STATE_SYNC_CORE)))
>> + return;
>> + sync_core();
>> +}
>
> IMO there should be an extremely clear specification somewhere for
> what this function is supposed to do.
>
> If I remember correctly, it's supposed to promise that the icache is
> synced before the next time we return to usermode for the current mm
> on this CPU. If that's correct, then let's document it very
> explicitly and let's also drop the "membarrier" from the name -- it's
> a primitive we'll need anyway given the existing migration bug.
I understand that on x86 (specifically), synchronizing the icache and
doing a core serializing instruction may mean the same thing.
However, on architectures like ARM, icache sync differs from core
serialization. Those architectures typically have either a user-space
accessible instruction or a system call to perform the icache flush.
The missing part for JIT is core serialization (also called context
synchronization). icache flush is already handled by pre-existing
means.
So the promise here given by membarrier_arch_mm_sync_core() is that
a core serializing instruction is issued before the next time we return
to usermode on the current thread. However, we only need that guarantee
if the current thread's mm is a registered MEMBARRIER_{...}_SYNC_CORE user.
Regarding the existing migration bug, what I think you want is a kind
of weaker "sync_core()", which ensures that a core serializing
instruction is issued before the next time the current thread returns
to usermode.
It could be e.g.: set_tsk_need_core_sync() which would set a
TIF_NEED_CORE_SYNC thread flag on the current thread.
Clearly, when this kind of thread flag is introduced as an
optimization over sync_core(), I would like to use that. However,
I don't think it replaces the membarrier_arch_mm_sync_core() entirely,
given that it would not check for the mm membarrier "SYNC_CORE"
registration state. It appears to me to be merely an optimization over
directly invoking sync_core.
What I suggest is that I update the comment above
membarrier_arch_mm_sync_core to spell out more clearly that all we
need is to have a core serializing instruction issued before the next
time the current thread returns to user-space. I can still use
sync_core for now, and we can then improve the implementation
whenever a new thread flag is introduced. The new comment would look
like:
/*
* x86-64 implements return to user-space through sysret, which is not a
* core-serializing instruction. Therefore, we need an explicit core
* serializing instruction after going from kernel thread back to
* user-space thread (active_mm moved back to current mm).
*
* This function ensures that a core serializing instruction is issued
* before the current thread returns to user-space.
*/
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
