linux-kernel - Re: Linux 2.6.25-rc4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 16 Mar 2008 09:56:25 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Anders Eriksson <aeriksson@...tmail.fm>
cc:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Jens Axboe <jens.axboe@...cle.com>,
	Ingo Molnar <mingo@...e.hu>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Linux 2.6.25-rc4 

On Sun, 16 Mar 2008, Anders Eriksson wrote:
> 
> 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).

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)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/