On 31.08.2022 16:11, Volodymyr Babchuk wrote: > Prior to this change, lifetime of pci_dev objects was protected by global > pcidevs_lock(). We are going to get if of this lock, so we need some > other mechanism to ensure that those objects will not disappear under > feet of code that access them. Reference counting is a good choice as > it provides easy to comprehend way to control object lifetime with > better granularity than global super lock. > > This patch adds two new helper functions: pcidev_get() and > pcidev_put(). pcidev_get() will increase reference counter, while > pcidev_put() will decrease it, destroying object when counter reaches > zero. > > pcidev_get() should be used only when you already have a valid pointer > to the object or you are holding lock that protects one of the > lists (domain, pseg or ats) that store pci_dev structs. > > pcidev_get() is rarely used directly, because there already are > functions that will provide valid pointer to pci_dev struct: > pci_get_pdev() and pci_get_real_pdev(). They will lock appropriate > list, find needed object and increase its reference counter before > returning to the caller. > > Naturally, pci_put() should be called after finishing working with a > received object. This is the reason why this patch have so many > pcidev_put()s and so little pcidev_get()s: existing calls to > pci_get_*() functions now will increase reference counter > automatically, we just need to decrease it back when we finished. > > This patch removes "const" qualifier from some pdev pointers because > pcidev_put() technically alters the contents of pci_dev structure.
I wonder if you have so few "get"s because in some cases references would be needed, but aren't being obtained. As a rule of thumb I'd expect any entity storing a pointer in a long-lived data structure to obtain a ref first. And we have quite a few struct fields pointing to devices. I'd also expect a reference to be held when a device is e.g. put on a domain's list. This would then likely mean that for example in deassign_device() (or maybe pci_add_device()) you wouldn't drop the ref in the success case, but instead the ref would transfer to the domain the device is added to. > --- > > - Jan, can I add your Suggested-by tag? Sure, why not. Jan
