On 2019-01-15 10:23 p.m., Bart Van Assche wrote:
On 1/15/19 6:34 PM, Douglas Gilbert wrote:
On 2019-01-15 7:50 p.m., Bart Van Assche wrote:
+static blk_status_t sd_setup_rw6_cmnd(struct scsi_cmnd *cmd, bool write,
+                      sector_t lba, unsigned int nr_blocks,
+                      unsigned char flags)
+{
+    if (unlikely(flags & 0x8)) {
+        /*
+         * This happens only if this drive failed 10byte rw
+         * command with ILLEGAL_REQUEST during operation and
+         * thus turned off use_10_for_rw.
+         */
+        scmd_printk(KERN_ERR, cmd, "FUA write on READ/WRITE(6) drive\n");
+        return BLK_STS_IOERR;
+    }
+
+    cmd->cmd_len = 6;
+    cmd->cmnd[0] = write ? WRITE_6 : READ_6;
+    cmd->cmnd[1] = (lba >> 16) & 0x1f;
+    cmd->cmnd[2] = (lba >> 8) & 0xff;
+    cmd->cmnd[3] = lba & 0xff;
+    cmd->cmnd[4] = nr_blocks;

Since you have made this a separate function, observing the principle of least
surprise, you may want to return an error if nr_blocks==0 since in that case
the code as it stands will read or write 256 blocks!

Hi Doug,

Next to returning an error if nr_blocks == 0, how about triggering a kernel warning if nr_blocks == 0 is passed to this function? If nr_blocks == 0 there is something wrong. I don't think that any code in the kernel submits REQ_OP_READ or REQ_OP_WRITE requests with length zero.

Yes a WARN is fine, but you want to stop the operation, especially if it
happens to be a WRITE(6). Some other code may re-use this function in
the future.

Reply via email to