Re: [PATCH/RFC] pci: dynids.use_driver_data considered harmful

From: Milton Miller
Date: Thu Jul 17 2008 - 10:34:34 EST



On Jul 17, 2008, at 2:07 AM, Greg KH wrote:

On Wed, Jul 16, 2008 at 05:18:18AM -0500, Milton Miller wrote:

Greg,

Please respond to this email and explain why the patch

pci: dynids.use_driver_data considered harmful

http://www.ussg.iu.edu/hypermail/linux/kernel/0807.1/index.html#2188

should not be applied. I am not arguing the correctness of
the removed code, rather its utility and benefit to the linux
community.

As far as I can tell, the code only succeeds in limiting the
usefulness of the pci dynamic id addition mechanism.
...

Sorry, I'm on the road right now and will not get back until Friday.
Then I have the big merges with Linus to get through. I'll try to get
to this by Monday, but my original point still stands, this was
implemented for a reason, saying that not enough drivers use it properly
does not make the need for it to go away. It is required for them, so
perhaps the other 419 drivers also need to have the flag set. That's
pretty trivial to do, right?


Fine, I'll give you a little time. But I want to hear specifics how it
helps drivers. I have shown how it hurts many drivers. My arguement
is that once we set the flag on the drivers, we will end up with it set
on all drivers or the remaining drivers will not care. There were 413+
drivers in linux-next that were compiled by allyesconfig, about 150 used
driver data.

If the purpose is to enforce range checking, then I'll start working on
patches for those 100 drivers to do that. But I don't see why a binary
flag helps any driver. The flag is not "I will accept a additional id
table", its "I will accept the additional driver data that I need to
operate" on my dynamically added id table.

milton

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