Re: [PATCH 9/11] Panic delay fix

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

 



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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux