All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
To: Boaz Harrosh <bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
Cc: Alan Stern
	<stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>,
	Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>,
	Jens Axboe <jens.axboe-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>,
	Matthew Dharm
	<mdharm-usb-JGfshJpz5UybPZpvUQj5UqxOck334EZe@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [BUG] 2.6.24-git usb reset problems
Date: Tue, 29 Jan 2008 14:17:20 -0500	[thread overview]
Message-ID: <1201634240.3069.33.camel@localhost.localdomain> (raw)
In-Reply-To: <479F774B.60303-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>

On Tue, 2008-01-29 at 20:58 +0200, Boaz Harrosh wrote:
> > Your new code does
> > 
> > int partial; <- stack uninitialised
> > sb_stor_bulk_transfer_sglist(..., &partial, ...);
> > scsi_set_resid(srb, scsi_bufflen(srb) - partial);
> > 
> > If the function doesn't touch partial, as it doesn't in the error legs,
> > resid now gets set with rubbish.
> > 
> > Actually, my code is still wrong .. we have to set it to
> > scsi_bufflen(srb) - scsi_resid(srb) so that it comes back the same if
> > left untouched.
> > 
> >> I have such a device and I get one reset but then every thing works nice.
> >> This is with debug on. I'll try to make it fail.
> > 
> > James
> > 
> > 
> Sorry I still don't see it.
> 
> original code did sb_stor_bulk_transfer_sg(..., &srp->resid, ...)
> 
> but sb_stor_bulk_transfer_sg does the:
>  int partial; <- stack uninitialised
>  sb_stor_bulk_transfer_sglist(..., &partial, ...);
> 
> and then unconditionally sets *residual = length_left;
> I do not see an "error legs" case in sb_stor_bulk_transfer_sg().

This is really programming 101. This:

static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned
int pipe,
		struct scatterlist *sg, int num_sg, unsigned int length,
		unsigned int *act_len)
{
	int result;

	/* don't submit s-g requests during abort/disconnect processing */
	if (us->flags & ABORTING_OR_DISCONNECTING)
		return USB_STOR_XFER_ERROR;

The return USB_STOR_XFER_ERROR; is called an error leg.  It returns
without updating *act_len thus leaving &partial uninitialised.

James

WARNING: multiple messages have this Message-ID (diff)
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Boaz Harrosh <bharrosh@panasas.com>
Cc: Alan Stern <stern@rowland.harvard.edu>, Greg KH <greg@kroah.com>,
	Jens Axboe <jens.axboe@oracle.com>,
	Matthew Dharm <mdharm-usb@one-eyed-alien.net>,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-scsi@vger.kernel.org
Subject: Re: [BUG] 2.6.24-git usb reset problems
Date: Tue, 29 Jan 2008 14:17:20 -0500	[thread overview]
Message-ID: <1201634240.3069.33.camel@localhost.localdomain> (raw)
In-Reply-To: <479F774B.60303@panasas.com>

On Tue, 2008-01-29 at 20:58 +0200, Boaz Harrosh wrote:
> > Your new code does
> > 
> > int partial; <- stack uninitialised
> > sb_stor_bulk_transfer_sglist(..., &partial, ...);
> > scsi_set_resid(srb, scsi_bufflen(srb) - partial);
> > 
> > If the function doesn't touch partial, as it doesn't in the error legs,
> > resid now gets set with rubbish.
> > 
> > Actually, my code is still wrong .. we have to set it to
> > scsi_bufflen(srb) - scsi_resid(srb) so that it comes back the same if
> > left untouched.
> > 
> >> I have such a device and I get one reset but then every thing works nice.
> >> This is with debug on. I'll try to make it fail.
> > 
> > James
> > 
> > 
> Sorry I still don't see it.
> 
> original code did sb_stor_bulk_transfer_sg(..., &srp->resid, ...)
> 
> but sb_stor_bulk_transfer_sg does the:
>  int partial; <- stack uninitialised
>  sb_stor_bulk_transfer_sglist(..., &partial, ...);
> 
> and then unconditionally sets *residual = length_left;
> I do not see an "error legs" case in sb_stor_bulk_transfer_sg().

This is really programming 101. This:

static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned
int pipe,
		struct scatterlist *sg, int num_sg, unsigned int length,
		unsigned int *act_len)
{
	int result;

	/* don't submit s-g requests during abort/disconnect processing */
	if (us->flags & ABORTING_OR_DISCONNECTING)
		return USB_STOR_XFER_ERROR;

The return USB_STOR_XFER_ERROR; is called an error leg.  It returns
without updating *act_len thus leaving &partial uninitialised.

James



  parent reply	other threads:[~2008-01-29 19:17 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-28 20:49 [BUG] 2.6.24-git usb reset problems Jens Axboe
2008-01-28 21:21 ` Greg KH
2008-01-29  7:48   ` Jens Axboe
2008-01-29 12:15   ` Boaz Harrosh
2008-01-29 15:00     ` Matthew Dharm
     [not found]     ` <479F18D3.9030709-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-29 13:54       ` Jens Axboe
2008-01-29 13:54         ` Jens Axboe
     [not found]         ` <20080129135443.GU15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 14:06           ` Boaz Harrosh
2008-01-29 14:06             ` Boaz Harrosh
     [not found]             ` <479F32E9.10609-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-29 14:11               ` Jens Axboe
2008-01-29 14:11                 ` Jens Axboe
     [not found]                 ` <20080129141108.GV15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 14:14                   ` Boaz Harrosh
2008-01-29 14:14                     ` Boaz Harrosh
2008-01-29 14:31                 ` Oliver Neukum
     [not found]                   ` <200801291531.39825.oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
2008-01-29 14:31                     ` Jens Axboe
2008-01-29 14:31                       ` Jens Axboe
2008-01-29 18:39                       ` Jens Axboe
     [not found]                         ` <20080129183910.GI15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 19:09                           ` Boaz Harrosh
2008-01-29 19:09                             ` Boaz Harrosh
2008-01-29 19:10                           ` Matthew Dharm
2008-01-29 19:10                             ` Matthew Dharm
     [not found]                             ` <20080129191024.GO14375-JGfshJpz5UybPZpvUQj5UqxOck334EZe@public.gmane.org>
2008-01-29 19:15                               ` Jens Axboe
2008-01-29 19:15                                 ` Jens Axboe
     [not found]                                 ` <20080129191504.GO15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 19:26                                   ` Jens Axboe
2008-01-29 19:26                                     ` Jens Axboe
2008-01-29 19:37                                 ` Matthew Dharm
2008-01-29 19:33                             ` James Bottomley
2008-01-29 19:35                               ` Jens Axboe
2008-01-29 19:45                                 ` Jens Axboe
     [not found]                                   ` <20080129194543.GR15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 19:58                                     ` Boaz Harrosh
2008-01-29 19:58                                       ` Boaz Harrosh
     [not found]                                       ` <479F856C.4090900-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-29 20:03                                         ` Jens Axboe
2008-01-29 20:03                                           ` Jens Axboe
     [not found]                                           ` <20080129200311.GV15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 20:04                                             ` James Bottomley
2008-01-29 20:04                                               ` James Bottomley
2008-01-29 20:06                                               ` Jens Axboe
     [not found]                                                 ` <20080129200651.GW15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 20:24                                                   ` James Bottomley
2008-01-29 20:24                                                     ` James Bottomley
2008-01-29 20:53                                                     ` Boaz Harrosh
2008-01-29 20:09                                         ` Boaz Harrosh
2008-01-29 20:09                                           ` Boaz Harrosh
     [not found]                                           ` <479F880F.20709-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-29 20:13                                             ` Jens Axboe
2008-01-29 20:13                                               ` Jens Axboe
     [not found]                                               ` <20080129201328.GZ15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-29 20:26                                                 ` Boaz Harrosh
2008-01-29 20:26                                                   ` Boaz Harrosh
2008-01-30 10:27                                     ` Geert Uytterhoeven
2008-01-30 10:27                                       ` Geert Uytterhoeven
     [not found]                                       ` <Pine.LNX.4.64.0801301125350.26562-DVqXPGhgXSn9uFGNBm7GzQ@public.gmane.org>
2008-01-30 10:38                                         ` Jens Axboe
2008-01-30 10:38                                           ` Jens Axboe
2008-01-30 14:38                                           ` James Bottomley
2008-01-30 18:06                                             ` Jens Axboe
     [not found]                                               ` <20080130180613.GU15220-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2008-01-30 19:07                                                 ` Jens Axboe
2008-01-30 19:07                                                   ` Jens Axboe
2008-01-29 15:50                   ` Boaz Harrosh
     [not found]                     ` <479F4B4B.2020000-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-29 17:42                       ` Oliver Neukum
2008-01-29 17:42                         ` Oliver Neukum
2008-01-29 14:13             ` Boaz Harrosh
2008-01-29 15:36       ` Alan Stern
2008-01-29 15:36         ` Alan Stern
     [not found]         ` <Pine.LNX.4.44L0.0801291032210.4229-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-01-29 15:54           ` Boaz Harrosh
2008-01-29 15:54             ` Boaz Harrosh
2008-01-29 16:34         ` James Bottomley
2008-01-29 18:27           ` Boaz Harrosh
     [not found]             ` <479F7009.3020306-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-29 18:48               ` James Bottomley
2008-01-29 18:48                 ` James Bottomley
2008-01-29 18:58                 ` Boaz Harrosh
     [not found]                   ` <479F774B.60303-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-01-29 19:17                     ` James Bottomley [this message]
2008-01-29 19:17                       ` James Bottomley
     [not found]                       ` <1201634240.3069.33.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-01-29 19:28                         ` Boaz Harrosh
2008-01-29 19:28                           ` Boaz Harrosh

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=1201634240.3069.33.camel@localhost.localdomain \
    --to=james.bottomley-d9phhud1jfjcxq6kfmz53/egyhegw8jk@public.gmane.org \
    --cc=bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org \
    --cc=greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org \
    --cc=jens.axboe-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mdharm-usb-JGfshJpz5UybPZpvUQj5UqxOck334EZe@public.gmane.org \
    --cc=stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.