On 03/28/2013 02:32 PM, Jeff Moyer wrote: > Hi, > > If blk_get_requet fails here, it means that the queue is dead. It seems > better to return a DEV_OFFLINED error code than the misleading > TEMP_UNAVAIL. Comments? > > Signed-off-by: Jeff Moyer <jmoyer@xxxxxxxxxx> > > diff --git a/drivers/scsi/device_handler/scsi_dh_hp_sw.c b/drivers/scsi/device_handler/scsi_dh_hp_sw.c > index 084062b..eec24d3 100644 > --- a/drivers/scsi/device_handler/scsi_dh_hp_sw.c > +++ b/drivers/scsi/device_handler/scsi_dh_hp_sw.c > @@ -118,7 +118,7 @@ static int hp_sw_tur(struct scsi_device *sdev, struct hp_sw_dh_data *h) > retry: > req = blk_get_request(sdev->request_queue, WRITE, GFP_NOIO); > if (!req) > - return SCSI_DH_RES_TEMP_UNAVAIL; > + return SCSI_DH_DEV_OFFLINED; Can we always assume that if using GFP_NOIO then resource allocation failures will never be returned to us? If so, then the patch is fine. If not then do we want to base this patch on top of "block: handle pointer error from blk_get_request" and check the return value. If -ENOMEM then return SCSI_DH_RES_TEMP_UNAVAIL, and if -ENODEV then return SCSI_DH_DEV_OFFLINED. You probably then want to also fix up scsi_dh_alua and scsi_dh_emc the same way. It looks like they have the same issue. scsi_dh_emc looks like it has extra errors in that the failure checks for send_inquiry_cmd do not handle get_req failures properly. It looks like it ignore them if the senselen is not set. > > req->cmd_type = REQ_TYPE_BLOCK_PC; > req->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | > -- > To unsubscribe from this list: 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 > -- To unsubscribe from this list: 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
