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]

Message-Id: <200803161907.04581.bzolnier@gmail.com>
Date:	Sun, 16 Mar 2008 19:07:04 +0100
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Anders Eriksson <aeriksson@...tmail.fm>,
	"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 Sunday 16 March 2008, Linus Torvalds wrote:
> 
> 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.

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
  it can be related to the problem experienced by Anders. ]

> So what probably happens is that yes, you get an error, but the IDE drive 

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
--
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/