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

[Jean Delvare]

Hi Greg,

On Thu, 17 Jul 2008 00:07:59 -0700, 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.
>
> (...)  I'll try to get
> to this by Monday, but my original point still stands, this was
> implemented for a reason,

Not a good enough argument, sorry. There have been many cases in the
past where code has been withdrawn after some times because we
realized that we got it wrong in the first place.

So, please explain what the current code is good for. Honestly, my
initial reaction to Milton's proposal was "what an idiot, this flag is
there for an obvious safety reason and we don't want to remove it" but
after reading both his arguments and the code, I found that I have
nothing to backup my claim. If you do, please let us know your
technical reasons.

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

If you are suggesting to blindly set the flag to all PCI drivers (or
even just all the ones which make use of the driver_data field -
doesn't make a difference), this simply shows how useless this flag is.
If you don't, then one would have to check the code of all drivers and
add validation code for the driver_data value; but then this no longer
falls into the "trivial" category.

Thanks,
-- 
Jean Delvare
--
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/