2014-06-06 11:46:38

by Matias Bjørling

Subject: [PATCH] Small correction to blk_rq_set_block_pc

Hi Jens,

The blk_rq_set_block_pc() overwrites rq->__data_len after its being set in
blk_rq_map_kern().

It leads to some interesting error messages when booted with SCSI device.

Matias Bjørling (1):
block: blk_rq_set_block_pc overwrites __data_len

drivers/block/pktcdvd.c | 2 +-
drivers/cdrom/cdrom.c | 2 +-
drivers/scsi/device_handler/scsi_dh_alua.c | 2 +-
drivers/scsi/device_handler/scsi_dh_rdac.c | 2 +-
drivers/scsi/scsi_lib.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)

--
1.9.1

2014-06-06 11:46:51

by Matias Bjørling

Subject: [PATCH] block: blk_rq_set_block_pc overwrites __data_len

Drivers that issue REQ_TYPE_BLOCK_PC command, calls blk_rq_map_kern() before
the command is intialized to REQ_TYPE_BLOCK_PC. As blk_rq_map_kern() increases
rq->__data_len, and blk_rq_set_block_pc sets it to zero. Results in a zero
request being sent to hw.

Signed-off-by: Matias Bjørling <[email protected]>
---
drivers/block/pktcdvd.c | 2 +-
drivers/cdrom/cdrom.c | 2 +-
drivers/scsi/device_handler/scsi_dh_alua.c | 2 +-
drivers/scsi/device_handler/scsi_dh_rdac.c | 2 +-
drivers/scsi/scsi_lib.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 0ce8a3e..758ac44 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -704,6 +704,7 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *

rq = blk_get_request(q, (cgc->data_direction == CGC_DATA_WRITE) ?
WRITE : READ, __GFP_WAIT);
+ blk_rq_set_block_pc(rq);

if (cgc->buflen) {
ret = blk_rq_map_kern(q, rq, cgc->buffer, cgc->buflen,
@@ -712,7 +713,6 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
goto out;
}

- blk_rq_set_block_pc(rq);
rq->cmd_len = COMMAND_SIZE(cgc->cmd[0]);
memcpy(rq->cmd, cgc->cmd, CDROM_PACKET_SIZE);

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 4d10024..0f40c95 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -2184,6 +2184,7 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
ret = -ENOMEM;
break;
}
+ blk_rq_set_block_pc(rq);

ret = blk_rq_map_user(q, rq, NULL, ubuf, len, GFP_KERNEL);
if (ret) {
@@ -2191,7 +2192,6 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
break;
}

- blk_rq_set_block_pc(rq);
rq->cmd[0] = GPCMD_READ_CD;
rq->cmd[1] = 1 << 2;
rq->cmd[2] = (lba >> 24) & 0xff;
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 3dcd93f..7bcf67e 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -120,6 +120,7 @@ static struct request *get_alua_req(struct scsi_device *sdev,
"%s: blk_get_request failed\n", __func__);
return NULL;
}
+ blk_rq_set_block_pc(rq);

if (buflen && blk_rq_map_kern(q, rq, buffer, buflen, GFP_NOIO)) {
blk_put_request(rq);
@@ -128,7 +129,6 @@ static struct request *get_alua_req(struct scsi_device *sdev,
return NULL;
}

- blk_rq_set_block_pc(rq);
rq->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
REQ_FAILFAST_DRIVER;
rq->retries = ALUA_FAILOVER_RETRIES;
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index ea35f0a..826069d 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -279,6 +279,7 @@ static struct request *get_rdac_req(struct scsi_device *sdev,
"get_rdac_req: blk_get_request failed.\n");
return NULL;
}
+ blk_rq_set_block_pc(rq);

if (buflen && blk_rq_map_kern(q, rq, buffer, buflen, GFP_NOIO)) {
blk_put_request(rq);
@@ -287,7 +288,6 @@ static struct request *get_rdac_req(struct scsi_device *sdev,
return NULL;
}

- blk_rq_set_block_pc(rq);
rq->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
REQ_FAILFAST_DRIVER;
rq->retries = RDAC_RETRIES;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6660e43..c3c1697 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -195,12 +195,12 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
req = blk_get_request(sdev->request_queue, write, __GFP_WAIT);
if (!req)
return ret;
+ blk_rq_set_block_pc(req);

if (bufflen && blk_rq_map_kern(sdev->request_queue, req,
buffer, bufflen, __GFP_WAIT))
goto out;

- blk_rq_set_block_pc(req);
req->cmd_len = COMMAND_SIZE(cmd[0]);
memcpy(req->cmd, cmd, req->cmd_len);
req->sense = sense;
--
1.9.1

2014-06-06 14:20:07

by Jens Axboe

Subject: Re: [PATCH] Small correction to blk_rq_set_block_pc

On 2014-06-06 05:46, Matias Bjørling wrote:
> Hi Jens,
>
> The blk_rq_set_block_pc() overwrites rq->__data_len after its being set in
> blk_rq_map_kern().
>
> It leads to some interesting error messages when booted with SCSI device.

Christoph beat you to it, I folded it in with the previous one.

--
Jens Axboe