It's a few days late, but I was waiting for some updates for some of the

most annoying regressions until releasing it, so the end result is

hopefully more useful as a result.
In particular, the block layer changes should hopefully have sorted

themselves out, and CD burning etc hopefully works for people again. Same

goes for the the scheduler regressions, and a number of annoying boot-time

problems.
In short, if you had any issues, please do test, and make sure that the

regression list gets updated (whether fixed or not).
The dirstat shows that (as usual) most of the changes are in drivers and

arch (~51% and ~17% respectively), with about half the driver updates

being in network drivers. Full details for those who care:
	   2.0% Documentation/

	   3.6% arch/blackfin/

	   3.6% arch/cris/

	   2.3% arch/sparc/kernel/

	   2.8% arch/x86/

	  17.3% arch/

	   2.1% drivers/char/

	   7.0% drivers/dma/

	   3.0% drivers/firewire/

	  24.0% drivers/net/

	  50.6% drivers/

	   5.6% fs/cifs/

	   9.4% fs/

	   3.6% include/

	   5.2% kernel/

	   3.9% mm/

	   2.5% net/
but in general, it's really a fair amount of small changes spread all

over, with most of the changes being quite small (604 commits, most of

them small, with the BNX2X network driver being and the new fsldma driver

the only one that got some bigger changes).
So give it all a good testing,
		Linus

--

randconfig testing found a bootup lockup in drivers/char/esp.c - find

the fix below. Not sure why it became more prominent in 2.6.25-rc4, the

bug seems rather old and i've been doing allyesconfig bootups for ages

with CONFIG_ESP enabled.
------------->

Subject: drivers/char/esp.c: fix bootup lockup

From: Ingo Molnar <mingo@elte.hu>

Date: Fri Mar 07 10:47:43 CET 2008
fix this bootup lockup:
 PM: Adding info for No Bus:ttyP63

 ttyP32 at 0x0240 (irq = 0) is an ESP primary port

 BUG: spinlock lockup on CPU#0, swapper/1, f56dd004

 Pid: 1, comm: swapper Not tainted 2.6.25-rc4-sched-devel.git-x86-latest.git #402 [<c03ac6f4>] _raw_spin_lock+0x134/0x140

  [<c08649be>] _spin_lock_irqsave+0x5e/0x80

  [<c0b9fbfe>] ? espserial_init+0x2be/0x6e0

  [<c0b9fbfe>] espserial_init+0x2be/0x6e0

  [<c0b877a3>] kernel_init+0x83/0x260

  [<c0b9f940>] ? espserial_init+0x0/0x6e0

  [<c010416a>] ? restore_nocheck_notrace+0x0/0xe

  [<c0b87720>] ? kernel_init+0x0/0x260

  [<c0b87720>] ? kernel_init+0x0/0x260

  [<c0104507>] kernel_thread_helper+0x7/0x10

  =======================
kzalloc() is not the way to initialize spinlocks anymore.
Signed-off-by: Ingo Molnar <mingo@elte.hu>

---

 drivers/char/esp.c |    1 +

 1 file changed, 1 insertion(+)
Index: linux/drivers/char/esp.c

===================================================================

--- linux.orig/drivers/char/esp.c

+++ linux/drivers/char/esp.c

@@ -2484,6 +2484,7 @@ static int __init espserial_init(void)

 			return 0;

 		}
+		spin_lock_init(&info->lock);

 		/* rx_trigger, tx_trigger are needed by autoconfig */

 		info->config.rx_trigger = rx_trigger;

 		info->config.tx_trigger = tx_trigger;

--

Good catch, thanks.

--

Is this a regression from 2.6.24?
Rafael

--

I don't think so, the init code hasn't been touched for years...
regards,

--js

--

hm, tonight's randconfig bootrun produced a failing (soft-hung) kernel

after about 120 iterations - and the log i captured _seems_ to indicate

some block IO (or libata) completion weirdness.
unfortunately, it's not readily reproducible, and i triggered it with

about 100 sched.git and 300 x86.git patches applied. BUT, virtually the

same 100+300 patches queue produced a successful 1000+ randconfig

testrun over the last weekend so i'm reasonably sure the regression is

new and came in via upstream. Also, the config is UP (and it's a rather

simple config in other aspects as well), so this must be something

rather fundamental, not an SMP race.
I just spent about an hour trying to figure out a pattern but the bug

just doesnt reproduce after 20 bootup attempts with the same bzImage.

When it hung then it hung for hours, so the condition is permanent.
I've attached the bootup log which includes the SysRq-T output and the

config. The hang seems to occur because an rc.sysinit task is not coming

back from io_schedule():
rc.sysinit    D f75bcc24     0  1922   1893

       f761c810 00000086 f75bcd38 f75bcc24 1954bff5 00000015 f7746000 f761c974

       f761c974 f7c17698 c180e7a8 f7747cc4 00000000 f7747ccc c180e7a8 c097bff7

       c01a3acb c097c27d c01a3aa0 f7872a90 00000002 c01a3aa0 f7747e48 c097c2fc

Call Trace:

 [<c097bff7>] io_schedule+0x37/0x70

 [<c01a3acb>] sync_buffer+0x2b/0x30

 [<c097c27d>] __wait_on_bit+0x4d/0x80

 [<c01a3aa0>] sync_buffer+0x0/0x30

 [<c01a3aa0>] sync_buffer+0x0/0x30

 [<c097c2fc>] out_of_line_wait_on_bit+0x4c/0x60

 [<c0142340>] wake_bit_function+0x0/0x40

 [<c01a3a51>] __wait_on_buffer+0x21/0x30

 [<c0209915>] ext3_bread+0x55/0x70

 [<c020cff8>] ext3_find_entry+0x258/0x660

 [<c03a0026>] avc_has_perm+0x46/0x50

 [<c03a0d14>] inode_has_perm+0x44/0x80

 [<c020de69>] ext3_lookup+0x29/0xa0

 [<c0189f90>] do_lookup+0x130/0x180

 [<c018b540>] __link_path_walk+0x340/0xd50

 ...
[ message continues ]

Sorry, I have _no_ ideas on what this could be. We haven't really had

any related changes in the block layer over that short a time frame. It

could of course have been introduced earlier, since it seems to be quite

elusive.
Presumably any hw issues would get noticed (like missing interrupt) and

trigger the error handler, so it looks like this IO is still stuck in

the queue somewhere. That mainly points the finger at AS, but given that

you cannot reproduce I'm not sure how best to proceed with this...
--

Jens Axboe
--

The problem looks very similar to the one recently reported by Anders:
http://lkml.org/lkml/2008/2/22/239
"Trying out 2.6.25-rc2 smartd always causes my box to hang. I can switch

vt:s and the keyboard seems to work.
Using sysrq-e I noticed a callpath open -> ext3 -> journals -> sync_buffer ->

io_scheduel -> generic_unplig_device.

I'd guess the open stems from smartd. Removing smartd from the startup, I'm

now using rc2 fine..."
Initially we thought that it is an IDE regression but after some testing

and further investigation it seems that IDE changes just made it more

likely to occur (yesterday it turned out that the kernel with the "guilty"

IDE commit sometimes boots fine).  The issue is easily reproducible.
PS We've already verified that it is not PREEMPT related or a compiler bug. 
Thanks,

Bart

--

smartd is not active here though - and starting/restarting it doesnt

provoke a hang.
	Ingo

--

I was a bit unclear on this, the problem is easily reproducible but _only_

on Anders' system (I couldn't reproduce it with smartd here either).
Thanks,

Bart

--

What IO scheduler is Anders using (and does it have an impact, eg does

it hang with deadline as well)?
--

Jens Axboe
--

CONFIG_IOSCHED_NOOP=y

CONFIG_IOSCHED_AS=y

CONFIG_IOSCHED_DEADLINE=y

CONFIG_IOSCHED_CFQ=y

CONFIG_DEFAULT_IOSCHED="cfq"
On request here's a sysrq-t from smartd on -rc3, you also see exactly where

smartd gives up (using the gentoo default smasrtd.conf).
Might a strace of smartd help?
Lest you folks come up with any ideas, I'll try another bisect run on saturday

between the was_guilty_but_now_sometimes_work and 25-r1, and see what that

gives.
Mar  6 22:04:27 tracker smartd[6283]: smartd version 5.37 [i686-pc-linux-gnu] Copyright (C) 2002-6 Bruce Allen

Mar  6 22:04:27 tracker smartd[6283]: Home page is http://smartmontools.sourceforge.net/

Mar  6 22:04:27 tracker smartd[6283]: Opened configuration file /etc/smartd.conf

Mar  6 22:04:27 tracker smartd[6283]: Configuration file /etc/smartd.conf parsed.

Mar  6 22:04:27 tracker smartd[6283]: Device: /dev/hda, opened

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, found in smartd database.

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, can't monitor Current Pending Sector count - no Attribute 197

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, can't monitor Offline Uncorrectable Sector count  - no Attribute 198

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, can't monitor Temperature, ignoring -W Directive

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, appears to lack SMART Self-Test log; disabling -l selftest (override with -T permissive Directive)

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, appears to lack SMART Error log; disabling -l error (override with -T permissive Directive)

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, is SMART capable. Adding to "monitor" list.

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, opened

Mar  6 22:04:28 tracker smartd[6283]: Device: /dev/hda, found in smartd database.

Mar  6 22:04:32 tracker ======

Mar  6 22:04:32 tracker dnsmasq       S 00000000     0  4181      1

Mar  6 22:04:32 tracker d25bbb10 00000056 00000000 0000000...
[ message continues ]

Can you try with noop or deadline as the default, to see if it
Nope, looking at the trace it'll be stuck waiting for the ioctl to

finish.
--

Jens Axboe
--

Fails in the same way using noop, deadline and cfq on -rc4
/A
--

I think we do want the bisect run here.
My worry is that this is likely very timing-sensitive, so when it starts

failing it might not be because of the commit that actually introduces the

bug, but because some other timing changed, but with some luck that won't

be the case.
			Linus

--

I'm on it. Slow machine. Household's router, 4000 versions to go...
/A
--

The bisect came up with this:
18a056feccabdfa9764016a615121b194828bc72 is first bad commit

commit 18a056feccabdfa9764016a615121b194828bc72

Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Date:   Sat Jan 26 20:13:12 2008 +0100
    ide: don't enable local IRQs for PIO-in in driver_cmd_intr() (take 2)
    Don't enable local IRQs for PIO-in protocol in driver_cmd_intr().
    While at it:
    * Remove redundant rq->cmd_type check.
    * Read status register after enabling local IRQs for no-data protocol.
    v2:

    * Re-add DRQ=1 check lost in v1 (noticed by Sergei).
    Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
--

Hmm, this is the first commit _after_ the previous "guilty"

commit 852738f39258deafb3d89c187cb1a4050820d555 so it just

--

Well, would that be practical to prepare a patch reverting this commit

and whatever depends on it so that Anders can verify it?
Rafael

--

We've already verified that 8527 just makes the problem more likely

to occur (the discussion is in the earlier "-rc3 regression" thread)

so reverting 8527 or 18a0 won't really help.
Thanks,

Bart

--

Many bisects later, now with taking care of making 'make oldconfig' off a

known good config for each iteration, and doing 10 reboots and 5 smartd

invocations for each version deemed good (not that anyone failed midway).
4d977e43d8ae758434e603cf2455d955f71c77c4 is first bad commit

commit 4d977e43d8ae758434e603cf2455d955f71c77c4

Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Date:   Sat Jan 26 20:13:12 2008 +0100
    ide: check BUSY and ERROR status bits before reading data in drive_cmd_intr()
    Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
--

Ok, this is interesting. It's clearly a regression, so we need to undo it.

However, it's not trivial to revert, since lots of things have changed

around that area since.
In particular, commit 7267c3377443322588cddaf457cf106839a60463 ("ide:

remove REQ_TYPE_ATA_CMD") ended up removing the whole drive_cmd_intr()

function, because now all the commands are handled with the

REQ_TYPE_ATA_TASKFILE model instead, which uses a whole another path.
And quite frankly, I think the commit you bisected to really is very

broken. It starts doing error handling *before* it has handled the DRQ

bit, and that's bogus, since iirc a lot of controllers need to have their

DRQ issues satisfied before anything else.
So what probably happens is that yes, you get an error, but the IDE drive

still wants the code to flush the data it generated, and if we don't do

that, it will never do anything else ever again. Resulting in a hang.
So Anders, can you try these two silly things:
 - there's a totally untested patch at the end here that tries to make the

   error handling do that DRQ flush unconditionally. Does it make a

   difference for you?
 - did you already try if ata_piix fixes it? 
but I do think that the commit you bisected is crap, crap, crap.
Bartlomiej?
		Linus

---

 drivers/ide/ide-io.c |    3 +--

 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c

index 7153796..9105c09 100644

--- a/drivers/ide/ide-io.c

+++ b/drivers/ide/ide-io.c

@@ -462,8 +462,7 @@ static ide_startstop_t ide_ata_error(ide_drive_t *drive, struct request *rq, u8

 		}

 	}
-	if ((stat & DRQ_STAT) && rq_data_dir(rq) == READ &&

-	    (hwif->host_flags & IDE_HFLAG_ERROR_STOPS_FIFO) == 0)

+	if ((stat & DRQ_STAT) && rq_data_dir(rq) == READ)

 		try_to_flush_leftover_data(drive);
 	if (rq->errors >= ERROR_MAX || blk_noretry_request(rq)) {

--

Ok, it's not going to make any difference, because that

IDE_HFLAG_ERROR_STOPS_FIFO flag won't be set for you anyway.
So assuming it's about a pending DRQ issue that needs to be flushed before

the controller will generate any more interrupts (which your bisect

implies it is), maybe we have one of
 - this ide_ata_error() case isn't called at all.
   This happens for at least WIN_SPECIFY (see ide_ata_error() - it

   returns early)
   Could you add a printk() to ide_ata_error() and see if it gets called

   when your machine hangs, and if so, what the case was.
 - try_to_flush_leftover_data() doesn't work (it does have a test for only

   acting on disk drives - is this perhaps a CD-ROM?)
 - or rq_data_dir(rq) is wrong for the offending command (again, maybe

   the printk() could help with that).
IOW, try something like this instead of the previous patch.
		Linus
---

 drivers/ide/ide-io.c |    9 ++++++---

 1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c

index 7153796..6a98937 100644

--- a/drivers/ide/ide-io.c

+++ b/drivers/ide/ide-io.c

@@ -414,6 +414,7 @@ static void try_to_flush_leftover_data (ide_drive_t *drive)

 {

 	int i = (drive->mult_count ? drive->mult_count : 1) * SECTOR_WORDS;
+printk("try_to_flush(%d,%d)\n", i, drive->media == ide_disk);

 	if (drive->media != ide_disk)

 		return;

 	while (i > 0) {

@@ -440,7 +441,10 @@ static ide_startstop_t ide_ata_error(ide_drive_t *drive, struct request *rq, u8

 {

 	ide_hwif_t *hwif = drive->hwif;
-	if (stat & BUSY_STAT || ((stat & WRERR_STAT) && !drive->nowerr)) {

+printk("ide_ata_error() stat=%02x err=%02x dir=%d CMD=%02x\n",

+	stat, err, rq_data_dir(rq), hwif->INB(IDE_COMMAND_REG));

+

+	if ((stat & BUSY_STAT) || ((stat & WRERR_STAT) && !drive->nowerr)) {

 		/* other bits are useless when BUSY */

 		rq->errors |= ERROR_RESET;

 	} else if (stat & ERR_STAT) {

@@ -462,8 +466...
[ message continues ]

Applied with fuzz 9 on all hunks.
Reboot. Run smartd. Nothing in dmesg. Hang.
--

We don't do error handling for special commands (REQ_TYPE_ATA_*) at all,

ide_error() just dumps device's status/error register(s) and finishes early:
ide_startstop_t ide_error (ide_drive_t *drive, const char *msg, u8 stat)

{

	struct request *rq;

	u8 err;
	err = ide_dump_status(drive, msg, stat);
	if ((rq = HWGROUP(drive)->rq) == NULL)

		return ide_stopped;
	/* retry only "normal" I/O: */

	if (!blk_fs_request(rq)) {

		rq->errors = 1;

		ide_end_drive_cmd(drive, stat, err);

		return ide_stopped;

	}
[ Yeah, I completely agree that it needs some re-design but I don't see how
This is unlikely as we should get some error message from ide_dump_status()

and we are not getting any (Anders, please correct me if I'm wrong).
Moreover the problem was initially narrowed down to commit

852738f39258deafb3d89c187cb1a4050820d555 which is two commits _before_

the one that we are currently discussing.
I think that we are looking into completely wrong direction...
Thanks,

Bart

--

Well that sounds bogus too, for all the same reasons already outlined. The 
.. and that initial narrowing down is now in doubt, with the new one

pointing to that DRQ-logic-changing commit!
IOW, Anders re-did things more carefully, and came to a new result. Why

are you apparenrly ignoring that fact?
			Linus

--

OK, lets try it.
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Subject: [PATCH] ide: correctly handle DRQ bit set on error also for special requests
commit 4d977e43d8ae758434e603cf2455d955f71c77c4 ("ide: check BUSY and ERROR

status bits before reading data in drive_cmd_intr()") changed DRQ handling

logic (as pointed out by Linus).
Fix it by flushing leftover data for commands using PIO-in protocol

and special requests (rq->cmd_type == REQ_TYPE_ATA_TASKFILE).
Cc: Anders Eriksson <aeriksson@fastmail.fm>

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---

against 2.6.25-rc5, untested
 drivers/ide/ide-io.c |   11 +++++++++--

 1 file changed, 9 insertions(+), 2 deletions(-)
Index: b/drivers/ide/ide-io.c

===================================================================

--- a/drivers/ide/ide-io.c

+++ b/drivers/ide/ide-io.c

@@ -540,12 +540,19 @@ EXPORT_SYMBOL_GPL(__ide_error);
 ide_startstop_t ide_error (ide_drive_t *drive, const char *msg, u8 stat)

 {

-	struct request *rq;

+	ide_hwif_t *hwif = drive->hwif;

+	struct request *rq = hwif->hwgroup->rq;

 	u8 err;
+	if (rq && rq->cmd_type == REQ_TYPE_ATA_TASKFILE) {

+		if (hwif->data_phase == TASKFILE_IN && (stat & DRQ_STAT) &&

+		    (hwif->host_flags & IDE_HFLAG_ERROR_STOPS_FIFO) == 0)

+			try_to_flush_leftover_data(drive);

+	}

+

 	err = ide_dump_status(drive, msg, stat);
-	if ((rq = HWGROUP(drive)->rq) == NULL)

+	if (rq == NULL)

 		return ide_stopped;
 	/* retry only "normal" I/O: */

--

rc5 fails with and without that patch.
/A
--

Can you try to sprinkle printk's around to see where it actually ends up

stopping?
Since all special commands should now go through the taskfile interface

(Bartlomiej - correct?), I guess just a printk() at the top of

do_rw_taskfile() should at least get the ball rolling. Even something that

just prints out task->data_phase and tf->command would start to unravel at

least what the last command is before it hangs..
		Linus

--

Tossing a prink in like so:

ide_startstop_t do_rw_taskfile (ide_drive_t *drive, ide_task_t *task)

{

        ide_hwif_t *hwif        = HWIF(drive);

        struct ide_taskfile *tf = &task->tf;

        ide_handler_t *handler = NULL;
        printk("do_rw_taskfile(task->data_phase=%i, tf->command=%i\n",task->data_phase,tf->command);
        if (task->data_phase == TASKFILE_MULTI_IN ||
Yields:

Mar 17 21:57:29 tippex smartd[5810]: smartd version 5.37 [i686-pc-linux-gnu] Copyright (C) 2002-6 Bruce Allen

Mar 17 21:57:29 tippex smartd[5810]: Home page is http://smartmontools.sourceforge.net/

Mar 17 21:57:29 tippex smartd[5810]: Opened configuration file /etc/smartd.conf

Mar 17 21:57:29 tippex do_rw_taskfile(task->data_phase=32, tf->command=37

Mar 17 21:57:29 tippex do_rw_taskfile(task->data_phase=1, tf->command=236

Mar 17 21:57:29 tippex smartd[5810]: Configuration file /etc/smartd.conf parsed.

Mar 17 21:57:29 tippex smartd[5810]: Device: /dev/hda, opened

Mar 17 21:57:29 tippex do_rw_taskfile(task->data_phase=0, tf->command=176

Mar 17 21:57:29 tippex smartd[5810]: Device: /dev/hda, found in smartd database.

Mar 17 21:57:29 tippex do_rw_taskfile(task->data_phase=0, tf->command=176

Mar 17 21:57:30 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 17 21:57:30 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 17 21:57:30 tippex do_rw_taskfile(task->data_phase=1, tf->command=236

Mar 17 21:57:30 tippex smartd[5810]: Device: /dev/hda, can't monitor Current Pending Sector count - no Attribute 197

Mar 17 21:57:30 tippex smartd[5810]: Device: /dev/hda, can't monitor Offline Uncorrectable Sector count  - no Attribute 198

Mar 17 21:57:30 tippex smartd[5810]: Device: /dev/hda, can't monitor Temperature, ignoring -W Directive

Mar 17 21:57:30 tippex smartd[5810]: Device: /dev/hda, appears to lack SMART Self-Test log; disabling -l selftest (override with -T permissive Directive)

Mar 17 21:57:30 tippex smartd[58...
[ message continues ]

Do you get into ide_error() too? That 176 is WIN_SMART, but you had

earlier WIN_SMART commands that worked ok, so it's something specific that

triggers it. data_phase = 1 is TASKFILE_IN.
So try to see if you get to ide_error(), which is where Bartlomiejs latest

patch was in effect - but add a few printk's there to print out the 'stat'

variable etc to see if it actually triggers. And maybe a WARN_ON(1) to get

the trace..
		Linus
--

Seems we don't. At least this addition didn't yield anything:
ide_startstop_t ide_error (ide_drive_t *drive, const char *msg, u8 stat)

{

        ide_hwif_t *hwif = drive->hwif;

        struct request *rq = hwif->hwgroup->rq;

        u8 err;
              printk("ide_error(msg=%s, stat=%i)\n",msg,stat);

              WARN_ON(1);

               if (rq && rq->cmd_type == REQ_TYPE_ATA_TASKFILE) {
Going to bed...
/A
--

Could you also add printk() to task_in_intr()?
---

 drivers/ide/ide-taskfile.c |    2 ++

 1 file changed, 2 insertions(+)
Index: b/drivers/ide/ide-taskfile.c

===================================================================

--- a/drivers/ide/ide-taskfile.c

+++ b/drivers/ide/ide-taskfile.c

@@ -433,6 +433,8 @@ static ide_startstop_t task_in_intr(ide_
 	/* new way for dealing with premature shared PCI interrupts */

 	if (!OK_STAT(stat, DRQ_STAT, BAD_R_STAT)) {

+		printk(KERN_INFO "%s: %s: stat=%02x\n",

+				 __func__, drive->name, stat);

 		if (stat & (ERR_STAT | DRQ_STAT))

 			return task_error(drive, rq, __FUNCTION__, stat);

 		/* No data yet, so wait for another IRQ. */
--

Mar 18 14:18:26 tippex do_rw_taskfile(task->data_phase=1, tf->command=236

Mar 18 14:18:26 tippex do_rw_taskfile(task->data_phase=0, tf->command=176

Mar 18 14:18:26 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 18 14:18:26 tippex task_in_intr: hda: stat=50
--

Uhhuh. That's READY_STAT | SRV_STAT. No error, no DRQ, no nothing.
And I think this also explains why your bisect found that old commit

4d977e43d8ae758434e603cf2455d955f71c77c4 to be problematic.
The thing is, that commit - and the taskfile code that it then all got

replaced with - use this totally bogus check:
	OK_STAT(stat, DRQ_STAT, BAD_R_STAT)

		...
which basically says that the only OK situation is that no error bits are

set, and DRQ _has_ to be set.
Before that, the code did the right thing in any _combination_ of bits: if

DRQ wasn't set, it would just say "oh, ok, the command is done", but the

taskfile code and the 4d977e43d commit code thinks that DRQ not being set

is an error, and then it gets confused when the actual error bits aren't

set!
I think that whole way of writing code is totally horribly bad! It's not

only trying to tie together bits that are independent, but it actually

gets a lot harder to understand too, because now you have *four* different

cases ("no error, no drq", "error, no drq", "no error, drq", "error, drq")

but you use one test to try to find the one you expect, and then the code

easily messes up on all the other three cases!
I personally think the code should be written more like the suggested

appended patch, which checks those error and drq bits _separately_, and

doesn't try to mix them up, because they really are independent.
Anders, does this patch change any behaviour?
It basically does:
 - if the error bits are set, we just error out (and expect ide_error() to

   flush the data fifo if necessary)
 - if the DRQ bit is *not* set, something unexpected happened, and we go

   out of line to handle that.
 - otherwise we just do the data in phase and see if we're all done.
Then, in the unexpected case, we try to see what is actually going on, and

that's where we do the same thing we always used to do (ie the thing that

commit 4d977e43d messed up): if the drive tells us it's all done, we just

end the co...
[ message continues ]

Which is with compliance with PIO-in protocol specification and years of

usage of the task_in_intr() code for fs code paths and HDIO_DRIVE_TASKFILE

ioctl has proven that real hardware also works this way (please note that
If INTRQ is asserted and "BSY is cleared to zero and DRQ is cleared, then

the device has completed the command with an error." (thus task_in_intr()

assumed ERR bit to be set), otherwise the value ERR may not be defined

(however task_in_intr() still assumed that it is OK to check ERR bit).
Additionally handling of premature shared PCI interrupts comes into the

picture making the whole thing much more messier.  It could happen in the

past that drive_is_ready() was called before drive had time to assert BSY

so later also task_in_intr() assumed that the drive is ready.  However this

should be already fixed as we now always guarantee sufficient delay after

the command was written so how's about the following patch which just makes

DRQ == 0 || BSY == 1 || ERR == 1 an error (ideally we should also proceed

with transfer if DRQ == 1 && ERR == 1 but it may have other gotchas so lets

keep it as it was for now):
---

preferably this should get some testing in -mm first
 drivers/ide/ide-taskfile.c |   11 +++--------

 1 file changed, 3 insertions(+), 8 deletions(-)
Index: b/drivers/ide/ide-taskfile.c

===================================================================

--- a/drivers/ide/ide-taskfile.c

+++ b/drivers/ide/ide-taskfile.c

@@ -431,14 +431,9 @@ static ide_startstop_t task_in_intr(ide_

 	struct request *rq = HWGROUP(drive)->rq;

 	u8 stat = ide_read_status(drive);
-	/* new way for dealing with premature shared PCI interrupts */

-	if (!OK_STAT(stat, DRQ_STAT, BAD_R_STAT)) {

-		if (stat & (ERR_STAT | DRQ_STAT))

-			return task_error(drive, rq, __FUNCTION__, stat);

-		/* No data yet, so wait for another IRQ. */

-		ide_set_handler(drive, &task_in_intr, WAIT_WORSTCASE, NULL);

-		return ide_started;

-	}

+	/* TODO: more complex handling is nee...
[ message continues ]

.. and years of drive_cmd_intr() which is much more widely used is *not* 
First off, the patch I sent out _works_.
Secondly, it's a hell of a lot more robust than yours is, exactly because

it doesn't get confused if the data direction or size bit disagrees with 
.. and what about all the magic special IDE commands that may be

drive-specific? What are we going to do about them?
In other words, we should not try to create an impossible-to-maintain

piece of shit code that does the wrong thing if you send a command to the

drive that the IDE layer doesn't understand (but the sending code

hopefully does).
We should make the core IDE code *robust*. 
Your "real fix" is nothing of the sort. It's just a workaround for the

fragility of the code that looks at the drive status. The real fix is to

be robust in the face of even unexpected drive status codes, *especially*

for the code that handles commands injected by the user!
In other words, you can talk about protocol specifications for things like

the regular filesystem READ/WRITE commands. But don't create total crap

like this that depends on the code knowing all possible outcomes of every

single possible command.
Your patch is utter crap.
I would call that *correct*.
And my point is, we used to be better. You made the code buggy and fragile

with that crap commit, and with the others like it (ie the already

much-mentioned commit 4d977e43d8ae758434e603cf2455d955f71c77c4).
And that is and was *exactly* my point. The reason I called the taskfile

code horrible was exactly the fact that it only worked if it thought it

knew exactly what was going on.
Deciding what to do based on the DRQ bit (and the READY/BUSY/ERROR bits)

is the *right* thing to do. It's the intelligent approach - actually

tekign the response of the hardware into account, and being robust. The

*stupid* and horrible thing to do is to think that you know better than

what the hardware tells you, and think that you can look up every com...
[ message continues ]

Please take a closer look - my "real fix" _only_ affects WIN_SMART command

and _not_ vendor special ones (no, there are none vendor special commands

using the same opcode).
This is actually how the ATA hardware operates.
You cannot just bang random commands and expect that later driver will be

able to 100% correctly deduce what the command wants from the drive status

itself (this may work more-or-less well for no-data and PIO-in by using DRQ

bit trick, but we also have PIO-out, DMA, PACKET etc).
IOW this won't fly as drive/controller alone doesn't provide use with enough
There is some misunderstaning here - taskfile approach lets user specify

_both_ the command and what it really wants (protocol etc.), and no - we

don't do any command checking with the spec but give user the control...
Call taskfile crap all you want but the fact is that taskfile approach

handles _all_ protocols and _all_ commands while the beloved "robust"

and "intelligent" approach is able to handle only 28-bit commands using

no-data or PIO-in protocols (this is what HDIO_DRIVE_CMD is supporting

and there is no way it will ever support more than that with its design).
The problem here is that we cannot fully trust neither hardware nor the
Please re-consider this decision given the above facts.
Thanks,

Bart

--

Oh, trust me, I understand your fix.
But the point is that your fix
 - doesn't fix any other commands (and there are tons of other commands

   people may be using)
 - and is totally pointless and isn't *needed* if we just fix this

   properly (ie we've never cared before, so why should we start caring

   now?).
See?
Fixing this properly is exactly what my patch did - it made the whole core 
You're totally not listening or understanding.
I'm not calling taskfile crap as a concept.
I'm calling fragile code that fails unnecessarily crap.
See the difference?
The old "drive_cmd_intr()" code (that you deleted) was fundamentally more

robust than the taskfile code you replaced it with. 
And that's not just an opinion. It's a *fact*. It's why this bug showed up

in the first place. Code that used to work because it didn't even care all

that deeply about every single detail being set up just the way it wanted

got replaced by code that was fragile.
Do you see the difference?
If we have a choice between robust code that just "does the right thing"

even in the face of problems, and code that "stops working when you look

at it wrong", which one should we choose? Which one is the better code?

Which one is crap, and which one isn't?
The fact is, the old "drive_cmd_intr()" code was simply more robust.
So this is why I feel so strongly about this. Robust code is just about

the most important thing we can have in the kernel. Bugs will always

happen, but when they happen, we shouldn't just fall over dead. And that's

exactly what code robustness is all about.
So we should make the DRQ/ERR status bit handling robust in the face of

hardware and software that does odd things. Because we definitely have

seen cases of both. Sometimes it is hw that doesn't work the way the spec

requires, sometimes it's software that doesn't follow the spec 100%. It

doesn't matter.
And once the status bit handling is robust (like it _used_ to be!), only

*then* should...
[ message continues ]

An example of this "aim higher", I think that we obviously should complete

the command if the controller says it's done with it (like the old

"drive_cmd_intr()" code used to do), but we might decide that printing a

warning would be the proper way to then also inform the user about the

fact that we expected the command to want more data, but it never did.
That way, the code not only handles the unexpected situation gracefully,

but the very fact that it was unexpected also gets logged.
[ And at that point your patch to make the specific SMART subcommands have

  strict logic about the data direction will actually matter - although

  maybe it turns out that there are _other_ tools that send other

  commands, and we decide that the warning isn't useful after all! ]
The thing is, I know for a fact that some system vendors use Linux for

burn-in and testing/setup of their components, and I would not be in the

least surprised if some of them use HDIO_DRIVE_CMD to do some

vendor-specific stuff.
And I think it's a fine thing to try to use just one set of code (the

taskfile code) to drive all these special commands, but the fact is, the

old HDIO_DRIVE_CMD interface fundamentally doesn't even _have_ the

capability to specify the total taskfile state.
So we simply know as a fact that

 (a) people do use an interface that used to work (HDIO_DRIVE_CMD)

and

 (b) it will fundamentally *never* have complete taskfile state, so we

     *have* to live with the fact that some of our commands simply don't

     have the full state that the taskfile code may have historically

     depended on but the old ide_drive_cmd() code didn't and cannot depend

     on.
Your patch tries to fix (b), but we also know that it fundamentally simply

*cannot* fix it for all cases. So even trying to is really pretty futile,

I think (although we can obviously hope that nobody uses HDIO_DRIVE_CMD

any more for anything but SMART and really simple commands, but that

sounds rather unlikely).
...
[ message continues ]

OK, I got the point.
Your patch is more robust and we should go with it

(and thanks for fixing this bug!).
Bart

--

Ok, I committed the patch as-is, since it's what Anders tested, but I'm

not at all convinced that it is necessarily the best or final form. The

things I am aware of but didn't think about all *that* deeply:
 - do we get return values etc right (ie if we complete the command that

   didn't give any data, how do we account the size of it?)
 - what about that remaining old unexpected case? I kept the "wait for it

   with timeout" behaviour for the case that wasn't an issue here, but if

   it really is a shared interrupt, that seems like it's going to always

   reset the timeout to WAIT_WORSTCASE, which doesn't sound really right.
so I think this particular bug is fixed and we should be better off, but

I'm definitely not claiming that the code shouldn't have people thinking

about improving it..
			Linus

--

This shared IRQs quirk should no longer be necessary so probably the best

way to handle BSY there would be to give drive some last chance and if it
Definitevely, I'll try to address "old unexpected case" for 2.6.26

(+ I'll keep looking for other issues as time permits) and of course

I'll be happy to review/merge any patches improving this area.
Bart

--

Seems we got something here. For all I can tell, it's working!
I had to hand apply the last hunk of your patch, and the other patches are there as

well. Can you respin it vs. -rc6 and I'll try on a clean tree?
/A
Mar 18 17:23:12 tippex smartd[5922]: Home page is http://smartmontools.sourceforge.net/

Mar 18 17:23:12 tippex smartd[5922]: Opened configuration file /etc/smartd.conf

Mar 18 17:23:13 tippex smartd[5922]: Configuration file /etc/smartd.conf parsed.

Mar 18 17:23:13 tippex smartd[5922]: Device: /dev/hda, opened

Mar 18 17:23:13 tippex do_rw_taskfile(task->data_phase=1, tf->command=236

Mar 18 17:23:13 tippex hda: tf: feat 0x00 nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00 dev 0x00 cmd 0xec

Mar 18 17:23:13 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 17:23:13 tippex smartd[5922]: Device: /dev/hda, found in smartd database.

Mar 18 17:23:13 tippex do_rw_taskfile(task->data_phase=0, tf->command=176

Mar 18 17:23:13 tippex hda: tf: feat 0xd8 nsect 0x00 lbal 0x01 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 17:23:13 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 17:23:13 tippex do_rw_taskfile(task->data_phase=0, tf->command=176

Mar 18 17:23:13 tippex hda: tf: feat 0xda nsect 0x00 lbal 0x00 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 17:23:13 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 17:23:13 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 18 17:23:13 tippex hda: tf: feat 0xd0 nsect 0x01 lbal 0x00 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 17:23:13 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 17:23:13 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 18 17:23:13 tippex hda: tf: feat 0xd1 nsect 0x01 lbal 0x01 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 17:23:13 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 17:23:13 tippex do_rw_taskfile(task->data_phase=1, tf->command=236

Mar 18 17:23:13 tippex hda: tf: feat 0x00 nsect ...
[ message continues ]

Actually, the patch I sent you should be against the current -git tree,

and I don't think anything has changed since -rc6 in this area.
I suspect the reason you had to hand-apply it may be the other

test-patches you had. Can you just undo those? Since it looks like it may

be working for you, the debug patches are just adding tons of noise by

now, and it would be good to hear whether the status with that last patch

is just a simple "ok, it works now".
		Linus

--

Latest git with your patch works perfectly.
Thanks a lot!!
/Anders
--

This is quite unexpected status value...
Please add "#define DEBUG" before ide_tf_load() in ide-taskfile.c

so we can see which SMART sub-command is causing this.
Thanks,

Bart

--

Mar 18 16:02:13 tippex do_rw_taskfile(task->data_phase=0, tf->command=176

Mar 18 16:02:13 tippex hda: tf: feat 0xda nsect 0x00 lbal 0x00 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 16:02:13 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 16:02:14 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 18 16:02:14 tippex hda: tf: feat 0xd0 nsect 0x01 lbal 0x00 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 16:02:14 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 16:02:14 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 18 16:02:14 tippex hda: tf: feat 0xd1 nsect 0x01 lbal 0x01 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 16:02:14 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 16:02:14 tippex do_rw_taskfile(task->data_phase=1, tf->command=236

Mar 18 16:02:14 tippex hda: tf: feat 0x00 nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00 dev 0x00 cmd 0xec

Mar 18 16:02:14 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 16:02:14 tippex do_rw_taskfile(task->data_phase=0, tf->command=176

Mar 18 16:02:14 tippex hda: tf: feat 0xd8 nsect 0x00 lbal 0x01 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 16:02:14 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 16:02:14 tippex do_rw_taskfile(task->data_phase=1, tf->command=176

Mar 18 16:02:14 tippex hda: tf: feat 0xd2 nsect 0xf1 lbal 0x00 lbam 0x4f lbah 0xc2 dev 0x00 cmd 0xb0

Mar 18 16:02:14 tippex hda: hob: nsect 0x00 lbal 0x00 lbam 0x00 lbah 0x00

Mar 18 16:02:14 tippex task_in_intr: hda: stat=50
--

I just lost track at what is confirmed and what not...
Anders, does the kernel at commit 852738f39258deafb3d89c187cb1a4050820d555

also fail or not?
Thanks,

Bart

--

Turns out it does work.
FWIW, here's the config I'm now using as base for every compile, just

accepting the default whenever asked.
#

# Automatically generated make config: don't edit

# Linux kernel version: 2.6.24

# Tue Mar 11 16:15:47 2008

#

# CONFIG_64BIT is not set

CONFIG_X86_32=y

# CONFIG_X86_64 is not set

CONFIG_X86=y

CONFIG_GENERIC_TIME=y

CONFIG_GENERIC_CMOS_UPDATE=y

CONFIG_CLOCKSOURCE_WATCHDOG=y

CONFIG_GENERIC_CLOCKEVENTS=y

CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y

CONFIG_LOCKDEP_SUPPORT=y

CONFIG_STACKTRACE_SUPPORT=y

CONFIG_SEMAPHORE_SLEEPERS=y

CONFIG_MMU=y

CONFIG_ZONE_DMA=y

CONFIG_QUICKLIST=y

CONFIG_GENERIC_ISA_DMA=y

CONFIG_GENERIC_IOMAP=y

CONFIG_GENERIC_BUG=y

CONFIG_GENERIC_HWEIGHT=y

CONFIG_ARCH_MAY_HAVE_PC_FDC=y

CONFIG_DMI=y

# CONFIG_RWSEM_GENERIC_SPINLOCK is not set

CONFIG_RWSEM_XCHGADD_ALGORITHM=y

# CONFIG_ARCH_HAS_ILOG2_U32 is not set

# CONFIG_ARCH_HAS_ILOG2_U64 is not set

CONFIG_GENERIC_CALIBRATE_DELAY=y

# CONFIG_GENERIC_TIME_VSYSCALL is not set

CONFIG_ARCH_SUPPORTS_OPROFILE=y

# CONFIG_ZONE_DMA32 is not set

CONFIG_ARCH_POPULATES_NODE_MAP=y

# CONFIG_AUDIT_ARCH is not set

CONFIG_GENERIC_HARDIRQS=y

CONFIG_GENERIC_IRQ_PROBE=y

CONFIG_X86_BIOS_REBOOT=y

CONFIG_KTIME_SCALAR=y

CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
#

# General setup

#

CONFIG_EXPERIMENTAL=y

CONFIG_BROKEN_ON_SMP=y

CONFIG_LOCK_KERNEL=y

CONFIG_INIT_ENV_ARG_LIMIT=32

CONFIG_LOCALVERSION=""

CONFIG_LOCALVERSION_AUTO=y

CONFIG_SWAP=y

CONFIG_SYSVIPC=y

CONFIG_SYSVIPC_SYSCTL=y

# CONFIG_POSIX_MQUEUE is not set

# CONFIG_BSD_PROCESS_ACCT is not set

# CONFIG_TASKSTATS is not set

# CONFIG_USER_NS is not set

# CONFIG_PID_NS is not set

# CONFIG_AUDIT is not set

CONFIG_IKCONFIG=y

CONFIG_IKCONFIG_PROC=y

CONFIG_LOG_BUF_SHIFT=16

# CONFIG_CGROUPS is not set

CONFIG_FAIR_GROUP_SCHED=y

CONFIG_FAIR_USER_SCHED=y

# CONFIG_FAIR_CGROUP_SCHED is not set

CONFIG_SYSFS_DEPRECATED=y

# CONFIG_RELAY is not set

# CONFIG_BLK_DEV_INITRD is not set

# CONFIG_CC_OPTIMIZE_FOR_SIZE is not...
[ message continues ]

On Sun, 16 Mar 2008 11:13:42 -0700 (PDT)
No it doesn't. DRQ simply means "drive has more data for the controller

if you want it". Interrupts are controlled via IEN and the interrupt line.
If the drive wants to give us data and we end the transaction that is

fine. In practice a tiny few devices crap themselves if we don't.
A few controllers require specific magic, either to ensure we never touch

data after an error, or that we drain enough bits to empty the internal

fifo the controller is using to improve ata performance and latches the

IRQ arrival against.
The PIIX/ICH is as it happens one of the most forgiving controllers

anyway. The later ICH (the ones that are also AHCI) are a bit fussier

about handling them to the spec because they seem to be some kind of

magic ICH emulation layer on an AHCI chip.
Alan
--

A _lot_ of chips require you to clear the DRQ by taking the data they 
More than a few tiny devices from what I remember. It tends to be the

other way around - most devices do *not* want to get new commands until

you've finished the previous one by draining the queues.
		Linus

--

On Sun, 16 Mar 2008 12:39:43 -0700 (PDT)
Almost none and mostly very old ones. I'm not saying we shouldn't do it

(except where it hangs the hardware - hence the FIFO flag) but for the

traces presented and hardware reported it appears to be a bit of a red
Not in my experience having maintained a lot of ATA drivers for a very

long time. In fact the changes for draining the DRQ went into libata only

very recently because it was only when we had a distro sized userbase

with PATA devices that it became apparent that a few corner case problems

remained.
Alan

--

..
Err.. I have a fairly modern PATA drive (160GB seagate 2.5")

that gets pissed if we leave DRQ hanging.  So it's not just

something for old/obsolete drives.
Or maybe it's the also-modern-ish ICH chipset that gets stuck.  Whatever.
-ml

--

ICH's with AHCI support seem to get peeved.
Alan

--

Well, I admittedly haven't been involved in IDE in about a decade, so

yeah, maybe it's simply no longer true. That said, if the second bisection

was accurate (which is admittedly a rasonably big "if"), it really looks

like it would be related to the fact that we used to empty the data buffer 
.. but as you noticed, it's almost never wrong to drain (the only chipset

it's marked for is some broken pdc202xx one), and it definitely *is* wrong

not to drain.
Also, one reason you'd not necessarily even see this is that with DMA,

this is a non-issue (since the hardware DMA engine will be doing all the

draining), so in order to ever see this you have to still use PIO _and_

you have to see IDE command errors in the first place _and_ you have to

have a device that actually keeps DRQ enabled even at an error.
All of which are hopefully fairly rare by now (and getting rarer, at last

for the PIO one).
I also wouldn't be entirely surprised if the DRQ behavior may even be

command-specific, with the regular data path for read/write quite possibly

being different from the special commands that go through some internal

drive firmware logic paths.
So I could well imagine (for example) that when a drive raises an IO error

due to a read or write fault, the DRQ line will be cleared by the drive,

but that special commands might have some firmware-directed separate FIFO

that needs draining.
				Linus

--

On Sunday 16 March 2008, Bartlomiej Zolnierkiewicz wrote:
Alt-SysRq-T traces that Anders posted shows that the system hangs inside

block layer, if not clearing DRQ would be the case we should see system

getting stuck inside IDE (or at least IDE commands timing out)...
I really do think that it is the same hang-in-block-layer issue that

Ingo managed to hit once (his system was using libata)...
Thanks,

Bart

--

The bisect ends up pointing at IDE bits everytime, plus it's io
There's no data to back any such conclusion up, that would be extremely

hand wavy (at best).
Ingo hit a hang in one boot out of hundreds to thousands. Anders can

reproduce easily, should be easy enough to narrow down.
--

Jens Axboe
--

When I tried libata, smartd runs just fine, FWIW.
--

I don't get any eror message anywhere.
I'd be happy to try out debug patches to trace the call path down and sprinkle

it with printks. But, shouldn't the sysrq-t log posted earlier give any clues

as to where things tank? 
--

That is instant system death on some chipsets. The state machine on the

promise for example empties the FIFO and if you touch the data port again

before issuing a command you lock the PCI bus.
Alan

--

I did that first, and it pointed to  8527* as the bad one. During this bisect

I took care to always start from a 2.6.24 .config, and do a make oldconfig from

that. A clean series of good emerged.. Unfortunately I don't have the configs

from the previous runs saved.
Given this, I concluded that the configs had to play a role in making a

version go bad, so I decided to do another one between  8527* and 2.6.25-rc1

always based on the  8527* config (which _worked_ when tested with the 2.6.24

config dragged forward. So the upped end of the first bisect was badly chosen)
--

Given that the problem manifested with 8527* in the past (w/ whatever config)

then 4d97* just cannot be a guilty commit (== the one introducing the problem).
IOW the 8527* and later commits just make the issue more likely to occur

(moreover there should be no difference in configs between 8527* and 4d97*).
I guess that re-doing bisection between 2.6.24 and 8527* may be fruitless

but maybe somebody has some idea how we can proceed further with debugging

the hang instead?  Jens/Ingo?
Thanks,

Bart

--

I share the same worry. Towards the end of the bisect run (something like the

4-th last reboot), I was asked to try "2.6.24". Now, I _thought_ 2.6.24 was way

before 852738f39258deafb3d89c187cb1a4050820d555, and hence it should be called

2.X.Y-foobaz something as the others were. Is this the way it should be, or did

I fscked up the bisect?
This was a bisect run between 852738f39.. and 2.5.25-rc1. I got a string of

"bad"s but TWO goods, actually. Those goods sustained a number of reruns of

smartd (I can share the BISECT_LOG if wanted).
And how we can end up with good_start+1 as the guilty one, and STILL have two

good ones during the bisect run..... That's beyond me. lets just say that my

faith in myself and/or bisect starts to decline...
Now I'm considering a 2.6.24 .. 8527 run.

/A
/A
--

8527 was merged before -rc1 so it is expected that resulting kernel was

called 2.6.24 (though it wasn't really 2.6.24) if CONFIG_LOCALVERSION_AUTO
This is worth a try but please remember that the problem may not show up

immediately so it would require few tests to verify each commit (it is also

possible that the problem won't show up at all if we are unlucky).
Thanks,

Bart

--

me neither... just wanted to give notice that something's brewing in

this area. Will know more after tonight's qa series i guess, if it gets

above 100 bootups ;-)
	Ingo

--

no luck - a few hundred successful randconfig bootups overnight (with a

rather complex userspace startup to full X plus networking, and the

testbox is used via the network to compile the next random kernel as

well - etc.) and no failure at all. So there's not much we can do at

this point - i'll keep you updated if anything new happens in the tests.
	Ingo

--

How many machines are there in your test farm? Would it be possible to

make the neccessary scripts available somewhere? I guess I could make

one or two machines do some automated testing...
--

(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

--

You can also use autoboot
ftp://ftp.firstfloor.org/pub/ak/autoboot/autoboot-1.tgz
[hmm I think it's a slightly older version, but should not

be too difficult to adapt]
These are the scripts we use for automatic testing regularly in Nuernberg.
It's a little different from Ingo's setup I think though and you

really need to use remote controllable power switches for it to work

well [I personally use some cheap USB controlled ones]
-Andi

--

I lack the power switches. I wonder how Ingo gets around that?
...OTOH power switch should be only needed when we hit a kernel bug,

that should hopefully be rare...
									Pavel

--

(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

--

I use gembird USB powerswitches. They are cheap and work
It happens more often than you think.
-Andi

--

the thing that took most time to develop were the Kconfig space changes,

i've released them 2 months ago:
   http://people.redhat.com/mingo/auto-qa-patches/
the scripts that install a bzImage on the target system are boring and

tied to my environment/hostnames.
with the auto-qa Kconfig patch applied, every "make randconfig" bzImage

(on x86) is supposed to boot up fine - if it doesnt it's a

bug/regression. Add entries to arch/x86/Kconfig.needed to force-enable

drivers that are needed on your boxes.
	Ingo

--

OK, so it could possibly be something else or perhaps something that's

existed for a long time.
To capture vital debugging information for cases like this, I had an

idea for a sysrq key that would dump queue and io scheduler info for all

known block devices. That should help pin point where the problem lies.

If we have any letters left in the sysrq namespace :-)
--

Jens Axboe
--

Time for alt-sysrq-cokebottle? :)

btw., we should reserve one sysrq letter as a "meta" character. If that

letter is 'X' [for Xtended SysRq] then SysRq-X SysRq-B could dump the

block info.
	Ingo

--

Not 'x', please, since we use that on powerpc already.  But otherwise

that's a good idea.
Paul.

--

Could be old brew, depends on when/how you can re-trigger it :-)
--

Jens Axboe
--

On Tue, 4 Mar 2008 21:03:44 -0800 (PST)
Ah, the IOMMU patches (alpha and parisc) that I submitted for -mm have

been merged somehow.
The parisc patches were tested but probably the alpha patches not.
Here's a patch for 32bits arch parisc.
=

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

Subject: [PATCH] parisc: fix IOMMU's device boundary overflow bug on 32bits arch
On 32bits boxes, boundary_size becomes zero due to a overflow and we

hit BUG_ON in iommu_is_span_boundary.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

Cc: Kyle McMartin <kyle@parisc-linux.org>

Cc: Matthew Wilcox <matthew@wil.cx>

Cc: Grant Grundler <grundler@parisc-linux.org>

---

 drivers/parisc/ccio-dma.c  |    4 ++--

 drivers/parisc/sba_iommu.c |    4 ++--

 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c

index 60d338c..62db3c3 100644

--- a/drivers/parisc/ccio-dma.c

+++ b/drivers/parisc/ccio-dma.c

@@ -366,8 +366,8 @@ ccio_alloc_range(struct ioc *ioc, struct device *dev, size_t size)

 	** ggg sacrifices another 710 to the computer gods.

 	*/
-	boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1, 1 << IOVP_SHIFT);

-	boundary_size >>= IOVP_SHIFT;

+	boundary_size = ALIGN((unsigned long long)dma_get_seg_boundary(dev) + 1,

+			      1ULL << IOVP_SHIFT) >> IOVP_SHIFT;
 	if (pages_needed <= 8) {

 		/*

diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c

index e834127..bdbe780 100644

--- a/drivers/parisc/sba_iommu.c

+++ b/drivers/parisc/sba_iommu.c

@@ -341,8 +341,8 @@ sba_search_bitmap(struct ioc *ioc, struct device *dev,

 	unsigned long shift;

 	int ret;
-	boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1, 1 << IOVP_SHIFT);

-	boundary_size >>= IOVP_SHIFT;

+	boundary_size = ALIGN((unsigned long long)dma_get_seg_boundary(dev) + 1,

+			      1ULL << IOVP_SHIFT) >> IOVP_SHIFT;
 #if defined(ZX1_SUPPORT)

 	BU...
[ message continues ]

Acked-by: Grant Grundler <grundler@parisc-linux.org>
thanks,

--