[lkml]   [2008]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [patch] scsi: revert "[SCSI] Get rid of scsi_cmnd->done"

On Wed, 2 Jan 2008, James Bottomley wrote:
> I disagree with this. We only have one reporter of a problem and it
> appears to be some type of obscure interaction with pktdvd which no-one
> can track down (although it's not really helped by the reporter not
> being very responsive).

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
it has been dicked-around-with by having totally weak excuses ("is it an
ide-scsi thing?")

> The correct thing to do is root cause the problem and fix it at source,
> since it's very likely that this is a pre-existing bug that was simply
> uncovered by the patch you're recommending we revert.

No, the correct thing would have been to fix it a month ago.

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

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.

And guess what? Look at what sd_done() does: it checks whether the request
was partially filled, and tries to handle that end-of-medium case nicely.
Which is *exactly* what seems to have broken.

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.

But almost two months after the fact, we should have had a patch that does
that, or we should revert it.



 \ /
  Last update: 2008-01-02 20:23    [W:0.062 / U:8.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site