* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> The x86 memory rules are that two loads always execute in order (ie
> rmb is a no-op).
>
> So I see no reason for a memory barrier after the monitor. [...]
Yes, I'm leaning towards that interpretation as well, but the reason
I'm a bit catious is the somewhat curious (to me!) wording of the
MONITOR instruction:
Sets up a linear address range to be monitored by hardware and
activates the monitor.
...
The MONITOR instruction is ordered as a load operation with respect
to other memory transactions. The instruction can be used at all
privilege levels and is subject to all permission checking and
faults associated with a byte load. Like a load, the MONITOR
instruction sets the A-bit but not the D-bit in page tables.
Where apparently the 'range' means 'full cache line surrounding the
memory address in question'.
We have no other load instructions that operate on such a large
'range' of addresses, and I wanted to make sure it's a true (single
byte) load for that specific address. The documentation does not
appear to explicitly state that it's a load for that address - only
that it's ordered as a load.
The reason I'm asking is because 'flags' itself might not be at the
beginning of the cache line, as it's in the middle of thread_info:
struct thread_info {
struct task_struct *task; /* main task structure */
struct exec_domain *exec_domain; /* execution domain */
__u32 flags; /* low level flags */
while 'MONITOR' appears to work on the cache line. So are all
addresses within that cache line ordered? Only the specific address
given to the instruction itself? Only the first word of the cacheline
itself?
The documentation is a bit vague, at least in my reading, and
depending on which actual word the instruction reads (if it reads any
word at all ... it's probably just setting up an address for MWAIT)
from that cacheline, its ordering properties might be surprising.
> [...] But both sides of clflush sounds sane, and as mentioned the
> "go to sleep" side isn't as critical as the "wake up" side if the
> monitor.
Yeah.
> Please let's just make that pre-monitor hack be a static key, and do
> mfence explicitly around the clflush inside that conditional
> section.
Agreed.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html