On Tue, 2024-02-06 at 20:47 -0800, Sean Christopherson wrote: > > I'm saying this: > > When processing mmu_notifier invalidations for gpc caches, pre-check for > overlap with the invalidation event while holding gpc->lock for read, and > only take gpc->lock for write if the cache needs to be invalidated. Doing > a pre-check without taking gpc->lock for write avoids unnecessarily > contending the lock for unrelated invalidations, which is very beneficial > for caches that are heavily used (but rarely subjected to mmu_notifier > invalidations). > > is much friendlier to readers than this: > > Taking a write lock on a pfncache will be disruptive if the cache is > heavily used (which only requires a read lock). Hence, in the MMU notifier > callback, take read locks on caches to check for a match; only taking a > write lock to actually perform an invalidation (after a another check). That's a somewhat subjective observation. I actually find the latter to be far more succinct and obvious. Actually... maybe I find yours harder because it isn't actually stating the situation as I understand it. You said "unrelated invalidation" in your first email, and "overlap with the invalidation event" in this one... neither of which makes sense to me because there is no *other* invalidation here. We're only talking about the MMU notifier gratuitously taking the write lock on a GPC that it *isn't* going to invalidate (the common case), and that disrupting users which are trying to take the read lock on that GPC.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature