Re: Problems with fakephp

From: Alex Chiang
Date: Tue Dec 02 2008 - 23:38:25 EST


* Trent Piepho <xyzzy@xxxxxxxxxxxxx>:
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/