From 76e700171940f40a290a4f056bc62344522296f4 Mon Sep 17 00:00:00 2001 From: imp Date: Sat, 23 Nov 2019 23:43:52 +0000 Subject: [PATCH] Push Giant down one layer The /dev/pci device doesn't need GIANT, per se. However, one routine that it calls, pci_find_dbsf implicitly does. It walks a list that can change when PCI scans a new bus. With hotplug, this means we could have a race with that scanning. To prevent that, take out Giant around scanning the list. However, given that we have places in the tree that drop giant, if held when we call into them, the whole use of Giant to protect newbus may be less effective that we desire, so add a comment about why we're talking it out, and we'll address the issue when we lock newbus with something other than Giant. --- sys/dev/pci/pci.c | 9 ++++++--- sys/dev/pci/pci_user.c | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c index e9e86a46a413..9a8a87a11934 100644 --- a/sys/dev/pci/pci.c +++ b/sys/dev/pci/pci.c @@ -445,18 +445,21 @@ pci_find_bsf(uint8_t bus, uint8_t slot, uint8_t func) device_t pci_find_dbsf(uint32_t domain, uint8_t bus, uint8_t slot, uint8_t func) { - struct pci_devinfo *dinfo; + struct pci_devinfo *dinfo = NULL; + /* Giant because newbus is Giant locked revisit with newbus locking */ + mtx_lock(&Giant); STAILQ_FOREACH(dinfo, &pci_devq, pci_links) { if ((dinfo->cfg.domain == domain) && (dinfo->cfg.bus == bus) && (dinfo->cfg.slot == slot) && (dinfo->cfg.func == func)) { - return (dinfo->cfg.dev); + break; } } + mtx_unlock(&Giant); - return (NULL); + return (dinfo != NULL ? dinfo->cfg.dev : NULL); } /* Find a device_t by vendor/device ID */ diff --git a/sys/dev/pci/pci_user.c b/sys/dev/pci/pci_user.c index 9e3842acf889..0052c0a6844e 100644 --- a/sys/dev/pci/pci_user.c +++ b/sys/dev/pci/pci_user.c @@ -119,7 +119,7 @@ static d_ioctl_t pci_ioctl; struct cdevsw pcicdev = { .d_version = D_VERSION, - .d_flags = D_NEEDGIANT, + .d_flags = 0, .d_open = pci_open, .d_close = pci_close, .d_ioctl = pci_ioctl,