diff options
author | Emil Tantilov <emil.s.tantilov@intel.com> | 2017-01-06 16:59:08 -0500 |
---|---|---|
committer | Bjorn Helgaas <bhelgaas@google.com> | 2017-02-03 14:42:38 -0500 |
commit | 5b0948dfe138f0837699f46f5877f4f81c252dac (patch) | |
tree | eb67c2325c5dea87be81c16ce19b872dd153ac6a | |
parent | 7184f5b451cf3dc61de79091d235b5d2bba2782d (diff) |
PCI: Lock each enable/disable num_vfs operation in sysfs
Enabling/disabling SRIOV via sysfs by echo-ing multiple values
simultaneously:
# echo 63 > /sys/class/net/ethX/device/sriov_numvfs&
# echo 63 > /sys/class/net/ethX/device/sriov_numvfs
# sleep 5
# echo 0 > /sys/class/net/ethX/device/sriov_numvfs&
# echo 0 > /sys/class/net/ethX/device/sriov_numvfs
results in the following bug:
kernel BUG at drivers/pci/iov.c:495!
invalid opcode: 0000 [#1] SMP
CPU: 1 PID: 8050 Comm: bash Tainted: G W 4.9.0-rc7-net-next #2092
RIP: 0010:[<ffffffff813b1647>]
[<ffffffff813b1647>] pci_iov_release+0x57/0x60
Call Trace:
[<ffffffff81391726>] pci_release_dev+0x26/0x70
[<ffffffff8155be6e>] device_release+0x3e/0xb0
[<ffffffff81365ee7>] kobject_cleanup+0x67/0x180
[<ffffffff81365d9d>] kobject_put+0x2d/0x60
[<ffffffff8155bc27>] put_device+0x17/0x20
[<ffffffff8139c08a>] pci_dev_put+0x1a/0x20
[<ffffffff8139cb6b>] pci_get_dev_by_id+0x5b/0x90
[<ffffffff8139cca5>] pci_get_subsys+0x35/0x40
[<ffffffff8139ccc8>] pci_get_device+0x18/0x20
[<ffffffff8139ccfb>] pci_get_domain_bus_and_slot+0x2b/0x60
[<ffffffff813b09e7>] pci_iov_remove_virtfn+0x57/0x180
[<ffffffff813b0b95>] pci_disable_sriov+0x65/0x140
[<ffffffffa00a1af7>] ixgbe_disable_sriov+0xc7/0x1d0 [ixgbe]
[<ffffffffa00a1e9d>] ixgbe_pci_sriov_configure+0x3d/0x170 [ixgbe]
[<ffffffff8139d28c>] sriov_numvfs_store+0xdc/0x130
...
RIP [<ffffffff813b1647>] pci_iov_release+0x57/0x60
Use the existing mutex lock to protect each enable/disable operation.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
-rw-r--r-- | drivers/pci/iov.c | 7 | ||||
-rw-r--r-- | drivers/pci/pci-sysfs.c | 23 | ||||
-rw-r--r-- | drivers/pci/pci.h | 2 |
3 files changed, 17 insertions, 15 deletions
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index 47227820406d..2479ae876482 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c | |||
@@ -124,7 +124,6 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset) | |||
124 | struct pci_sriov *iov = dev->sriov; | 124 | struct pci_sriov *iov = dev->sriov; |
125 | struct pci_bus *bus; | 125 | struct pci_bus *bus; |
126 | 126 | ||
127 | mutex_lock(&iov->dev->sriov->lock); | ||
128 | bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id)); | 127 | bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id)); |
129 | if (!bus) | 128 | if (!bus) |
130 | goto failed; | 129 | goto failed; |
@@ -162,7 +161,6 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset) | |||
162 | __pci_reset_function(virtfn); | 161 | __pci_reset_function(virtfn); |
163 | 162 | ||
164 | pci_device_add(virtfn, virtfn->bus); | 163 | pci_device_add(virtfn, virtfn->bus); |
165 | mutex_unlock(&iov->dev->sriov->lock); | ||
166 | 164 | ||
167 | pci_bus_add_device(virtfn); | 165 | pci_bus_add_device(virtfn); |
168 | sprintf(buf, "virtfn%u", id); | 166 | sprintf(buf, "virtfn%u", id); |
@@ -181,12 +179,10 @@ failed2: | |||
181 | sysfs_remove_link(&dev->dev.kobj, buf); | 179 | sysfs_remove_link(&dev->dev.kobj, buf); |
182 | failed1: | 180 | failed1: |
183 | pci_dev_put(dev); | 181 | pci_dev_put(dev); |
184 | mutex_lock(&iov->dev->sriov->lock); | ||
185 | pci_stop_and_remove_bus_device(virtfn); | 182 | pci_stop_and_remove_bus_device(virtfn); |
186 | failed0: | 183 | failed0: |
187 | virtfn_remove_bus(dev->bus, bus); | 184 | virtfn_remove_bus(dev->bus, bus); |
188 | failed: | 185 | failed: |
189 | mutex_unlock(&iov->dev->sriov->lock); | ||
190 | 186 | ||
191 | return rc; | 187 | return rc; |
192 | } | 188 | } |
@@ -195,7 +191,6 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id, int reset) | |||
195 | { | 191 | { |
196 | char buf[VIRTFN_ID_LEN]; | 192 | char buf[VIRTFN_ID_LEN]; |
197 | struct pci_dev *virtfn; | 193 | struct pci_dev *virtfn; |
198 | struct pci_sriov *iov = dev->sriov; | ||
199 | 194 | ||
200 | virtfn = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus), | 195 | virtfn = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus), |
201 | pci_iov_virtfn_bus(dev, id), | 196 | pci_iov_virtfn_bus(dev, id), |
@@ -218,10 +213,8 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id, int reset) | |||
218 | if (virtfn->dev.kobj.sd) | 213 | if (virtfn->dev.kobj.sd) |
219 | sysfs_remove_link(&virtfn->dev.kobj, "physfn"); | 214 | sysfs_remove_link(&virtfn->dev.kobj, "physfn"); |
220 | 215 | ||
221 | mutex_lock(&iov->dev->sriov->lock); | ||
222 | pci_stop_and_remove_bus_device(virtfn); | 216 | pci_stop_and_remove_bus_device(virtfn); |
223 | virtfn_remove_bus(dev->bus, virtfn->bus); | 217 | virtfn_remove_bus(dev->bus, virtfn->bus); |
224 | mutex_unlock(&iov->dev->sriov->lock); | ||
225 | 218 | ||
226 | /* balance pci_get_domain_bus_and_slot() */ | 219 | /* balance pci_get_domain_bus_and_slot() */ |
227 | pci_dev_put(virtfn); | 220 | pci_dev_put(virtfn); |
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 066628776e1b..25d010d449a3 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c | |||
@@ -472,6 +472,7 @@ static ssize_t sriov_numvfs_store(struct device *dev, | |||
472 | const char *buf, size_t count) | 472 | const char *buf, size_t count) |
473 | { | 473 | { |
474 | struct pci_dev *pdev = to_pci_dev(dev); | 474 | struct pci_dev *pdev = to_pci_dev(dev); |
475 | struct pci_sriov *iov = pdev->sriov; | ||
475 | int ret; | 476 | int ret; |
476 | u16 num_vfs; | 477 | u16 num_vfs; |
477 | 478 | ||
@@ -482,38 +483,46 @@ static ssize_t sriov_numvfs_store(struct device *dev, | |||
482 | if (num_vfs > pci_sriov_get_totalvfs(pdev)) | 483 | if (num_vfs > pci_sriov_get_totalvfs(pdev)) |
483 | return -ERANGE; | 484 | return -ERANGE; |
484 | 485 | ||
486 | mutex_lock(&iov->dev->sriov->lock); | ||
487 | |||
485 | if (num_vfs == pdev->sriov->num_VFs) | 488 | if (num_vfs == pdev->sriov->num_VFs) |
486 | return count; /* no change */ | 489 | goto exit; |
487 | 490 | ||
488 | /* is PF driver loaded w/callback */ | 491 | /* is PF driver loaded w/callback */ |
489 | if (!pdev->driver || !pdev->driver->sriov_configure) { | 492 | if (!pdev->driver || !pdev->driver->sriov_configure) { |
490 | dev_info(&pdev->dev, "Driver doesn't support SRIOV configuration via sysfs\n"); | 493 | dev_info(&pdev->dev, "Driver doesn't support SRIOV configuration via sysfs\n"); |
491 | return -ENOSYS; | 494 | ret = -ENOENT; |
495 | goto exit; | ||
492 | } | 496 | } |
493 | 497 | ||
494 | if (num_vfs == 0) { | 498 | if (num_vfs == 0) { |
495 | /* disable VFs */ | 499 | /* disable VFs */ |
496 | ret = pdev->driver->sriov_configure(pdev, 0); | 500 | ret = pdev->driver->sriov_configure(pdev, 0); |
497 | if (ret < 0) | 501 | goto exit; |
498 | return ret; | ||
499 | return count; | ||
500 | } | 502 | } |
501 | 503 | ||
502 | /* enable VFs */ | 504 | /* enable VFs */ |
503 | if (pdev->sriov->num_VFs) { | 505 | if (pdev->sriov->num_VFs) { |
504 | dev_warn(&pdev->dev, "%d VFs already enabled. Disable before enabling %d VFs\n", | 506 | dev_warn(&pdev->dev, "%d VFs already enabled. Disable before enabling %d VFs\n", |
505 | pdev->sriov->num_VFs, num_vfs); | 507 | pdev->sriov->num_VFs, num_vfs); |
506 | return -EBUSY; | 508 | ret = -EBUSY; |
509 | goto exit; | ||
507 | } | 510 | } |
508 | 511 | ||
509 | ret = pdev->driver->sriov_configure(pdev, num_vfs); | 512 | ret = pdev->driver->sriov_configure(pdev, num_vfs); |
510 | if (ret < 0) | 513 | if (ret < 0) |
511 | return ret; | 514 | goto exit; |
512 | 515 | ||
513 | if (ret != num_vfs) | 516 | if (ret != num_vfs) |
514 | dev_warn(&pdev->dev, "%d VFs requested; only %d enabled\n", | 517 | dev_warn(&pdev->dev, "%d VFs requested; only %d enabled\n", |
515 | num_vfs, ret); | 518 | num_vfs, ret); |
516 | 519 | ||
520 | exit: | ||
521 | mutex_unlock(&iov->dev->sriov->lock); | ||
522 | |||
523 | if (ret < 0) | ||
524 | return ret; | ||
525 | |||
517 | return count; | 526 | return count; |
518 | } | 527 | } |
519 | 528 | ||
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index cb17db242f30..8dd38e69d6f2 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h | |||
@@ -270,7 +270,7 @@ struct pci_sriov { | |||
270 | u16 driver_max_VFs; /* max num VFs driver supports */ | 270 | u16 driver_max_VFs; /* max num VFs driver supports */ |
271 | struct pci_dev *dev; /* lowest numbered PF */ | 271 | struct pci_dev *dev; /* lowest numbered PF */ |
272 | struct pci_dev *self; /* this PF */ | 272 | struct pci_dev *self; /* this PF */ |
273 | struct mutex lock; /* lock for VF bus */ | 273 | struct mutex lock; /* lock for setting sriov_numvfs in sysfs */ |
274 | resource_size_t barsz[PCI_SRIOV_NUM_BARS]; /* VF BAR size */ | 274 | resource_size_t barsz[PCI_SRIOV_NUM_BARS]; /* VF BAR size */ |
275 | }; | 275 | }; |
276 | 276 | ||