From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Yinghai Lu <yinghai@kernel.org>,
Jesse Barnes <jbarnes@virtuousgeek.org>,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
Matthew Wilcox <matthew@wil.cx>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH 1/7] PCI: Make sriov work with hotplug remove
Date: Tue, 24 Jan 2012 15:34:08 +1100 [thread overview]
Message-ID: <1327379648.19850.31.camel@pasglop> (raw)
In-Reply-To: <CA+55aFwEbbq0yuN4mAtfDSbOE86mAWsBBz+pgBzn8_QHVk+B3g@mail.gmail.com>
On Mon, 2012-01-23 at 11:34 -0800, Linus Torvalds wrote:
> On Mon, Jan 23, 2012 at 10:45 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Why isn't this magically true in this case? If some *other* random
> > entry than the one that is being iterated over can magically be
> > removed, then the whole thing is just pure and utter crap, and no
> > amount of list maintenance can ever fix it.
> >
> > So explain.
>
> Ahh. I finally understand what's going on. The virtual device attached
> to a physical device can go away, and it's on the same damn list.
>
> That's broken. Virtual devices set up by a physical device should be
> *children* of the physical device, not "siblings". But that's
> apparently not what the broken virtual PCI layer does.
Thank the PCI SIG for that ... they are sibling functions (or even
devices in some case) of the PF :-(
> So I think that there are two possible solutions:
>
> (a) fix the virtual devices that are attached to physical devices to
> really be children of the physical device, with the physical device as
> a "bridge" to that virtual bus.
This will confuse various other aspects of the PCI code since they are
really siblings from an addressing standpoint (ie bus/dev/fn)
Cheers,
Ben.
> I think this is the correct solution from any sane standpoint (now the
> topology of the device tree actually matches the logical
> relationship), which is why I think this is the RightThing(tm) to do.
> And it should automatically fix this insane issue where removing a
> device from a bus can remove *multiple* devices from the same list.
>
> (b) if that isn't an option, and the virtual devices make a mess, at
> least don't make the code uglier, just do something like:
>
> while (!list_empty(&bus->devices)) {
> struct pci_dev *dev = list_first_entry(struct pci_dev, bus_list);
>
> pci_stop_bus_device(dev);
> }
>
> which at least isn't butt ugly. Add a large comment about how
> pci_stop_bus_device() can remove multiple devices due to the virtual
> devices having been done badly.
>
> Who is in charge of the whole 'is_virtfn' mess? How realistic is it to
> fix that crud?
>
> Linus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-01-24 4:34 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-21 9:52 [PATCH 0/7] PCI: pcie hotplug related patch Yinghai Lu
2012-01-21 9:52 ` [PATCH 1/7] PCI: Make sriov work with hotplug remove Yinghai Lu
2012-01-23 16:06 ` Linus Torvalds
2012-01-23 18:30 ` Yinghai Lu
2012-01-23 18:45 ` Linus Torvalds
2012-01-23 19:34 ` Linus Torvalds
2012-01-23 19:59 ` Yinghai Lu
2012-01-23 20:48 ` Yinghai Lu
2012-01-23 22:35 ` Linus Torvalds
2012-01-24 4:34 ` Benjamin Herrenschmidt [this message]
2012-01-23 19:36 ` Yinghai Lu
2012-01-23 19:44 ` Linus Torvalds
2012-01-23 21:34 ` Yinghai Lu
2012-01-23 22:30 ` Yinghai Lu
2012-01-23 22:38 ` Linus Torvalds
2012-01-21 9:52 ` [PATCH 2/7] PCI: Separate pci_bus_read_dev_vendor_id from pci_scan_device Yinghai Lu
2012-01-21 9:52 ` [PATCH 3/7] pciehp: Checking pci conf reading to new added device instead of sleep 1s Yinghai Lu
2012-01-21 9:52 ` [PATCH 4/7] pciehp: print out link status when dlla get active Yinghai Lu
2012-01-21 9:52 ` [PATCH 5/7] pciehp: Add pcie_wait_link_not_active() Yinghai Lu
2012-01-21 9:52 ` [PATCH 6/7] pciehp: Add Disable/enable link functions Yinghai Lu
2012-01-23 16:13 ` Linus Torvalds
2012-01-24 5:36 ` Yinghai Lu
2012-01-21 9:52 ` [PATCH 7/7] pciehp: Disable/enable link during slot power off/on Yinghai Lu
2012-01-21 10:26 ` [PATCH 0/7] PCI: pcie hotplug related patch Yinghai Lu
2012-01-27 18:36 ` Jesse Barnes
2012-01-27 18:58 ` Yinghai Lu
2012-01-30 3:42 ` Kenji Kaneshige
-- strict thread matches above, loose matches on Subject: below --
2012-01-27 18:55 [PATCH -v2 " Yinghai Lu
2012-01-27 18:55 ` [PATCH 1/7] PCI: Make sriov work with hotplug remove Yinghai Lu
2012-01-27 19:43 ` Jesse Barnes
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=1327379648.19850.31.camel@pasglop \
--to=benh@kernel.crashing.org \
--cc=jbarnes@virtuousgeek.org \
--cc=kaneshige.kenji@jp.fujitsu.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=matthew@wil.cx \
--cc=torvalds@linux-foundation.org \
--cc=yinghai@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.