Alan Stern wrote:
On Thu, 29 Sep 2005, Mike Christie wrote:
Alan Stern wrote:
James:
This report is based on 2.6.14-rc2-git6. The code in your scsi-misc-2.6
git tree is somewhat different (and I don't know which is more current),
but it still contains the same bug.
In scsi_prep_fn, a request can get deferred if scsi_init_io fails to
allocate an sg table. When this happens, the scsi_cmnd isn't released and
the request is not marked DONTPREP.
Then when scsi_prep_fn is called again, the request may be killed for
a number of reasons. The code branches to the kill: label near the end of
the routine, which returns BLKPREP_KILL.
Isn't it true that when this happens, the scsi_cmnd allocated during the
original prep will never be released?
It appears that scsi_prep_fn is undecided about whether or not the request
is allowed to have a scsi_cmnd already. The jumps to kill: seem to assume
that it isn't, but the code for allocating a new scsi_cmnd tests for an
existing one first.
The gotos used to be just a return BLKPREP* and were added so I did not
have to write DID_NO_CONNECT or unplug multiple times :) I think you are
right and we need to further unwind what a previous prep had done
becuase when we return with BLKPREP_KILL we only hear about this command
again if it's request has a end_io function or waiting completion.
I'm still not very clear about the conditions under which a request on the
queue can be partially prepared -- for example, scsi_cmnd assigned but
not the sg table. For the normal submission pathways, it looks like this
happens only when the sg allocation fails. In those cases it wouldn't
hurt to release the scsi_cmnd before deferring. Or before returning
BLKPREP_KILL.
But what about other pathways? As long as the special scsi_request things
exist, I don't know what should be done. I saw you had submitted patches
to get rid of them; how far has that progressed?
I am not done. I still have osst to convert and I think Doug found a bug
I cannot reproduce. I was not sure if everyone was happy with the
max_sectors and the SCSI_MAX_PHYS_SEGMENTS compile option either.
I think I need to change the interface too. Passing in a scatterlist is
nice beucase we do not have to touch the ULDs much, but I think if we
went Christoph's route and used a array of bio_bvecs it might be nicer.
We would need a bio helper function that could build bios from bvecs
though and I think maybe that should be done based on the bioset stuff.
I am not sure though.
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
