On Wed, Mar 20, 2019 at 1:47 PM Stephane Eranian <eran...@google.com> wrote: > > On Tue, Mar 19, 2019 at 11:20 AM Peter Zijlstra <pet...@infradead.org> wrote: > > > > On Tue, Mar 19, 2019 at 10:52:01AM -0700, Stephane Eranian wrote: > > > > Not quite; the control on its own doesn't directly write the MSR. And > > > > even when the work-around is allowed, we'll not set the MSR unless there > > > > is also demand for PMC3. > > > > > > > Trying to understand this better here. When the workaround is enabled > > > (tfa=0), you lose PMC3 and transactions operate normally. > > > > > When it is disabled (tfa=1), transactions are all aborted and PMC3 is > > > available. > > > > Right, but we don't expose tfa. > > > > > If you are saying that when there is a PMU event requesting PMC3, then > > > you need PMC3 avail, so you set the MSR so that tfa=1 forcing all > > > transactions to abort. > > > > Right, so when allow_tfa=1 (default), we only set tfa=1 when PMC3 is > > requested. > > > > This has the advantage that, > > > > TSX only workload -> works > > perf 4 counteres -> works > > > > Only when you need both of them, do you get 'trouble'. > > > > > But in that case, you are modifying the execution of the workload when > > > you are monitoring it, assuming it uses TSX. > > > > We assume you are not in fact using TSX, not a lot of code does. If you > > do use TSX a lot, and you don't want to interfere, you have to set > > allow_tfa=0 and live with one counter less. > > > > Any which way around you turn this stone, it sucks. > > > > > You want lowest overhead and no modifications to how the workload > > > operates, otherwise how representative is the data you are collecting? > > > > Sure; but there are no good choices here. This 'fix' will break > > something. We figured TSX+4-counter-perf was the least common scenario. > > > > We konw of people that rely on 4 counter being present; you want to > > explain to them how when doing an update their program suddently doesn't > > work anymore? > > > > Or you want to default to tfa=1; but then you have to explain to those > > people relying on TSX why their workload stopped working. > > > > > I understand that there is no impact on apps not using TSX, well, > > > except on context switch where you have to toggle that MSR. > > > > There is no additional code in the context switch; only the perf event > > scheduling code prods at the MSR. > > > > > But for workloads using TSX, there is potentially an impact. > > > > Yes, well, if you're a TSX _and_ perf user, you now have an extra knob > > to play with; that's not something I can do anything about. We're forced > > to make a choice here. > > > > > > Yeah, meh. You're admin, you can 'fix' it. In practise I don't expect > > > > most people to care about the knob, and the few people that do, should > > > > be able to make it work. > > > > > > I don't understand how this can work reliably. > > > > > You have a knob to toggle that MSR. > > > > No, we don't have this knob. > > > > > Then, you have another one inside perf_events > > > > Only this knob exists allow_tfa. > > > > > and then the sysadmin has to make sure nobody (incl. NMI watchdog) is > > > using the PMU when this all happens. > > > > You're very unlucky if the watchdog runs on PMC3, normally it runs on > > Fixed1 or something. Esp early after boot. (Remember, we schedule fixed > > counters first, and then general purpose counters, with a preference for > > lower counters). > > > > Anyway, you can trivially switch it off if you want. > > > > > How can this be a practical solution? Am I missing something here? > > > > It works just fine; it is unfortunate that we have this interaction but > > that's not something we can do anything about. We're forced to deal with > > this. > > > Right now, if I do: > > echo 0 > /sys/bus/event_source/devices/cpu/allow_tsx_force_abort > > Then I don't have the guarantee on when there will be no abort when I > return from the echo. > the MSR is accessed only on PMU scheduling. I would expect a sysadmin > to want some guarantee > if this is to be switched on/off at runtime. If not, then having a > boot time option is better in my opinion. > > This other bit I noticed is that cpuc->tfa_shadow is used to avoid the > wrmsr(), but I don't see the > code that makes sure the init value (0) matches the value of the MSR. > Is this MSR guarantee to be > zero on reset? How about on kexec()? >
Furthermore, depending on what is measured on each CPU, i.e., PMC3 is used or not, the TFA may be set to true or false dynamically. So if you have a TSX workload it may be impacted depending on which CPU it runs on. I don't think users would like that approach. > > But if you're a TSX+perf user, have your boot scripts do: > > > > echo 0 > /sys/bus/event_source/devices/cpu/allow_tsx_force_abort > > > > and you'll not use PMC3 and TSX will be 'awesome'. If you don't give a > > crap about TSX (most people), just boot and be happy. > > > > If you do care about TSX+perf and want to dynamically toggle for some > > reason, you just have to be a little careful.