* 2.4.23-uv3 patch set released
@ 2003-12-29 7:58 James Bourne
2003-12-30 11:36 ` Marcelo Tosatti
0 siblings, 1 reply; 15+ messages in thread
From: James Bourne @ 2003-12-29 7:58 UTC (permalink / raw)
To: Linux Kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.4.23-uv3 patch set released
2003-12-29 7:58 2.4.23-uv3 patch set released James Bourne
@ 2003-12-30 11:36 ` Marcelo Tosatti
2003-12-30 19:59 ` Linus Torvalds
2003-12-31 6:10 ` James Bourne
0 siblings, 2 replies; 15+ messages in thread
From: Marcelo Tosatti @ 2003-12-30 11:36 UTC (permalink / raw)
To: James Bourne; +Cc: Linux Kernel
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).
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.4.23-uv3 patch set released
2003-12-30 11:36 ` Marcelo Tosatti
@ 2003-12-30 19:59 ` Linus Torvalds
2003-12-30 21:46 ` no DRQ after issuing WRITE was " Marcelo Tosatti
2004-01-03 22:10 ` Pavel Machek
2003-12-31 6:10 ` James Bourne
1 sibling, 2 replies; 15+ messages in thread
From: Linus Torvalds @ 2003-12-30 19:59 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: James Bourne, Linux Kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 19:59 ` Linus Torvalds
@ 2003-12-30 21:46 ` Marcelo Tosatti
2003-12-30 21:57 ` Linus Torvalds
2004-01-03 22:10 ` Pavel Machek
1 sibling, 1 reply; 15+ messages in thread
From: Marcelo Tosatti @ 2003-12-30 21:46 UTC (permalink / raw)
To: Daniel Tram Lux, steve
Cc: Linus Torvalds, James Bourne, Linux Kernel, Gergely Tamas,
Bartlomiej Zolnierkiewicz
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...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 21:46 ` no DRQ after issuing WRITE was " Marcelo Tosatti
@ 2003-12-30 21:57 ` Linus Torvalds
2003-12-30 22:21 ` Marcelo Tosatti
2003-12-30 22:23 ` Rob Love
0 siblings, 2 replies; 15+ messages in thread
From: Linus Torvalds @ 2003-12-30 21:57 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: Daniel Tram Lux, steve, James Bourne, Linux Kernel,
Gergely Tamas, Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 21:57 ` Linus Torvalds
@ 2003-12-30 22:21 ` Marcelo Tosatti
2003-12-30 23:18 ` Eric D. Mudama
2003-12-30 22:23 ` Rob Love
1 sibling, 1 reply; 15+ messages in thread
From: Marcelo Tosatti @ 2003-12-30 22:21 UTC (permalink / raw)
To: Linus Torvalds
Cc: Marcelo Tosatti, Daniel Tram Lux, steve, James Bourne,
Linux Kernel, Gergely Tamas, Bartlomiej Zolnierkiewicz
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 21:57 ` Linus Torvalds
2003-12-30 22:21 ` Marcelo Tosatti
@ 2003-12-30 22:23 ` Rob Love
2003-12-30 22:54 ` Linus Torvalds
1 sibling, 1 reply; 15+ messages in thread
From: Rob Love @ 2003-12-30 22:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: Marcelo Tosatti, Daniel Tram Lux, steve, James Bourne,
Linux Kernel, Gergely Tamas, Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 22:23 ` Rob Love
@ 2003-12-30 22:54 ` Linus Torvalds
2003-12-30 22:58 ` Rob Love
0 siblings, 1 reply; 15+ messages in thread
From: Linus Torvalds @ 2003-12-30 22:54 UTC (permalink / raw)
To: Rob Love
Cc: Marcelo Tosatti, Daniel Tram Lux, steve, James Bourne,
Linux Kernel, Gergely Tamas, Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 22:54 ` Linus Torvalds
@ 2003-12-30 22:58 ` Rob Love
2004-01-03 11:22 ` Daniel Tram Lux
0 siblings, 1 reply; 15+ messages in thread
From: Rob Love @ 2003-12-30 22:58 UTC (permalink / raw)
To: Linus Torvalds
Cc: Marcelo Tosatti, Daniel Tram Lux, steve, James Bourne,
Linux Kernel, Gergely Tamas, Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 22:21 ` Marcelo Tosatti
@ 2003-12-30 23:18 ` Eric D. Mudama
0 siblings, 0 replies; 15+ messages in thread
From: Eric D. Mudama @ 2003-12-30 23:18 UTC (permalink / raw)
To: Linux Kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.4.23-uv3 patch set released
2003-12-30 11:36 ` Marcelo Tosatti
2003-12-30 19:59 ` Linus Torvalds
@ 2003-12-31 6:10 ` James Bourne
1 sibling, 0 replies; 15+ messages in thread
From: James Bourne @ 2003-12-31 6:10 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Linux Kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2003-12-30 22:58 ` Rob Love
@ 2004-01-03 11:22 ` Daniel Tram Lux
2004-01-03 18:57 ` Linus Torvalds
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Tram Lux @ 2004-01-03 11:22 UTC (permalink / raw)
To: Rob Love
Cc: Linus Torvalds, Marcelo Tosatti, steve, James Bourne,
Linux Kernel, Gergely Tamas, Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2004-01-03 11:22 ` Daniel Tram Lux
@ 2004-01-03 18:57 ` Linus Torvalds
2004-01-03 19:27 ` Daniel Tram Lux
0 siblings, 1 reply; 15+ messages in thread
From: Linus Torvalds @ 2004-01-03 18:57 UTC (permalink / raw)
To: Daniel Tram Lux
Cc: Rob Love, Marcelo Tosatti, steve, James Bourne, Linux Kernel,
Gergely Tamas, Bartlomiej Zolnierkiewicz
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;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: no DRQ after issuing WRITE was Re: 2.4.23-uv3 patch set released
2004-01-03 18:57 ` Linus Torvalds
@ 2004-01-03 19:27 ` Daniel Tram Lux
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Tram Lux @ 2004-01-03 19:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rob Love, Marcelo Tosatti, steve, James Bourne, Linux Kernel,
Gergely Tamas, Bartlomiej Zolnierkiewicz
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 ;-).
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.4.23-uv3 patch set released
2003-12-30 19:59 ` Linus Torvalds
2003-12-30 21:46 ` no DRQ after issuing WRITE was " Marcelo Tosatti
@ 2004-01-03 22:10 ` Pavel Machek
1 sibling, 0 replies; 15+ messages in thread
From: Pavel Machek @ 2004-01-03 22:10 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Marcelo Tosatti, James Bourne, Linux Kernel
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?]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2004-01-03 22:08 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-12-29 7:58 2.4.23-uv3 patch set released James Bourne
2003-12-30 11:36 ` Marcelo Tosatti
2003-12-30 19:59 ` Linus Torvalds
2003-12-30 21:46 ` no DRQ after issuing WRITE was " Marcelo Tosatti
2003-12-30 21:57 ` Linus Torvalds
2003-12-30 22:21 ` Marcelo Tosatti
2003-12-30 23:18 ` Eric D. Mudama
2003-12-30 22:23 ` Rob Love
2003-12-30 22:54 ` Linus Torvalds
2003-12-30 22:58 ` Rob Love
2004-01-03 11:22 ` Daniel Tram Lux
2004-01-03 18:57 ` Linus Torvalds
2004-01-03 19:27 ` Daniel Tram Lux
2004-01-03 22:10 ` Pavel Machek
2003-12-31 6:10 ` James Bourne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).