Re: [patch] scsi: revert "[SCSI] Get rid of scsi_cmnd->done"

From: Matthew Wilcox
Date: Wed Jan 02 2008 - 14:40:41 EST


On Wed, Jan 02, 2008 at 11:19:14AM -0800, Linus Torvalds wrote:
> It's totally immaterial if we have one reporter or many. The fact is, that
> thing has been outstanding for almost two months now. The root cause is
> almost certainly known (and Matthew is apparently even aware of it), but

I really don't think the root cause is known.

> By now, it needs to either have a patch, or to be reverted. It's too late
> to make excuses.

I think reverting it is the correct thing to do.

> And no, it doesn't really need any more reporters or ways to reproduce it,
> all it needs is looking at the patch and seeing what the differences are
> AT A SOURCE level. And quite frankly, I see one huge honking difference,
> which is that new test for
>
> if (cmd->request->cmd_type != REQ_TYPE_BLOCK_PC)
>
> which will disable all the crud that sd_done() does.

sd_done and sr_done are called for REQ_TYPE_FS -- if the request comes
in through one of the SG interfaces, we call scsi_setup_blk_pc_cmnd()
which sets the ->done callback to scsi_blk_pc_done.

> So I think you are making totally idiotic excuses. I may not know the SCSI
> layer all that intimately, and maybe I'm wrong and there is some other
> cause for this, but quite frankly, I doubt it. And I can look at just the
> patch and have a damn good idea of why something is broken, but I can't
> actually fix it myself because I don't know how to look up a a
> "scsi_driver" from a "scsi_cmnd" sanely.

That's in the patch. But it would be the wrong thing to do because SG
commands should not be molested by the sr/sd driver.

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/