On Tue, 20 Dec 2011, Sebastian Andrzej Siewior wrote:
> The status/sense URB is allocated on per-command basis. A read/write
> looks the following way on a stream-less connection:
>
> - send cmd tag X, queue status
> - receive status, oh it is a read for tag X. queue status & read
> - receive read
> - receive status, oh I'm done for tag X. Cool call complete and free
> status urb.
>
> This block repeats itself 1:1 for further commands and looks great so
> far. Lets take a look now what happens if we do allow multiple commands:
>
> - send cmd tag X, queue statusX (belongs to the command with the X tag)
> - send cmd tag Y, queue statusY (belongs to the command with the Y tag)
> - receive statusX, oh it is a read for tag X. queue statusX & a read
> - receive read
> - receive statusY, oh I'm done for tag X. Cool call complete and free statusY.
> - receive statusX, oh it is a read for tag Y. queue statusY & before we
> queue the read the the following message can be observed:
> |sd 0:0:0:0: [sda] sense urb submission failure
> followed by a second attempt with the same result.
>
> In order to address this problem there only one status URB for each scsi
> host (as suggested by Matthew) in case we don't have stream support.
> This URB is requeued until the device removed. Nothing changes on stream
> based endpoints.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> Okay. That is the third patch of my UAS series. The only issue I'm aware
> now is the fact that uas does not work on hcds which do not support sgs.
> One bug at a time.....
>
> drivers/usb/storage/uas.c | 71 ++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 61 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index ecf2070..e3b936e 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -99,6 +99,8 @@ struct uas_dev_info {
> unsigned use_streams:1;
> unsigned uas_sense_old:1;
> struct scsi_cmnd *cmnd;
> + /* only used if no stream support is available */
> + struct urb *status_urb;
> };
>
> enum {
> @@ -117,6 +119,7 @@ struct uas_cmd_info {
> unsigned int state;
> unsigned int stream;
> struct urb *cmd_urb;
> + /* only used if stream support is available */
> struct urb *status_urb;
> struct urb *data_in_urb;
> struct urb *data_out_urb;
Very minor stylistic point... It's not clear whether the comments
apply to the items preceding them or the items following them. You
might want to put each comment on the same line with its respective
item.
Alan Stern
P.S.: Another very minor point, this time grammatical. (In fact this
is an _extremely_ common mistake -- virtually everybody does it, native
English speakers included.)
The word "only" generally modifies the term following it. So when you
write "X is only used if Y", the grammatical meaning is: "If Y is true
then X is used but nothing else happens to X -- for example, X isn't
changed or deleted". (As an illustration of this point, consider the
sentence "The disk is only read, not written, if the read-protect
flag is set".)
Instead you should write "X is used only if Y", which means what you
want: "If Y isn't true then X isn't used". Or in this case: "used only
if stream support is available".
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
