All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eran Liberty <liberty@extricom.com>
To: linux-kernel@vger.kernel.org
Subject: [PATCH 2.6.24-rc4] PCI: refuse to re-add a device to a bus upon pci_scan_child_bus()
Date: Sun, 20 Jul 2008 13:31:54 +0300	[thread overview]
Message-ID: <4883141A.4080209@extricom.com> (raw)
In-Reply-To: <48591941.4070408@extricom.com>

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

Dear Penguins,

As  a follow up on my own post ( http://lkml.org/lkml/2008/6/18/195 ), I 
have  solved a small bug/misbehavior within the pci_scan_single_device().

The misbehavior manifest itself upon calling pci_scan_child_bus() with a 
bus which already contain some devices. After scanning is done devices 
that already existed will be present twice in the PCI bus devise list. 
Once with is_added indication and once without. Trying to add this bus 
will cause a resource conflict as the same device is already present and 
initialized.

This patch will simply prevent a device to be added to a bus list if it 
is already there.

Points to consider:
1. I am not a PCI Guru and might have over looked the bigger picture. Is 
it OK to prevent a device of being re-added?
2. I have decided that two devices are, in fact, the same instance if it 
has the same: vendor, device, and devfn. Is there a finer test for a 
device identity?

p.s. As per http://www.kernel.org/pub/linux/docs/lkml/#s1-10 it is 
preferred for Mozilla Firefox clients to attach the patch, hence my 
patch is attached.

Liberty

Signed-off-by: Eran Liberty <liberty@extrictom.org>
---

[-- Attachment #2: refuse_readd_pci_device.patch --]
[-- Type: text/x-patch, Size: 675 bytes --]

Index: drivers/pci/probe.c
===================================================================
--- drivers/pci/probe.c	(revision 102)
+++ drivers/pci/probe.c	(revision 103)
@@ -996,11 +996,21 @@
 struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
 {
 	struct pci_dev *dev;
+	struct list_head *ln;
 
 	dev = pci_scan_device(bus, devfn);
 	if (!dev)
 		return NULL;
 
+	/* refuse to re-add a device */
+	list_for_each(ln, &bus->devices) {
+		struct pci_dev *d = pci_dev_b(ln);
+		if ((d->vendor == dev->vendor) &&
+		    (d->device == dev->device) &&
+		    (d->devfn == dev->devfn))
+			return NULL;
+	}
+
 	pci_device_add(dev, bus);
 
 	return dev;

  reply	other threads:[~2008-07-20 10:41 UTC|newest]

Thread overview: 141+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-18 14:18 2.6.24-rc4: pci_remove_bus_device() => pci_scan_child_bus() => pci_bus_add_devices bug? Eran Liberty
2008-07-20 10:31 ` Eran Liberty [this message]
2008-07-20 16:48   ` [PATCH 2.6.24-rc4] PCI: refuse to re-add a device to a bus upon pci_scan_child_bus() Eran Liberty
2008-07-21 19:18 ` [PATCH 2.6.26] " Eran Liberty
2008-07-21 19:49   ` Matthew Wilcox
2008-07-22  8:21     ` eran liberty
2008-07-22 11:49       ` Matthew Wilcox
2008-07-22 13:08         ` Eran Liberty
2008-07-22 13:14         ` Eran Liberty
2008-07-22 14:13           ` Matthew Wilcox
2008-07-22 15:25             ` Eran Liberty
2008-07-22 16:52               ` Matthew Wilcox
2008-07-22 17:41                 ` Eran Liberty
2008-07-22 18:11                   ` Matthew Wilcox
2008-07-23 18:31                     ` Eran Liberty
2008-07-27 11:01                       ` Eran Liberty
2008-07-27 15:08                         ` Matthew Wilcox
2008-08-18  8:08 ` ftrace introduces instability into kernel 2.6.27(-rc2,-rc3) Eran Liberty
2008-08-18 15:07   ` Steven Rostedt
2008-08-18 15:07     ` Steven Rostedt
2008-08-18 15:47     ` Mathieu Desnoyers
2008-08-18 15:47       ` Mathieu Desnoyers
2008-08-18 16:12       ` Steven Rostedt
2008-08-18 16:12         ` Steven Rostedt
2008-08-18 17:04         ` Mathieu Desnoyers
2008-08-18 17:04           ` Mathieu Desnoyers
2008-08-18 17:21       ` Scott Wood
2008-08-18 17:21         ` Scott Wood
2008-08-18 18:27         ` Steven Rostedt
2008-08-18 18:27           ` Steven Rostedt
2008-08-18 18:29           ` Scott Wood
2008-08-18 18:29             ` Scott Wood
2008-08-19  1:53           ` Benjamin Herrenschmidt
2008-08-19  1:53             ` Benjamin Herrenschmidt
2008-08-19  2:28             ` Steven Rostedt
2008-08-19  2:28               ` Steven Rostedt
2008-08-19  2:39               ` Benjamin Herrenschmidt
2008-08-19  2:39                 ` Benjamin Herrenschmidt
2008-08-19  2:41                 ` Steven Rostedt
2008-08-19  2:41                   ` Steven Rostedt
2008-08-19  2:47                   ` Mathieu Desnoyers
2008-08-19  2:47                     ` Mathieu Desnoyers
2008-08-19  3:32                     ` Steven Rostedt
2008-08-19  3:32                       ` Steven Rostedt
2008-08-19  3:36                       ` Mathieu Desnoyers
2008-08-19  3:36                         ` Mathieu Desnoyers
2008-08-19  4:00                         ` Steven Rostedt
2008-08-19  4:00                           ` Steven Rostedt
2008-08-19 16:47                     ` Steven Rostedt
2008-08-19 16:47                       ` Steven Rostedt
2008-08-19 17:34                       ` Mathieu Desnoyers
2008-08-19 17:34                         ` Mathieu Desnoyers
2008-08-19 21:08                         ` Steven Rostedt
2008-08-19 21:08                           ` Steven Rostedt
2008-08-20  9:40                           ` Nick Piggin
2008-08-20  9:40                             ` Nick Piggin
2008-08-19 21:47                         ` Benjamin Herrenschmidt
2008-08-19 21:47                           ` Benjamin Herrenschmidt
2008-08-19 23:58                           ` Jeremy Fitzhardinge
2008-08-19 23:58                             ` Jeremy Fitzhardinge
2008-08-20  1:17                             ` Benjamin Herrenschmidt
2008-08-20  1:17                               ` Benjamin Herrenschmidt
2008-08-19  2:56                 ` Benjamin Herrenschmidt
2008-08-19  2:56                   ` Benjamin Herrenschmidt
2008-08-19  3:12                   ` Steven Rostedt
2008-08-19  3:12                     ` Steven Rostedt
2008-08-19  4:17                     ` Benjamin Herrenschmidt
2008-08-19  4:17                       ` Benjamin Herrenschmidt
2008-08-20  7:18                       ` Benjamin Herrenschmidt
2008-08-20  7:18                         ` Benjamin Herrenschmidt
2008-08-20 13:14                         ` Steven Rostedt
2008-08-20 13:14                           ` Steven Rostedt
2008-08-20 13:19                           ` Steven Rostedt
2008-08-20 13:19                             ` Steven Rostedt
2008-08-20 13:36                             ` Eran Liberty
2008-08-20 13:36                               ` Eran Liberty
2008-08-20 13:43                               ` Steven Rostedt
2008-08-20 13:43                                 ` Steven Rostedt
2008-08-20 14:02                                 ` Eran Liberty
2008-08-20 14:02                                   ` Eran Liberty
2008-08-20 14:55                                   ` Jon Smirl
2008-08-20 14:55                                     ` Jon Smirl
2008-08-20 15:23                                     ` Steven Rostedt
2008-08-20 15:23                                       ` Steven Rostedt
2008-08-20 18:23                                     ` Eran Liberty
2008-08-20 18:23                                       ` Eran Liberty
2008-08-20 18:33                                       ` Steven Rostedt
2008-08-20 18:33                                         ` Steven Rostedt
2008-08-20 15:27                                   ` Steven Rostedt
2008-08-20 15:27                                     ` Steven Rostedt
2008-08-20 21:37                                   ` Benjamin Herrenschmidt
2008-08-20 21:37                                     ` Benjamin Herrenschmidt
2008-08-20 14:16                           ` Josh Boyer
2008-08-20 14:16                             ` Josh Boyer
2008-08-20 14:22                             ` Steven Rostedt
2008-08-20 14:22                               ` Steven Rostedt
2008-08-20 14:50                               ` Josh Boyer
2008-08-20 14:50                                 ` Josh Boyer
2008-09-15 16:30                                 ` [PATCH 2.6.26] SERIAL DRIVER: Handle Multiple consecutive sysrq from the serial Eran Liberty
2008-09-15 16:30                                   ` Eran Liberty
2008-09-17 23:46                                   ` Andrew Morton
2008-09-18  6:58                                     ` Eran Liberty
2008-09-18  6:58                                       ` Eran Liberty
2008-08-20 21:36                           ` ftrace introduces instability into kernel 2.6.27(-rc2,-rc3) Benjamin Herrenschmidt
2008-08-20 21:36                             ` Benjamin Herrenschmidt
2008-08-20 21:44                             ` Steven Rostedt
2008-08-20 21:44                               ` Steven Rostedt
2008-08-18 18:47         ` Steven Rostedt
2008-08-18 18:47           ` Steven Rostedt
2008-08-18 18:56           ` Scott Wood
2008-08-18 19:28             ` Steven Rostedt
2008-08-18 18:25     ` Eran Liberty
2008-08-18 18:25       ` Eran Liberty
2008-08-18 18:41       ` Mathieu Desnoyers
2008-08-18 18:41         ` Mathieu Desnoyers
2008-08-19  1:54         ` Benjamin Herrenschmidt
2008-08-19  1:54           ` Benjamin Herrenschmidt
2008-08-19  9:56         ` Eran Liberty
2008-08-19  9:56           ` Eran Liberty
2008-08-19 13:02           ` Mathieu Desnoyers
2008-08-19 13:02             ` Mathieu Desnoyers
2008-08-19 21:46             ` Benjamin Herrenschmidt
2008-08-19 21:46               ` Benjamin Herrenschmidt
2008-08-18 18:50       ` Steven Rostedt
2008-08-18 18:50         ` Steven Rostedt
2008-08-19 12:09         ` Eran Liberty
2008-08-19 12:09           ` Eran Liberty
2008-08-19 13:05           ` Mathieu Desnoyers
2008-08-19 13:05             ` Mathieu Desnoyers
2008-08-19 14:21             ` Eran Liberty
2008-08-19 14:21               ` Eran Liberty
2008-08-19 14:42               ` Mathieu Desnoyers
2008-08-19 14:42                 ` Mathieu Desnoyers
2008-08-19 20:15           ` Steven Rostedt
2008-08-19 20:15             ` Steven Rostedt
2008-08-20 11:18             ` Eran Liberty
2008-08-20 11:18               ` Eran Liberty
2008-08-20 13:12               ` Steven Rostedt
2008-08-20 13:12                 ` Steven Rostedt
2008-08-19  1:51     ` Benjamin Herrenschmidt
2008-08-19  1:51       ` Benjamin Herrenschmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4883141A.4080209@extricom.com \
    --to=liberty@extricom.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.