2.4.23-uv3 patch set released

2.4.23-uv3 patch set released

[James Bourne]

The Update Version patchset is a set of patches which include only fatal
compile/runtime bug fixes and security updates for the current kernel
version.  This patch set can be used in production environments for those
who wish to run 2.4.23, but do not use vendor kernels and at the same time
require patches which add to the stability of the current release kernel
version.  This is a patch set only, it does not include kernel source.

Current version is 2.4.23-uv3 which contains many patches pulled from the
2.4-bk mailing list and adds the last 5 patches in the list below.

The complete URL to the patch set is
http://www.hardrock.org/kernel/current-updates/linux-2.4.23-updates.patch

Individual patches can be viewed and downloaded from
http://www.hardrock.org/kernel/current-updates/

This patch set only contains and will only contain security updates and
fixes for the latest kernel version.  Each individual patch contains text
WRT the patch itself and the creator of the patch, I will try to keep doing
that as standard reference for the complete collection.

Please send bug reports to jbourne@hardrock.org and CC
linux-kernel@vger.kernel.org.

Patch specifics are:
linux-2.4.23-updates.patch: Contains all the patches below.         

linux-2.4.23-extraversion.patch: Updated the extraversion in the Makefile

linux-2.4.23-file_lock_acct.patch: Remove broken file lock accounting

linux-2.4.23-ht-detect.patch: Fixup smb_boot_cpus(): Fix HT detection bug

linux-2.4.23-ipfw_compat_oops.patch: fix for a known bug in the netfilter

linux-2.4.23-ll_rw_blk_race_fix.patch: from -aa tree: Fix potential fsync()
	race condition

linux-2.4.23-lockd_reclaim.patch: Drop module count if lockd reclaimer
	thread failed to start

linux-2.4.23-no_idt.patch: fix reboot/no_idt bug

linux-2.4.23-oom_kill.patch: out_of_memory() locking issue

linux-2.4.23-rbs_clobber.patch: ia64: Fix a bug in sigtramp() which
	corrupted ar.rnat when unwinding across a signal trampoline
	(in user space).  Reported by Laurent Morichetti.

linux-2.4.23-root_rlim.patch: Make root a special case for per-user process
	limits.

linux-2.4.23-rtc-compile.patch: Patch to allow RTC to compile properly on
	some systems, see http://lkml.org/lkml/2003/12/1/150

duplicate-pid-fix-2.4.23.patch: Without this, duplicate pids can be
	allocated, which will make one of them unkillable (signals are
	deliverd to only one of them), and this can be exploitable (I don't
	know for sure, but maybe, like brk()).

irda-log-buster-2.4.23.patch: I just ran 2.4.23, and after a few min the
	disk reached 100% capacity. A quick check lead to to oversized
	kernel log, and to the following changeset.

iseries-saverestore-flags-2.4.23.patch: [PPC64] Fix save_flags/restore_flags
	on iSeries.

mct_u232-baudratefix-2.4.23.patch: Fix a problem in the 'mct_u232' driver
	whereby output data gets held up in the USB/RS-232 adapter for RS-232
	devices which don't assert the 'CTS' signal.

odirect-offset-2.4.23.patch: here's an obvious mistake i made in the NFS
	O_DIRECT implementation.  A missing type cast causes the offset of direct
	read and write requests to wrap at 4GB.

ppc64-pmc-compile-fix-2.4.23.patch: [PPC64] Fix compile error in
	arch/ppc64/kernel/pmc.c

ppc64-smp_call_function_late_ipi-2.4.23.patch: [PPC64] Fix smp_call_function
	so we don't crash if an IPI is very late.

rtc-leak-2.4.23.patch: [PATCH] Fix rtc leak

sparc32-build-fix-2.4.23.patch: [SPARC32]: Fix build after asm/system.h
	include was added to linux/spinlock.h

usb-serial-edgeport-counter-and-alignment-2.4.23.patch: [PATCH] USB: fix bug
	when errors happen in ioedgeport driver

linux-2.4.23-airo-oops-fix.patch: [wireless airo] Delay MIC activation to
	prevent Oops

linux-2.4.23-fix-airo-pci-registration.patch: [wireless airo] Fix PCI
	registration

linux-2.4.23-ide-busy-race-fix.patch: Daniel Lux: Fix IDE busy-wait race

linux-2.4.23-init-ioc3-timer.patch: [PATCH] Initialize ioc3_timer before use

linux-2.4.23-raid1-blocksize.patch: [PATCH] Fix RAID1 blocksize check


Regards
James Bourne

-- 
James Bourne                  | Email:            jbourne@hardrock.org          
Unix Systems Administrator    | WWW:           http://www.hardrock.org
Custom Unix Programming       | Linux:  The choice of a GNU generation
----------------------------------------------------------------------
 "All you need's an occasional kick in the philosophy." Frank Herbert  

Re: 2.4.23-uv3 patch set released

[Marcelo Tosatti]

On Mon, 29 Dec 2003, James Bourne wrote:

> linux-2.4.23-ide-busy-race-fix.patch: Daniel Lux: Fix IDE busy-wait race

I screwed up the merge of this patch, you also want to apply "Fix IDE
busywait merge" (its the next changeset after this one).

Re: 2.4.23-uv3 patch set released

[Linus Torvalds]

On Tue, 30 Dec 2003, Marcelo Tosatti wrote:
> 
> On Mon, 29 Dec 2003, James Bourne wrote:
> 
> > linux-2.4.23-ide-busy-race-fix.patch: Daniel Lux: Fix IDE busy-wait race
> 
> I screwed up the merge of this patch, you also want to apply "Fix IDE
> busywait merge" (its the next changeset after this one).

I really think that the patch is wrong - it's just insane to "test one 
more time in case we got an interrupt", when the code around it plays with 
the interrupt flag all the time. Especially when it is supposed to wait 
for several timer interrupts while doing this. It appears that the timeout 
itself is just too damn borderline.

I suspect that anything in IDE that uses that idiotic "local_irq_set()"  
macro is just broken, and should be rewritten to explicitly enable 
interrupts. As it is, the code is just incredibly _strange_, and it 
actually enables interrupts at all the wrong places.

Anyway, in ide_wait_stat(), the "timeout" value is always either 
"WAIT_DRQ" (5*HZ/100) or it is "WAIT_READY" (3*HZ/100). And look at 
WAIT_READY a bit more: 

	#if defined(CONFIG_APM) || defined(CONFIG_APM_MODULE)
	#define WAIT_READY      (5*HZ)          /* 5sec - some laptops are very slow */
	#else
	#define WAIT_READY      (3*HZ/100)      /* 30msec - should be instantaneous */
	#endif /* CONFIG_APM || CONFIG_APM_MODULE */

I bet that the _real_ problem is this. That "3*HZ/100" value is just too 
damn short. It has already been increased to 5*HZ for anything that has 
APM enabled, but anybody who doesn't use APM gets a _really_ short 
timeout.

My suggestion: change the non-APM timeout to something much larger. Make
it ten times bigger, rather than leaving it at a value that us so small
that a single interrupt could make a difference..

In fact, right now a single timer interrupt on 2.4.x is the difference
between waiting 20ms and 30ms. That's a _big_ relative difference.

Andrew - unless you disagree, I'd just be inlined to change both the DRQ
and READY timeouts to be a bit larger. On working hardware it shouldn't
matter, so how about just making them both be something like 100 msec (and
leave that strange really big APM value alone).

On 2.6.x, the higher timer frequency should make the time keeping more 
exact anyway (29-30ms rather than 20-30ms), but that's a small random 
detail..

			Linus

no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Marcelo Tosatti]

Daniel, Steve,

Linus believes the reason for the "no DRQ after issuing WRITE" (and
further problems caused by it) is the too small timeout used in
ide_wait_stat().

Can you guys please change this line in include/linux/ide.h

#define WAIT_DRQ        (5*HZ/100)      /* 50msec - spec allows up to 20ms */

to

#define WAIT_DRQ	(5*HZ/50)

And check if that makes the problem go away?

On Tue, 30 Dec 2003, Linus Torvalds wrote:

> I suspect that anything in IDE that uses that idiotic "local_irq_set()"
> macro is just broken, and should be rewritten to explicitly enable
> interrupts. As it is, the code is just incredibly _strange_, and it
> actually enables interrupts at all the wrong places.
>
> Anyway, in ide_wait_stat(), the "timeout" value is always either
> "WAIT_DRQ" (5*HZ/100) or it is "WAIT_READY" (3*HZ/100). And look at
> WAIT_READY a bit more:
>
> 	#if defined(CONFIG_APM) || defined(CONFIG_APM_MODULE)
> 	#define WAIT_READY      (5*HZ)          /* 5sec - some laptops are very slow */
> 	#else
> 	#define WAIT_READY      (3*HZ/100)      /* 30msec - should be instantaneous */
> 	#endif /* CONFIG_APM || CONFIG_APM_MODULE */
>
> I bet that the _real_ problem is this. That "3*HZ/100" value is just too
> damn short. It has already been increased to 5*HZ for anything that has
> APM enabled, but anybody who doesn't use APM gets a _really_ short
> timeout.
>
> My suggestion: change the non-APM timeout to something much larger. Make
> it ten times bigger, rather than leaving it at a value that us so small
> that a single interrupt could make a difference..
>
> In fact, right now a single timer interrupt on 2.4.x is the difference
> between waiting 20ms and 30ms. That's a _big_ relative difference.

Small correction: people are not hitting the WAIT_READY (they are hitting
the problem from ide-disk.c, which uses WAIT_DRQ). But still...

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Linus Torvalds]

On Tue, 30 Dec 2003, Marcelo Tosatti wrote:
> 
> Small correction: people are not hitting the WAIT_READY (they are hitting
> the problem from ide-disk.c, which uses WAIT_DRQ). But still...

Ok. Do you have the full trace? In particular, if there is no locking in 
that path, and interrupts are enabled, you could possibly get not just an 
interrupt, but a preemption event. Now _that_ could blow up the timeout to 
any amount of time, and then even 100ms might not be enough.

Is CONFIG_PREEMPT on in the cases, and is there really no locking 
anywhere? Preempting in the middle of the data transfer phase sounds like 
a fundamentally bad idea, and maybe the code needs a few preempt 
disable/enable pairs somewhere?

			Linus

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Marcelo Tosatti]

On Tue, 30 Dec 2003, Linus Torvalds wrote:

>
>
> On Tue, 30 Dec 2003, Marcelo Tosatti wrote:
> >
> > Small correction: people are not hitting the WAIT_READY (they are hitting
> > the problem from ide-disk.c, which uses WAIT_DRQ). But still...
>
> Ok. Do you have the full trace? In particular, if there is no locking in
> that path, and interrupts are enabled, you could possibly get not just an
> interrupt, but a preemption event. Now _that_ could blow up the timeout to
> any amount of time, and then even 100ms might not be enough.

The problem is happening in 2.4 too so I believe preemption is not the
culprit. Here are some details:

steve@drifthost.com wrote:

"Well i only just started getting them and i started with 2.4.20 and
upgraded to 2.4.21 about 6weeks or so ago (or when it came out)"

"hda: status timeout: status=0xd0 { Busy }
hda: no DRQ after issuing WRITE
ide0: reset: success
hda: status timeout: status=0xd0 { Busy }
hda: no DRQ after issuing WRITE
ide0: reset: success"

daniel@starbattle.com wrote:

"hda: no DRQ after issuing WRITE
ide0: reset: success
hda: status timeout: status=0xd0 { Busy }

hda: no DRQ after issuing WRITE
ide0: reset: success"

(Daniel wrote the patch which got applied to 2.4, it fixed the problems
for him).

There are several other reports of "no DRQ after issuing {MULTI}WRITE",
some of them probably involved with this bug, some of them potentially
not. You can find more reports (both from 2.6 and 2.4) at:

http://marc.theaimsgroup.com/?l=linux-kernel&w=2&r=1&s=no+DRQ+after+issuing+WRITE&q=b

> Is CONFIG_PREEMPT on in the cases, and is there really no locking
> anywhere? Preempting in the middle of the data transfer phase sounds like
> a fundamentally bad idea, and maybe the code needs a few preempt
> disable/enable pairs somewhere?

>From my fast code read, there is no other locking involved.

It sounds you are right, the timeout is too small --- we need confirmation
from the people who can hit it that increasing it fixes the problem.

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Eric D. Mudama]

On Tue, Dec 30 at 20:21, Marcelo Tosatti wrote:
>"hda: no DRQ after issuing WRITE
>ide0: reset: success
>hda: status timeout: status=0xd0 { Busy }
>
>hda: no DRQ after issuing WRITE
>ide0: reset: success"
>
>(Daniel wrote the patch which got applied to 2.4, it fixed the problems
>for him).
>
>There are several other reports of "no DRQ after issuing {MULTI}WRITE",
>some of them probably involved with this bug, some of them potentially
>not. You can find more reports (both from 2.6 and 2.4) at:

Old ATA specifications had the concept of an auto-write segment,
in that the drive had to begin accepting data an extremly short time
after the command had been issued.

>From what I understand, there was a time when people attempted to
remove this from the spec, however, it was discovered that some old
BIOSs didn't bother to check the DRDY bit at all after issuing a PIO
write, and they immediately just went straight to data
transfer... without the drive ready to receive the data, it would
corrupt the block since the first N words of data wouldn't be seen.

In more modern versions of the ATA specification, the only time
constraint built into PIO protocol transfers is that the hardware has
at most 400ns to assert BSY following the write of the command
register.  Since every drive today has hardware to automate this
process, that time constraint is never violated.

Other than that, the drive is free to leave BSY asserted as long as it
needs to prior to setting DRQ and being ready for data-transfer in.

I could just be talking totally tangential to the issue being
discussed, but is the 20/30 or 29/30ms wait being discussed at this
point in the protocol, or is it elsewhere?

Unless it has been something like 5-8 *seconds* without DRQ=1 and
BSY=0, I don't think the driver should reset the device.

--eric


-- 
Eric D. Mudama
edmudama@mail.bounceswoosh.org

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Rob Love]

On Tue, 2003-12-30 at 16:57, Linus Torvalds wrote:

> Is CONFIG_PREEMPT on in the cases, and is there really no locking 
> anywhere? Preempting in the middle of the data transfer phase sounds like 
> a fundamentally bad idea, and maybe the code needs a few preempt 
> disable/enable pairs somewhere?

Is the kernel patched with kernel preemption?  It is not in stock 2.4.

Anyhow, if interrupts are disabled, preemption should be disabled (we
check for that condition in both preempt_schedule() and
return_from_intr).

If interrupts are not disabled, then preempting would definitely be a
bad thing.  But I would think, for the same reasons you do not want to
preempt, you would want interrupts disabled ..

	Rob Love

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Linus Torvalds]

On Tue, 30 Dec 2003, Rob Love wrote:
> 
> Anyhow, if interrupts are disabled, preemption should be disabled (we
> check for that condition in both preempt_schedule() and
> return_from_intr).

Interrupts are _not_ disabled here, very much on purpose. If they were, 
then "jiffies" wouldn't update, and the timeouts wouldn't work.

This is what that _stupid_ "local_irq_set()" function does: it saves the 
old irq masking state, and then it enables it.

The whole concept doesn't make any sense. If you enable interrupts, there 
is little point in saving the callers irq mask, since it already got 
deflated.

		Linus

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Rob Love]

On Tue, 2003-12-30 at 17:54, Linus Torvalds wrote:

> Interrupts are _not_ disabled here, very much on purpose. If they were, 
> then "jiffies" wouldn't update, and the timeouts wouldn't work.
> 
> This is what that _stupid_ "local_irq_set()" function does: it saves the 
> old irq masking state, and then it enables it.
> 
> The whole concept doesn't make any sense. If you enable interrupts, there 
> is little point in saving the callers irq mask, since it already got 
> deflated.

Ah, OK.  local_irq_set() is worthless, then.

Curious to see the results of upping the timeout.

	Rob Love

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Daniel Tram Lux]

Rob Love wrote:

>On Tue, 2003-12-30 at 17:54, Linus Torvalds wrote:
>
>  
>
>>Interrupts are _not_ disabled here, very much on purpose. If they were, 
>>then "jiffies" wouldn't update, and the timeouts wouldn't work.
>>
>>This is what that _stupid_ "local_irq_set()" function does: it saves the 
>>old irq masking state, and then it enables it.
>>
>>The whole concept doesn't make any sense. If you enable interrupts, there 
>>is little point in saving the callers irq mask, since it already got 
>>deflated.
>>    
>>
>
>Ah, OK.  local_irq_set() is worthless, then.
>
>Curious to see the results of upping the timeout.
>
>	Rob Love
>
>  
>
I tried setting the timeout up as a first fix, it also decreased the 
frequency of the error,
however it did not get rid of the error.
I used:

#define WAIT_DRQ        (10*HZ/100)      /* 100msec - spec allows up to 20ms */

in stead of:

#define WAIT_DRQ        (5*HZ/100)      /* 50msec - spec allows up to 20ms */


The device the error occurs with is a cf card. The error also occurs 
much more frequently in
2.4.23 than in 2.4.20 (but it can be provoked in 2.4.20). Neither use 
the preemption patch
and both are from kernel.org. The platform is based on an AMD Elan 
processor which is
a 486 compatible processor, running at 133 Mhz. The IDE subsytem does 
not use any extra
drivers and is not a PCI ide chipset.

The test I use to provoke the error is moving a directory tree from hdc 
(a normal harddisk)
to hda (the cf card), removing the dir on hdc, copy it back from hda to 
hdc, and remove it
from hda, then start all over.....
While doing this there is a flood ping running and the machine is being 
flood pinged + there
is traffic on three serial ports (RS485).

The way the code works right now there is no way you can tell how much 
time has passed
since the status register last got read out due to a possible interrupt. 
So when I made the patch
I saw two possibilities, either disabeling the interrupts while first 
reading the status and then
checking the timeout, after which the interrupts would be enabled again.
Or to just make one extra check after the timout has expired because 
that is cheaper
than returning, failing and then resetting the drive. After I applied my 
patch (using the
5*HZ/100 timeout) my test ran for a full weekend without giving the 
timeout error.
Before the error would occur about every 3 minutes with 2.4.23 and every 
couple of
hours on 2.4.20. (I didn't try to patch 2.4.20).

The ide standard gives a timeout for the busy wait of 20 ms which should 
not be exceeded
and the documentation from sandisk (the cf card is from sandisk) claims 
to conform to this.

If anybody has any other suggestions/tests I can try these out on monday 
when I am back
at work.

Regards

       Daniel Tram Lux

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Linus Torvalds]

On Sat, 3 Jan 2004, Daniel Tram Lux wrote:
>
> I tried setting the timeout up as a first fix, it also decreased the 
> frequency of the error,
> however it did not get rid of the error.

That is scary beyond belief.

Basically you doubled your timeout, and you certainly _should_ have seen 
at least one more status read from the extra 50 msec you waited. And since 
your patch (which fixes it) only adds _one_ status read, that implies that 
the extra 50 msec timeout didn't get a single time through the loop.

> The device the error occurs with is a cf card. The error also occurs
> much more frequently in 2.4.23 than in 2.4.20 (but it can be provoked in
> 2.4.20). Neither use the preemption patch and both are from kernel.org.
> The platform is based on an AMD Elan processor which is a 486 compatible
> processor, running at 133 Mhz. The IDE subsytem does not use any extra
> drivers and is not a PCI ide chipset.

Yes, that path is only used for PIO writes, so it's clearly not a 
high-performance IDE setup. Adn yes, I guess with a slow enough CPU and 
enough interrupt load, you literally could spend 50ms just handling irqs. 
Still, that is pretty damn scary. But I guess the load:

>  ... there is a flood ping running and the machine is being 
> flood pinged + there is traffic on three serial ports (RS485).

is pretty extreme.

> I saw two possibilities, either disabeling the interrupts while first
> reading the status and then checking the timeout, after which the
> interrupts would be enabled again. Or to just make one extra check after
> the timout has expired because that is cheaper than returning, failing
> and then resetting the drive. After I applied my patch (using the
> 5*HZ/100 timeout) my test ran for a full weekend without giving the
> timeout error.

Ok, I think I'm convinced. That loop and the IDE usage of interrupt
enables is just crap. I don't think your addition is very pretty, but the 
alternative is to rewrite the loop to be sane, which isn't going to happen 
in a stable kernel.

How about a slightly more minimal patch, though? Ie does this work for 
you?

		Linus

----
===== drivers/ide/ide-iops.c 1.18 vs edited =====
--- 1.18/drivers/ide/ide-iops.c	Wed Jun 11 18:23:09 2003
+++ edited/drivers/ide/ide-iops.c	Sat Jan  3 10:54:21 2004
@@ -647,6 +647,15 @@
 		timeout += jiffies;
 		while ((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) {
 			if (time_after(jiffies, timeout)) {
+				/*
+				 * One last read after the timeout in case
+				 * heavy interrupt load made us not make any
+				 * progress during the timeout..
+				 */
+				stat = hwif->INB(IDE_STATUS_REG);
+				if (!(stat & BUSY_STAT))
+					break;
+
 				local_irq_restore(flags);
 				*startstop = DRIVER(drive)->error(drive, "status timeout", stat);
 				return 1;

Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released

[Daniel Tram Lux]

Linus Torvalds wrote:

>On Sat, 3 Jan 2004, Daniel Tram Lux wrote:
>  
>
>>I tried setting the timeout up as a first fix, it also decreased the 
>>frequency of the error,
>>however it did not get rid of the error.
>>    
>>
>
>That is scary beyond belief.
>
>Basically you doubled your timeout, and you certainly _should_ have seen 
>at least one more status read from the extra 50 msec you waited. And since 
>your patch (which fixes it) only adds _one_ status read, that implies that 
>the extra 50 msec timeout didn't get a single time through the loop.
>
>  
>
>>The device the error occurs with is a cf card. The error also occurs
>>much more frequently in 2.4.23 than in 2.4.20 (but it can be provoked in
>>2.4.20). Neither use the preemption patch and both are from kernel.org.
>>The platform is based on an AMD Elan processor which is a 486 compatible
>>processor, running at 133 Mhz. The IDE subsytem does not use any extra
>>drivers and is not a PCI ide chipset.
>>    
>>
>
>Yes, that path is only used for PIO writes, so it's clearly not a 
>high-performance IDE setup. Adn yes, I guess with a slow enough CPU and 
>enough interrupt load, you literally could spend 50ms just handling irqs. 
>Still, that is pretty damn scary. But I guess the load:
>
>  
>
>> ... there is a flood ping running and the machine is being 
>>flood pinged + there is traffic on three serial ports (RS485).
>>    
>>
>
>is pretty extreme.
>
These systems are going to Texas and I am in Denmark, so I wanted to be 
sure that the
system also works under heavy load. This is not the typical use however ;-).

>
>  
>
>>I saw two possibilities, either disabeling the interrupts while first
>>reading the status and then checking the timeout, after which the
>>interrupts would be enabled again. Or to just make one extra check after
>>the timout has expired because that is cheaper than returning, failing
>>and then resetting the drive. After I applied my patch (using the
>>5*HZ/100 timeout) my test ran for a full weekend without giving the
>>timeout error.
>>    
>>
>
>Ok, I think I'm convinced. That loop and the IDE usage of interrupt
>enables is just crap. I don't think your addition is very pretty, but the 
>alternative is to rewrite the loop to be sane, which isn't going to happen 
>in a stable kernel.
>
>How about a slightly more minimal patch, though? Ie does this work for 
>you?
>
>		Linus
>
>----
>===== drivers/ide/ide-iops.c 1.18 vs edited =====
>--- 1.18/drivers/ide/ide-iops.c	Wed Jun 11 18:23:09 2003
>+++ edited/drivers/ide/ide-iops.c	Sat Jan  3 10:54:21 2004
>@@ -647,6 +647,15 @@
> 		timeout += jiffies;
> 		while ((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) {
> 			if (time_after(jiffies, timeout)) {
>+				/*
>+				 * One last read after the timeout in case
>+				 * heavy interrupt load made us not make any
>+				 * progress during the timeout..
>+				 */
>+				stat = hwif->INB(IDE_STATUS_REG);
>+				if (!(stat & BUSY_STAT))
>+					break;
>+
> 				local_irq_restore(flags);
> 				*startstop = DRIVER(drive)->error(drive, "status timeout", stat);
> 				return 1;
>  
>
The patch seems functionally to be equivalent, I can test it first thing 
on monday. I agree that the loop
should be rewritten too, it is neither clear for reading nor very 
accurate when it comes to measuring
the timeout.
I also was planning to test this patch with a reduced timeout of 
3*HZ/100, do you think that is worth
testing?

             Daniel
P.S.
I will try to make my patches more minimal in future ;-).

Re: 2.4.23-uv3 patch set released

[Pavel Machek]

Hi!

> Anyway, in ide_wait_stat(), the "timeout" value is always either 
> "WAIT_DRQ" (5*HZ/100) or it is "WAIT_READY" (3*HZ/100). And look at 
> WAIT_READY a bit more: 
> 
> 	#if defined(CONFIG_APM) || defined(CONFIG_APM_MODULE)
> 	#define WAIT_READY      (5*HZ)          /* 5sec - some laptops are very slow */
> 	#else
> 	#define WAIT_READY      (3*HZ/100)      /* 30msec - should be instantaneous */
> 	#endif /* CONFIG_APM || CONFIG_APM_MODULE */
> 
> I bet that the _real_ problem is this. That "3*HZ/100" value is just too 
> damn short. It has already been increased to 5*HZ for anything that has 
> APM enabled, but anybody who doesn't use APM gets a _really_ short 
> timeout.
> 
> My suggestion: change the non-APM timeout to something much larger. Make
> it ten times bigger, rather than leaving it at a value that us so small
> that a single interrupt could make a difference..
> 
> In fact, right now a single timer interrupt on 2.4.x is the difference
> between waiting 20ms and 30ms. That's a _big_ relative difference.
> 
> Andrew - unless you disagree, I'd just be inlined to change both the DRQ
> and READY timeouts to be a bit larger. On working hardware it shouldn't
> matter, so how about just making them both be something like 100 msec (and
> leave that strange really big APM value alone).

I believe you should get rid of that CONFIG_APM. Its wrong. CONFIG_APM
no longer corresponds with "is laptop". You can have laptop with ACPI
etc.

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: 2.4.23-uv3 patch set released

[James Bourne]

On Tue, 30 Dec 2003, Marcelo Tosatti wrote:

> 
> 
> On Mon, 29 Dec 2003, James Bourne wrote:
> 
> > linux-2.4.23-ide-busy-race-fix.patch: Daniel Lux: Fix IDE busy-wait race
> 
> I screwed up the merge of this patch, you also want to apply "Fix IDE
> busywait merge" (its the next changeset after this one).

Thanks for the info.  I did catch that on the bk list and appended it before
releasing that patch, so it is there (just confirmed).

Regards
James

-- 
James Bourne                  | Email:            jbourne@hardrock.org          
Unix Systems Administrator    | WWW:           http://www.hardrock.org
Custom Unix Programming       | Linux:  The choice of a GNU generation
----------------------------------------------------------------------
 "All you need's an occasional kick in the philosophy." Frank Herbert