[02/15] scsi/osd: don't save block errors into req_results
diff mbox

Message ID 20170518131812.22956-3-hch@lst.de
State New
Headers show

Commit Message

Christoph Hellwig May 18, 2017, 1:17 p.m. UTC
We will only have sense data if the command exectured and got a SCSI
result, so this is pointless.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/osd/osd_initiator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bart Van Assche May 24, 2017, 4:04 p.m. UTC | #1
On Thu, 2017-05-18 at 15:17 +0200, Christoph Hellwig wrote:
> We will only have sense data if the command exectured and got a SCSI
> result, so this is pointless.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/osd/osd_initiator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
> index 8a1b94816419..14785177ce7b 100644
> --- a/drivers/scsi/osd/osd_initiator.c
> +++ b/drivers/scsi/osd/osd_initiator.c
> @@ -477,7 +477,7 @@ static void _set_error_resid(struct osd_request *or, struct request *req,
>  			     int error)
>  {
>  	or->async_error = error;
> -	or->req_errors = scsi_req(req)->result ? : error;
> +	or->req_errors = scsi_req(req)->result;
>  	or->sense_len = scsi_req(req)->sense_len;
>  	if (or->sense_len)
>  		memcpy(or->sense, scsi_req(req)->sense, or->sense_len);

Hello Christoph,

Are you sure that that code is not necessary? From osd_initiator.c:

static void _put_request(struct request *rq)
{
	/*
	 * If osd_finalize_request() was called but the request was not
	 * executed through the block layer, then we must release BIOs.
	 * TODO: Keep error code in or->async_error. Need to audit all
	 *       code paths.
	 */
	if (unlikely(rq->bio))
		blk_end_request(rq, -ENOMEM, blk_rq_bytes(rq));
	else
		blk_put_request(rq);
}

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig May 26, 2017, 5:15 a.m. UTC | #2
On Wed, May 24, 2017 at 04:04:40PM +0000, Bart Van Assche wrote:
> Are you sure that that code is not necessary? From osd_initiator.c:
> 
> static void _put_request(struct request *rq)
> {
> 	/*
> 	 * If osd_finalize_request() was called but the request was not
> 	 * executed through the block layer, then we must release BIOs.
> 	 * TODO: Keep error code in or->async_error. Need to audit all
> 	 *       code paths.
> 	 */
> 	if (unlikely(rq->bio))
> 		blk_end_request(rq, -ENOMEM, blk_rq_bytes(rq));
> 	else
> 		blk_put_request(rq);
> }

Which isn't using it at all.  It has a ten year old comment to pass
on some error, but even then ORing two different error types together
would no be very helpful.

> 
> Bart.---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 8a1b94816419..14785177ce7b 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -477,7 +477,7 @@  static void _set_error_resid(struct osd_request *or, struct request *req,
 			     int error)
 {
 	or->async_error = error;
-	or->req_errors = scsi_req(req)->result ? : error;
+	or->req_errors = scsi_req(req)->result;
 	or->sense_len = scsi_req(req)->sense_len;
 	if (or->sense_len)
 		memcpy(or->sense, scsi_req(req)->sense, or->sense_len);