| Message ID | 1440679281-13234-5-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 27, 2015 at 02:41:02PM +0200, Hannes Reinecke wrote: > } > > err = alua_check_sense(sdev, &sense_hdr); > - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) > + if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) { > + sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", > + ALUA_DH_NAME); > + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); > goto retry; > - sdev_printk(KERN_INFO, sdev, > - "%s: rtpg sense code %02x/%02x/%02x\n", > - ALUA_DH_NAME, sense_hdr.sense_key, > - sense_hdr.asc, sense_hdr.ascq); > - err = SCSI_DH_IO; > + } > + sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n", > + ALUA_DH_NAME); > + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); > + return SCSI_DH_IO; > } > - if (err != SCSI_DH_OK) > - return err; I think you need to keep this if, given that submit_rtpg can return others error than SCSI_DH_IO as well. While you're at it you might remove the h->senselen check, and kill the assignment of the scsi_normalize_sense bool return value to the err variable similar to how you did in stpg_endio. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/01/2015 11:48 AM, Christoph Hellwig wrote: > On Thu, Aug 27, 2015 at 02:41:02PM +0200, Hannes Reinecke wrote: >> } >> >> err = alua_check_sense(sdev, &sense_hdr); >> - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) >> + if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) { >> + sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", >> + ALUA_DH_NAME); >> + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); >> goto retry; >> - sdev_printk(KERN_INFO, sdev, >> - "%s: rtpg sense code %02x/%02x/%02x\n", >> - ALUA_DH_NAME, sense_hdr.sense_key, >> - sense_hdr.asc, sense_hdr.ascq); >> - err = SCSI_DH_IO; >> + } >> + sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n", >> + ALUA_DH_NAME); >> + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); >> + return SCSI_DH_IO; >> } >> - if (err != SCSI_DH_OK) >> - return err; > > I think you need to keep this if, given that submit_rtpg can return > others error than SCSI_DH_IO as well. > > While you're at it you might remove the h->senselen check, and > kill the assignment of the scsi_normalize_sense bool return value > to the err variable similar to how you did in stpg_endio. > Okay, will be doing so. Cheers, Hannes
On Thu, 2015-08-27 at 14:41 +0200, Hannes Reinecke wrote: > Use standard logging functions instead of hand-crafted ones. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/scsi/device_handler/scsi_dh_alua.c | 34 +++++++++++++----------------- > 1 file changed, 15 insertions(+), 19 deletions(-) > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c > index a20c8bf..7b43ee3 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -23,6 +23,7 @@ > #include <linux/delay.h> > #include <linux/module.h> > #include <scsi/scsi.h> > +#include <scsi/scsi_dbg.h> > #include <scsi/scsi_eh.h> > #include <scsi/scsi_dh.h> > > @@ -194,22 +195,16 @@ static void stpg_endio(struct request *req, int error) > goto done; > } > > - if (req->sense_len > 0) { > - err = scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE, > - &sense_hdr); > - if (!err) { > - err = SCSI_DH_IO; > - goto done; > - } > + if (scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE, > + &sense_hdr)) { > err = alua_check_sense(h->sdev, &sense_hdr); > if (err == ADD_TO_MLQUEUE) { > err = SCSI_DH_RETRY; > goto done; > } > - sdev_printk(KERN_INFO, h->sdev, > - "%s: stpg sense code: %02x/%02x/%02x\n", > - ALUA_DH_NAME, sense_hdr.sense_key, > - sense_hdr.asc, sense_hdr.ascq); > + sdev_printk(KERN_INFO, h->sdev, "%s: stpg failed\n", > + ALUA_DH_NAME); > + scsi_print_sense_hdr(h->sdev, ALUA_DH_NAME, &sense_hdr); > err = SCSI_DH_IO; > } else if (error) > err = SCSI_DH_IO; > @@ -530,16 +525,17 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_ > } > > err = alua_check_sense(sdev, &sense_hdr); > - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) > + if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) { > + sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", > + ALUA_DH_NAME); > + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); > goto retry; > - sdev_printk(KERN_INFO, sdev, > - "%s: rtpg sense code %02x/%02x/%02x\n", > - ALUA_DH_NAME, sense_hdr.sense_key, > - sense_hdr.asc, sense_hdr.ascq); > - err = SCSI_DH_IO; > + } > + sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n", > + ALUA_DH_NAME); > + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); > + return SCSI_DH_IO; > } > - if (err != SCSI_DH_OK) > - return err; > > len = (h->buff[0] << 24) + (h->buff[1] << 16) + > (h->buff[2] << 8) + h->buff[3] + 4; Reviewed-by: Ewan D. Milne <emilne@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c index a20c8bf..7b43ee3 100644 --- a/drivers/scsi/device_handler/scsi_dh_alua.c +++ b/drivers/scsi/device_handler/scsi_dh_alua.c @@ -23,6 +23,7 @@ #include <linux/delay.h> #include <linux/module.h> #include <scsi/scsi.h> +#include <scsi/scsi_dbg.h> #include <scsi/scsi_eh.h> #include <scsi/scsi_dh.h> @@ -194,22 +195,16 @@ static void stpg_endio(struct request *req, int error) goto done; } - if (req->sense_len > 0) { - err = scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE, - &sense_hdr); - if (!err) { - err = SCSI_DH_IO; - goto done; - } + if (scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE, + &sense_hdr)) { err = alua_check_sense(h->sdev, &sense_hdr); if (err == ADD_TO_MLQUEUE) { err = SCSI_DH_RETRY; goto done; } - sdev_printk(KERN_INFO, h->sdev, - "%s: stpg sense code: %02x/%02x/%02x\n", - ALUA_DH_NAME, sense_hdr.sense_key, - sense_hdr.asc, sense_hdr.ascq); + sdev_printk(KERN_INFO, h->sdev, "%s: stpg failed\n", + ALUA_DH_NAME); + scsi_print_sense_hdr(h->sdev, ALUA_DH_NAME, &sense_hdr); err = SCSI_DH_IO; } else if (error) err = SCSI_DH_IO; @@ -530,16 +525,17 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_ } err = alua_check_sense(sdev, &sense_hdr); - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) + if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) { + sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", + ALUA_DH_NAME); + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); goto retry; - sdev_printk(KERN_INFO, sdev, - "%s: rtpg sense code %02x/%02x/%02x\n", - ALUA_DH_NAME, sense_hdr.sense_key, - sense_hdr.asc, sense_hdr.ascq); - err = SCSI_DH_IO; + } + sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n", + ALUA_DH_NAME); + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); + return SCSI_DH_IO; } - if (err != SCSI_DH_OK) - return err; len = (h->buff[0] << 24) + (h->buff[1] << 16) + (h->buff[2] << 8) + h->buff[3] + 4;
Use standard logging functions instead of hand-crafted ones. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/device_handler/scsi_dh_alua.c | 34 +++++++++++++----------------- 1 file changed, 15 insertions(+), 19 deletions(-)