Message-ID: <20081203043804.GA26130@ldl.fc.hp.com>
Date:	Tue, 2 Dec 2008 21:38:04 -0700
From:	Alex Chiang <achiang@...com>
To:	Trent Piepho <xyzzy@...akeasy.org>
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>, eike-kernel@...tec.de,
	gregkh@...e.de
Subject: Re: Problems with fakephp

* Trent Piepho <xyzzy@...akeasy.org>:
> On Mon, 1 Dec 2008, Alex Chiang wrote:
> > I think there are now enough ideas in this thread that they're
> > starting to get confusing.
> >
> > 	1) Function vs. device removal
> > 	2) User interface
> > 	3) Existing fakephp bugs
> >
> > For (1), do you need function level hotplug? Or will you be able
> > to get away with device level?
> 
> While I have some hardware the can use function level hotplug (secondary
> functions can be controlled by registers in the primary function), it's not
> something *I* make use of.  But function level hotplug has been there for
> years so it seems like a regression to remove that ability and break the
> existing interface.

That seems reasonable.

> I guess my first though is should be there a new interface, as part of the
> pci core rather than pci hotplug, for adding/removing devices from Linux's
> view?  By devices I mean in the Linux "struct device" sense, so PCI
> functions.
> 
> I think that seems reasonable.  fakephp isn't the best interface.  My patch
> to add "remove" to pci-sysfs ended up being very simple, unless there's a
> serious flaw in it I've overlooked.

I think we should definitely merge your 'remove' attribute patch
for PCI functions. That should be independent of the rest of our
discussion.

It will probably help the SR-IOV folks too.

> So once we have that the question becomes how to keep some compatibility
> with the old fakephp interface.  Either a new legacy compat module like
> I've done or by fixing fakephp.
> 
> I'm more inclined to have the new legacy compat module:
> 
> - It's quite a bit simpler than fakephp so far.
> - It already works better than fakephp ever did.  fakephp can't do
> recursive bridge removal and won't co-exist well with a new pci core
> remove/add interface.
> - Fakephp's use of devices as "slots" appears to be fundamentally at odds
> with the hotplug core.  It's just going to cause problems in the future.
> The new compat module doesn't use hotplug at all, so it shouldn't get in
> the way.

Maybe we should just replace fakephp wholesale with your new
driver?

Or coming at it from another angle, I don't see what benefit
we'll have from keeping both fakephp and your driver. And if
fakephp is as broken as you describe, then it will only cause
more confusion if a user loads both fakephp and legacy_fakephp.

If the user removes a bridge via the correct legacy_fakephp
interface, fakephp won't notice, and we'll just have a broken
mess on our hands.

It would be better to have just one, correctly working fakephp,
even if the implementation is 100% different and truly not even a
"real" hotplug driver.

I think the way forward is:

	- merge in the function level hotplug patch
	- wholesale replacement of fakephp with new fakephp
	- schedule new fakephp for deprecation
	- encourage anyone who wants function level hotplug to
	  use the 'remove' attribute

Thoughts? Jesse, Willy, Eike, Greg?

Thanks.

/ac

--
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