public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Douglas Gilbert <dgilbert@interlog.com>
To: Tony Battersby <tonyb@cybernetics.com>,
	linux-scsi@vger.kernel.org,
	"James E.J. Bottomley" <JBottomley@parallels.com>,
	Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH][SCSI] scsi-mq: fix requests that use a separate CDB buffer
Date: Fri, 22 Aug 2014 20:52:47 -0400	[thread overview]
Message-ID: <53F7E5DF.3090809@interlog.com> (raw)
In-Reply-To: <53F79FC3.6040907@cybernetics.com>

On 14-08-22 03:53 PM, Tony Battersby wrote:
> This patch fixes code such as the following with scsi-mq enabled:
>
>      rq = blk_get_request(...);
>      blk_rq_set_block_pc(rq);
>
>      rq->cmd = my_cmd_buffer; /* separate CDB buffer */
>
>      blk_execute_rq_nowait(...);
>
> Code like this appears in e.g. sg_start_req() in drivers/scsi/sg.c (for
> large CDBs only).  Without this patch, scsi_mq_prep_fn() will set
> rq->cmd back to rq->__cmd, causing the wrong CDB to be sent to the device.

Still looking at this one. 'sg_write_same --32' is my
tool of choice. The target is scsi_debug which needs
dif=2 (because mkp read the draft and only allows
WRITE SAME(32) when the protection_level=2). Turned
on command tracing and this is what I saw:

sd 7:0:0:0: scsi_debug: cmd 9e 10 00 00 00 00 00 00 00 00 00 00 00 20 00 00
sd 7:0:0:0: scsi_debug: cmd 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 08 
24 6b d3 00 88 ff ff 20 00 c7 e8 00 00 00 00

So the WS(32) command did get through, it is the second
command that arrived on its tail that is a worry. So yes,
IMO it is broken.


Now the >16 byte cdb length in the sg driver
introduced in lk 3.17-rc1 was copied directly
from the bsg driver which has had that capability
for some time. Since your patch touches two files
in drivers/block I'm wondering the bsg driver's
 >16 byte cdb length capability is broken in
lk 3.16 ?

Doug Gilbert




  reply	other threads:[~2014-08-23  0:52 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-22 19:53 [PATCH][SCSI] scsi-mq: fix requests that use a separate CDB buffer Tony Battersby
2014-08-23  0:52 ` Douglas Gilbert [this message]
2014-08-23  1:23   ` Douglas Gilbert
2014-08-23 19:09 ` Douglas Gilbert
2014-08-25 14:04   ` Tony Battersby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53F7E5DF.3090809@interlog.com \
    --to=dgilbert@interlog.com \
    --cc=JBottomley@parallels.com \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=tonyb@cybernetics.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox