Re: Linux 2.6.25-rc4

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

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