Message-ID: <Pine.LNX.4.58.0812010356410.2943@shell2.speakeasy.net>
Date:	Mon, 1 Dec 2008 05:00:05 -0800 (PST)
From:	Trent Piepho <xyzzy@...akeasy.org>
To:	Alex Chiang <achiang@...com>
cc:	"Darrick J. Wong" <djwong@...ibm.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	linux-pci <linux-pci@...r.kernel.org>,
	Matthew Wilcox <matthew@....cx>
Subject: Problems with fakephp

On Fri, 28 Nov 2008, Alex Chiang wrote:
> * Trent Piepho <xyzzy@...akeasy.org>:
> Let's try and find a solution that will work for everyone. I
> don't think that going back to one entry in /sys/bus/pci/slots
> per function is a solution.
>
> Last time, Willy proposed a few ideas in this thread:
>
> 	http://lkml.org/lkml/2008/9/9/28

It looks like you weren't opposed to reverting the original patch.  I
think that should be done for 2.6.27 and 2.6.28, to restore the existing
interface.  I can made the patch.  I realize that fakephp's (ab)use of the
hotplug system doesn't fit in with your plans, and there's no reason not
to do anything about that.  But it should be done the right way.  fakephp
was perhaps not the best interface for getting linux to re-scan for PCI
devices and for removing existing ones, but none the less it was the long
established and only interface for this.

What should be done is to create a better interface for disabling PCI
devices/functions/bridges and re-scanning.  Then create a compatibility
system that will allow software the used fakephp to continue to work.  Now
fakephp can change to be more like a real hotplug driver.  At some time in
future, the compatibility layer, which will have been listed in the
feature removal schedule, can be removed.

You might also be interested to know that fakephp as it is now has a bug
in it.  Another reason to revert the change.  Removing a bridge doesn't
work so well.

# lspci -tv
           +-10.0  LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ul
           \-11.0-[0000:02]--+-00.0  Advanced Micro Devices [AMD] 79c970 [PCn
                             +-01.0  Ensoniq ES1371 [AudioPCI-97]
                             \-02.0  VMware Inc Unknown device 0770
# cat fake[6-9]/address
0000:00:11
0000:02:00
0000:02:01
0000:02:02
# echo 0 > fake6/power
# lspci -tv
           \-10.0  LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ul
[ 00:11 and everything behind it are gone, as expected ]
# ls
fake1/  fake2/  fake3/  fake4/  fake5/  fake7/  fake8/  fake9/
[ notice that fake[7-9] are still there! ]
# cat fake6/address
Segmentation fault
BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c02089c3>] address_read_file+0x23/0x70

The problem is that fakephp's slots aren't really slots, they are devices.
If a PCI device goes away and it's not done with fakephp, then fakephp
doesn't know about it and the fakephp "slot" sticks around pointing to a
removed device.  I think I can come up with a better system that fixes
these problems.

I also noticed this in drivers/pci/probe.c:pci_scan_device()

list_for_each_entry(slot, &bus->slots, list)
	if(PCI_SLOT(devfn) == slot->number)
		dev->slot = slot;

This appears to assume that there will only one slot with a given number
on the same bus.  But if fakephp and real hotplug driver are both loaded,
this won't be the case anymore, will it?

I also have to wonder, if the bus + slot number is unique, then why not
use that to name the slot's kobject?  Instead of something like "fake123",
where the number is basically meaningless.  If you have a meaningful
unique ID, why not use that?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists