Messages in this thread | | | Date | Wed, 14 Feb 2007 11:47:08 -0800 | From | Zachary Amsden <> | Subject | Re: [PATCH 9/11] Panic delay fix |
| |
Pavel Machek wrote: > On Thu 2007-02-08 07:36:12, Rusty Russell wrote: > >> On Wed, 2007-02-07 at 12:35 +0000, Pavel Machek wrote: >> >>> Ugh, it sounds like paravirt is more b0rken then I thought. It should >>> always to the proper delay, then replace those udelays that are not >>> needed on virtualized hardware with something else. >>> >>> Just magically defining udelay into nop is broken. >>> >> We'd have to audit and figure out what udelays are for hardware and >> which are not, but the evidence is that the vast majority of them are >> for hardware and not needed for virtualization. >> > > You did not time to do the full audit, so you just did #define. >
Yes, of course. Since 99% of the drivers are completely irrelevant for paravirt, and 99% of the udelays are in drivers, there isn't much point to auditing a bunch of code we're not even going to be affected by. The default case for udelay is it is not needed.
>> Changing udelay to "hardware_udelay" or something all over the kernel >> would have delayed the paravirt_ops merge by an infinite amount 8) >> > > And here you claim you could not do the right thing, because people > would notice you are doing huge search/replace without audit, and > would stop you. So you simply hidden it from them :-(. >
What ludicrousness is this? Hidden what? That the default case for udelay is that it is not needed?
> Plus... udelay() should just work under virtualization, right? You get > slightly slower kernel, but still working, so the "full audit" is not > as hard as you are telling me. >
Save the time of doing a useless full audit and making sure we didn't accidentally redefine or misspell some symbol on a bunch of architectures we aren't even set up to compile for.
> Just replace udelay() with hardware_udelay() on places that matter in > your workload... >
That's inconsistent. We would be doing 2 SCSI drivers, part of the IDE code, some i386 arch code, some random places in the kernel... and now nobody else knows whether to use udelay or hardware_udelay and the code gets jumbled to the point that it is useless because there is no clear distinction between the two. It is non-trivial to come up with a list of source files that we have to actually do this to. One C-file calls a shared routine in a library, and now you've got a hidden udelay that you have absolutely no way of detecting. The right thing to do if you want to do it on a line by line basis is exactly the opposite. Remove udelay and find out what breaks. Bugs are easier to find and fix than hidden code. If I were to do it on a line by line basis, I would chose to replace udelay() with real_time_udelay() for those places that actually need it.
Zach - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |