Hi Dave, On Thu, Jul 09, 2020 at 12:32:46PM +1000, Dave Chinner wrote: ... > > > if (trylock AGI) > > > return; > > > > (adding some notes here, this patch doesn't use try lock here > > finally but unlock perag and take AGI and relock and recheck tail_empty.... > > since the tail non-empty is rare...) > > *nod* > > My point was largely that this sort of thing is really obvious and > easy to optimise once the locking is cleanly separated. Adding a > trylock rather than drop/relock is another patch for the series :P I thought trylock way yesterday as well... Apart from that we don't have the AGI trylock method yet, it seems not work as well... Thinking about one thread is holding a AGI lock and wants to lock perag. And another thread holds a perag, but the trylock will fail and the second wait-lock will cause deadlock... like below: Thread 1 Thread 2 lock AGI lock perag lock perag trylock AGI lock AGI <- dead lock here And trylock perag instead seems not work well as well, since it fails more frequently so we need lock AGI and then lock perag as a fallback. So currently, I think it's better to use unlock, do something, regrab, recheck method for now... And yeah, that can be done in another patch if some better way. (e.g. moving modifing AGI into pre-commit, so we can only take one per-ag lock here...) Thanks, Gao Xiang > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx >