Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Nov 2019 23:43:52 +0000 (UTC)
From:      Warner Losh <imp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r355037 - head/sys/dev/pci
Message-ID:  <201911232343.xANNhqkQ097797@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: imp
Date: Sat Nov 23 23:43:52 2019
New Revision: 355037
URL: https://svnweb.freebsd.org/changeset/base/355037

Log:
  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.

Modified:
  head/sys/dev/pci/pci.c
  head/sys/dev/pci/pci_user.c

Modified: head/sys/dev/pci/pci.c
==============================================================================
--- head/sys/dev/pci/pci.c	Sat Nov 23 23:41:21 2019	(r355036)
+++ head/sys/dev/pci/pci.c	Sat Nov 23 23:43:52 2019	(r355037)
@@ -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 */

Modified: head/sys/dev/pci/pci_user.c
==============================================================================
--- head/sys/dev/pci/pci_user.c	Sat Nov 23 23:41:21 2019	(r355036)
+++ head/sys/dev/pci/pci_user.c	Sat Nov 23 23:43:52 2019	(r355037)
@@ -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,



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201911232343.xANNhqkQ097797>