Hi Mikey, First of all, thanks for you detailed review. I really appreciate your comments here.
On 09/17/2018 02:25 AM, Michael Neuling wrote: > On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote: >> This patchset for the hardware transactional memory (TM) subsystem aims to >> avoid spending a lot of time on TM suspended mode in kernel space. It >> basically >> changes where the reclaim/recheckpoint will be executed. >> >> Once a CPU enters in transactional state it uses a footprint area to track >> down the load/stores performed in transaction so it can be verified later >> to decide if a conflict happened due to some change done in that state. If >> a transaction is active in userspace and there is an exception that takes >> the CPU to the kernel space the CPU moves the transaction to suspended >> state but does not discard the footprint area. > > In this description, you should differente between memory and register > (GPR/VSX/SPR) footprints. Right, reading the ISA, I understand that footprint is a term for memory only and it represents the modified memory that was stored during a transactional state (that after tbegin). For registers, the ISA talks about checkpointed registers, which is the register state *before* a transaction starts. I.e, for register it is the previous state, and for memory, it is the current/live state. That said, if the transactional is aborted, the memory footprint is discarded and the checkpointed registers replaces the live registers. > In suspend, the CPU can disregard the memory footprint at any point, but it > has> to keep the register footprint. Yes! Anyway, I was just trying to describe how the hardware works, it is not related to the kernel at the paragraph above, but I will make sure I will re-write it better. >> This patchset aims to reclaim the checkpointed footprint as soon as the >> kernel is invoked, in the beginning of the exception handlers, thus freeing >> room to other CPUs enter in suspended mode, avoiding too many CPUs in >> suspended >> state that can cause the CPUs to stall. The same mechanism is done on kernel >> exit, doing a recheckpoint as late as possible (which will reload the >> checkpointed state into CPU's room) at the exception return. > > OK, but we are still potentially in suspend in userspace, so that doesn't help > us on the lockup issue. > > We need fake suspend in userspace to prevent lockups. Correct. I will make sure I document it. This patchset is the very first step to start creating a work around for the hardware limitations. >> The way to achieve this goal is creating a macro (TM_KERNEL_ENTRY) which >> will check if userspace was in an active transaction just after getting >> into kernel and reclaiming if that's the case. Thus all exception handlers >> will call this macro as soon as possible. >> >> All exceptions should reclaim (if necessary) at this stage and only >> recheckpoint if the task is tagged as TIF_RESTORE_TM (i.e. was in >> transactional state before being interrupted), which will be done at >> ret_from_except_lite(). >> >> Ideally all reclaims will happen at the exception entrance, however during >> the recheckpoint process another exception can hit the CPU which might >> cause the current thread to be rescheduled, thus there is another reclaim >> point to be considered at __switch_to(). > > Can we do the recheckpoint() later so that it's when we have interrupts off > and > can't be rescheduled? Yes! After thinking on it for a long time, this is definitely what should be done. I will send a v2 with this change (and others being discussed here) Thank you, Breno
