Quoting Douglas Anderson (2020-04-24 09:46:56) > The rpmh-rsc code had both a driver-level lock (sometimes referred to > in comments as drv->lock) and a lock per-TCS. The idea was supposed > to be that there would be times where you could get by with just > locking a TCS lock and therefor other RPMH users wouldn't be blocked. > > The above didn't work out so well. > > Looking at tcs_write() the bigger drv->lock was held for most of the > function anyway. Only the __tcs_buffer_write() and > __tcs_set_trigger() calls were called without holding the drv->lock. > It actually turns out that in tcs_write() we don't need to hold the > drv->lock for those function calls anyway even if the per-TCS lock > isn't there anymore. From the newly added comments in the code, this > is because: > - We marked "tcs_in_use" under lock. > - Once "tcs_in_use" has been marked nobody else could be writing > to these registers until the interrupt goes off. > - The interrupt can't go off until we trigger w/ the last line > of __tcs_set_trigger(). > Thus, from a tcs_write() point of view, the per-TCS lock was useless. > > Looking at rpmh_rsc_write_ctrl_data(), only the per-TCS lock was held. > It turns out, though, that this function already needs to be called > with the equivalent of the drv->lock held anyway (we either need to > hold drv->lock as we will in a future patch or we need to know no > other CPUs could be running as happens today). Specifically > rpmh_rsc_write_ctrl_data() might be writing to a TCS that has been > borrowed for writing an active transation but it never checks this. > > Let's eliminate this extra overhead and avoid possible AB BA locking > headaches. > > Suggested-by: Maulik Shah <mkshah@xxxxxxxxxxxxxx> > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
