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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Aug 14, 2008, at 5:12 PM, Greg KH wrote:

On Wed, Aug 06, 2008 at 09:31:18AM +0200, Jean Delvare wrote:
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.

Fair enough :)

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.

The technical reason was that this flag was needed to let some drivers
work properly with the new_id file, right?

If the flag goes away, they break from what I can tell.

That is not a detailed technical argument, its an  assumption.


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.

It's pretty "trivial" to look to see if the field is set in the pci_id
structure, that should be all that is needed, right?


So please identify at least one such driver where the only correct answer is 0. I even made it easy for you, i identified which drivers set dynamic_id and how. I identified specific drivers where its the wrong answer.

So: If you are arguing the code is still needed, please identify at least one case that it is correct. (Oh, I don't buy that if 0 is safe but 1 is equally safe, that the existing code is correct).

milton

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux