On 11/04/2022 10:35, Jan Beulich wrote: > Prior extension of these functions to enable per-device quarantine page > tables already didn't add more locking there, but merely left in place > what had been there before. But really locking is unnecessary here: > We're running with pcidevs_lock held (i.e. multiple invocations of the > same function [or their teardown equivalents] are impossible, and hence > there are no "local" races), while all consuming of the data being > populated here can't race anyway due to happening sequentially > afterwards. See also the comment in struct arch_pci_dev. > > Signed-off-by: Jan Beulich <jbeul...@suse.com>
It is only legitimate to drop these calls if you delete the mapping_lock entirely. Otherwise you're breaking the semantics of mapping_lock. Your argument of "well this is already guarded by the pci lock" means that these are uncontended lock/unlock operations, and therefore not interesting to drop in the first place. This patch is specifically setting us up for an XSA in the future when the behaviour of the the PCI lock changes, the fix for which will be revert this patch. ~Andrew
