[BUG] 2.6.24-git usb reset problems - Kernel

This is a discussion on [BUG] 2.6.24-git usb reset problems - Kernel ; On Tue, Jan 29 2008 at 20:48 +0200, James Bottomley wrote: > On Tue, 2008-01-29 at 20:27 +0200, Boaz Harrosh wrote: >> On Tue, Jan 29 2008 at 18:34 +0200, James Bottomley wrote: >>> On Tue, 2008-01-29 at 10:36 -0500, ...

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast
Results 21 to 40 of 45

Thread: [BUG] 2.6.24-git usb reset problems

  1. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008 at 20:48 +0200, James Bottomley wrote:
    > On Tue, 2008-01-29 at 20:27 +0200, Boaz Harrosh wrote:
    >> On Tue, Jan 29 2008 at 18:34 +0200, James Bottomley wrote:
    >>> On Tue, 2008-01-29 at 10:36 -0500, Alan Stern wrote:
    >>>> On Tue, 29 Jan 2008, Boaz Harrosh wrote:
    >>>>
    >>>>> --- a/drivers/usb/storage/transport.c
    >>>>> +++ b/drivers/usb/storage/transport.c
    >>>>> @@ -462,18 +462,24 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe,
    >>>>> * Common used function. Transfer a complete command
    >>>>> * via usb_stor_bulk_transfer_sglist() above. Set cmnd resid
    >>>>> */
    >>>>> -int usb_stor_bulk_srb(struct us_data* us, unsigned int pipe,
    >>>>> - struct scsi_cmnd* srb)
    >>>>> +int usb_stor_bulk_srb_length(struct us_data* us, unsigned int pipe,
    >>>>> + struct scsi_cmnd* srb, unsigned length)
    >>>>> {
    >>>>> unsigned int partial;
    >>>>> int result = usb_stor_bulk_transfer_sglist(us, pipe, scsi_sglist(srb),
    >>>>> - scsi_sg_count(srb), scsi_bufflen(srb),
    >>>>> + scsi_sg_count(srb), length,
    >>>>> &partial);
    >>>>>
    >>>>> scsi_set_resid(srb, scsi_bufflen(srb) - partial);
    >>>>> return result;
    >>>>> }
    >>>>>
    >>>>> +int usb_stor_bulk_srb(struct us_data* us, unsigned int pipe,
    >>>>> + struct scsi_cmnd* srb)
    >>>>> +{
    >>>>> + return usb_stor_bulk_srb_length(us, pipe, srb, scsi_bufflen(srb));
    >>>>> +}
    >>>>> +
    >>>> I don't like this patch very much. Why add another layer of
    >>>> indirection when the two subroutines do hardly any work? Leave
    >>>> usb_stor_bulk_srb() the way it was, and add usb_stor_bulk_srb_length()
    >>>> as a separate routine that simply calls usb_stor_bulk_transfer_sglist()
    >>>> and scsi_set_resid().
    >>>>
    >>>> BTW, the standard coding style calls for a blank line after the list of
    >>>> local variables at the start of a function or block.
    >>> There's another bug in the transport.c conversion in that the residuals
    >>> are updated with bogus data in several error cases, since
    >>> usb_stor_bulk_transfer_sglist() only sets the actual length if the urb
    >>> is actually sent.
    >>>
    >>> I'm not sure if this is is the solution to the problem at hand, but it
    >>> definitely fixes another bug in the code.
    >>>
    >>> James
    >>>
    >>> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
    >>> index d9f4912..bab0858 100644
    >>> --- a/drivers/usb/storage/transport.c
    >>> +++ b/drivers/usb/storage/transport.c
    >>> @@ -465,7 +465,7 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe,
    >>> int usb_stor_bulk_srb(struct us_data* us, unsigned int pipe,
    >>> struct scsi_cmnd* srb)
    >>> {
    >>> - unsigned int partial;
    >>> + unsigned int partial = scsi_get_resid(srb);
    >>> int result = usb_stor_bulk_transfer_sglist(us, pipe, scsi_sglist(srb),
    >>> scsi_sg_count(srb), scsi_bufflen(srb),
    >>> &partial);
    >>>
    >>>
    >>> -

    >> But then this is weird because it is not what usb_stor_bulk_transfer_sg() is doing
    >> which was the one called before.

    >
    > Um, yes it was. The original code did this
    >
    > sb_stor_bulk_transfer_sg(..., &srp->resid, ...)
    >
    > Which was at liberty not to touch resid, which it chose not to do in the
    > error legs.
    >
    > 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().

    I have just cut and pasted the !use_sg from sb_stor_bulk_transfer_sg names
    and all

    Boaz

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008 at 20:39 +0200, Jens Axboe wrote:
    > On Tue, Jan 29 2008, Jens Axboe wrote:
    >> On Tue, Jan 29 2008, Oliver Neukum wrote:
    >>> Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    >>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    >>>>> On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    >>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    >>>>>>> Greg KH wrote:
    >>>
    >>>>>> No difference, still just a lot of resets.
    >>>>>>
    >>>>> Where you able to figure out which usb storage transport is used?
    >>>>>
    >>>>> in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    >>>>> functions. I'm not sure if these get stored in sysfs perhaps. This will
    >>>>> pinpoint better where to look. Let me research a bit.
    >>>> Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    >>>> transport is 'Bulk'
    >>> You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    >>> That should tell the reason for the resets.

    >> Sure, I'll do that. Will post the results tonight.

    >
    > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > in the device, waited 10 seconds or so and pulled it out. These are the
    > messages.
    >
    > It all looks good until the MODE_SENSE command, where it only transfers
    > 4 of 192 bytes.
    >


    > usb-storage: Command MODE_SENSE (6 bytes)
    > usb-storage: 1a 00 3f 00 c0 00
    > usb-storage: Bulk Command S 0x43425355 T 0x4 L 192 F 128 Trg 0 LUN 0 CL 6
    > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > usb-storage: Status code 0; transferred 31/31
    > usb-storage: -- transfer complete
    > usb-storage: Bulk command transfer result=0
    > usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
    > usb-storage: Status code -121; transferred 4/192
    > usb-storage: -- short read transfer
    > usb-storage: Bulk data transfer result 0x1
    > usb-storage: Attempting to get CSW...



    I get something similar but better:
    usb-storage: Command MODE_SENSE (6 bytes)
    usb-storage: 1a 00 3f 00 c0 00
    usb-storage: Bulk Command S 0x43425355 T 0x6 L 192 F 128 Trg 0 LUN 0 CL 6
    usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    usb-storage: Status code 0; transferred 31/31
    usb-storage: -- transfer complete
    usb-storage: Bulk command transfer result=0
    usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
    usb-storage: Status code -121; transferred 36/192
    usb-storage: -- short read transfer
    usb-storage: Bulk data transfer result 0x1
    usb-storage: Attempting to get CSW...

    So I get 36 bytes, then code goes on into one reset, and every thing is then
    fine.

    Could you put us out of our mesery and revert that patch:
    [SCSI] usb: transport - convert to accessors and !use_sg code path removal
    (6d416e6173394defda5933e419e805b696681b7e)

    to make sure this is it. I hate to do this to you, but I cannot reproduce the
    failure down here. If it works please send a log with the debugs on perhaps
    we can compare.

    You will need to configure out the CONFIG_USB_STORAG_* they will not compile
    you should have only have CONFIG_USB_STORAGE & CONFIG_USB_STORAGE_DEBUG. it should
    support your HW.


    Thanks Jens
    Boaz


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [BUG] 2.6.24-git usb reset problems

    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


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > >> Greg KH wrote:
    > > >
    > > > > > > No difference, still just a lot of resets.
    > > > > > >
    > > > > > Where you able to figure out which usb storage transport is used?
    > > > > >
    > > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > > > functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > > > > pinpoint better where to look. Let me research a bit.
    > > > >
    > > > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > > transport is 'Bulk'
    > > >
    > > > You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > That should tell the reason for the resets.

    > >
    > > Sure, I'll do that. Will post the results tonight.

    >
    > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > in the device, waited 10 seconds or so and pulled it out. These are the
    > messages.
    >
    > It all looks good until the MODE_SENSE command, where it only transfers
    > 4 of 192 bytes.


    No, that's not the problem. This is the problem:

    > usb-storage: *** thread awakened.
    > usb-storage: Command TEST_UNIT_READY (6 bytes)
    > usb-storage: 00 00 00 00 00 00
    > usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > usb-storage: Status code 0; transferred 31/31
    > usb-storage: -- transfer complete
    > usb-storage: Bulk command transfer result=0
    > usb-storage: Attempting to get CSW...
    > usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > usb-storage: Status code 0; transferred 13/13
    > usb-storage: -- transfer complete
    > usb-storage: Bulk status result = 0
    > usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > usb-storage: -- transport indicates command failure
    > usb-storage: Issuing auto-REQUEST_SENSE
    > usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > usb-storage: Status code 0; transferred 31/31
    > usb-storage: -- transfer complete
    > usb-storage: Bulk command transfer result=0
    > usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > usb-storage: usb_sg_init returned -22
    > usb-storage: Bulk data transfer result 0x4
    > usb-storage: -- auto-sense failure
    > usb-storage: storage_pre_reset
    > ehci_hcd 0000:00:1d.7: port 1 high speed
    > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > ehci_hcd 0000:00:1d.7: port 1 high speed
    > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > usb-storage: storage_post_reset
    > usb-storage: usb_reset_composite_device returns 0
    > usb-storage: scsi cmd done, result=0x70000
    > usb-storage: *** thread sleeping.


    For some reason, usb_sg_init is boned during auto-sense.

    Matt

    --
    Matthew Dharm Home: mdharm-usb@one-eyed-alien.net
    Maintainer, Linux USB Mass Storage Driver

    It was a new hope.
    -- Dust Puppy
    User Friendly, 12/25/1998

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.7 (GNU/Linux)

    iD8DBQFHn3ogHL9iwnUZqnkRAtE7AJ9r7JsQ+hvBOO7u5sfJA6 Pu8SlmVwCePHWe
    mguggEvYLuxxVzU1AocRqec=
    =LBk4
    -----END PGP SIGNATURE-----


  5. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008, Matthew Dharm wrote:
    > On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > > Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > >> Greg KH wrote:
    > > > >
    > > > > > > > No difference, still just a lot of resets.
    > > > > > > >
    > > > > > > Where you able to figure out which usb storage transport is used?
    > > > > > >
    > > > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > > > > functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > > > > > pinpoint better where to look. Let me research a bit.
    > > > > >
    > > > > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > > > transport is 'Bulk'
    > > > >
    > > > > You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > > That should tell the reason for the resets.
    > > >
    > > > Sure, I'll do that. Will post the results tonight.

    > >
    > > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > in the device, waited 10 seconds or so and pulled it out. These are the
    > > messages.
    > >
    > > It all looks good until the MODE_SENSE command, where it only transfers
    > > 4 of 192 bytes.

    >
    > No, that's not the problem. This is the problem:


    It's where the problem starts, otherwise there would not be a need to
    sense :-)

    > > usb-storage: *** thread awakened.
    > > usb-storage: Command TEST_UNIT_READY (6 bytes)
    > > usb-storage: 00 00 00 00 00 00
    > > usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > usb-storage: Status code 0; transferred 31/31
    > > usb-storage: -- transfer complete
    > > usb-storage: Bulk command transfer result=0
    > > usb-storage: Attempting to get CSW...
    > > usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > > usb-storage: Status code 0; transferred 13/13
    > > usb-storage: -- transfer complete
    > > usb-storage: Bulk status result = 0
    > > usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > > usb-storage: -- transport indicates command failure
    > > usb-storage: Issuing auto-REQUEST_SENSE
    > > usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > usb-storage: Status code 0; transferred 31/31
    > > usb-storage: -- transfer complete
    > > usb-storage: Bulk command transfer result=0
    > > usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > > usb-storage: usb_sg_init returned -22
    > > usb-storage: Bulk data transfer result 0x4
    > > usb-storage: -- auto-sense failure
    > > usb-storage: storage_pre_reset
    > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > usb-storage: storage_post_reset
    > > usb-storage: usb_reset_composite_device returns 0
    > > usb-storage: scsi cmd done, result=0x70000
    > > usb-storage: *** thread sleeping.

    >
    > For some reason, usb_sg_init is boned during auto-sense.


    My guess would be that sg == NULL, hence the -EINVAL.

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008, Jens Axboe wrote:
    > On Tue, Jan 29 2008, Matthew Dharm wrote:
    > > On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > > On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > > > Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > >> Greg KH wrote:
    > > > > >
    > > > > > > > > No difference, still just a lot of resets.
    > > > > > > > >
    > > > > > > > Where you able to figure out which usb storage transport is used?
    > > > > > > >
    > > > > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > > > > > functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > > > > > > pinpoint better where to look. Let me research a bit.
    > > > > > >
    > > > > > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > > > > transport is 'Bulk'
    > > > > >
    > > > > > You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > > > That should tell the reason for the resets.
    > > > >
    > > > > Sure, I'll do that. Will post the results tonight.
    > > >
    > > > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > > in the device, waited 10 seconds or so and pulled it out. These are the
    > > > messages.
    > > >
    > > > It all looks good until the MODE_SENSE command, where it only transfers
    > > > 4 of 192 bytes.

    > >
    > > No, that's not the problem. This is the problem:

    >
    > It's where the problem starts, otherwise there would not be a need to
    > sense :-)
    >
    > > > usb-storage: *** thread awakened.
    > > > usb-storage: Command TEST_UNIT_READY (6 bytes)
    > > > usb-storage: 00 00 00 00 00 00
    > > > usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > usb-storage: Status code 0; transferred 31/31
    > > > usb-storage: -- transfer complete
    > > > usb-storage: Bulk command transfer result=0
    > > > usb-storage: Attempting to get CSW...
    > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > > > usb-storage: Status code 0; transferred 13/13
    > > > usb-storage: -- transfer complete
    > > > usb-storage: Bulk status result = 0
    > > > usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > > > usb-storage: -- transport indicates command failure
    > > > usb-storage: Issuing auto-REQUEST_SENSE
    > > > usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > usb-storage: Status code 0; transferred 31/31
    > > > usb-storage: -- transfer complete
    > > > usb-storage: Bulk command transfer result=0
    > > > usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > > > usb-storage: usb_sg_init returned -22
    > > > usb-storage: Bulk data transfer result 0x4
    > > > usb-storage: -- auto-sense failure
    > > > usb-storage: storage_pre_reset
    > > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > usb-storage: storage_post_reset
    > > > usb-storage: usb_reset_composite_device returns 0
    > > > usb-storage: scsi cmd done, result=0x70000
    > > > usb-storage: *** thread sleeping.

    > >
    > > For some reason, usb_sg_init is boned during auto-sense.

    >
    > My guess would be that sg == NULL, hence the -EINVAL.


    Yep, trace below:

    WARNING: at drivers/usb/storage/transport.c:426
    usb_stor_bulk_transfer_sglist()
    Pid: 12536, comm: usb-storage Not tainted 2.6.24 #74
    [<7810541a>] show_trace_log_lvl+0x1a/0x30
    [<78105e82>] show_trace+0x12/0x20
    [<7810689c>] dump_stack+0x6c/0x80
    [] usb_stor_bulk_transfer_sglist+0x14e/0x160 [usb_storage]
    [] usb_stor_bulk_srb_length+0x30/0x50 [usb_storage]
    [] usb_stor_bulk_srb+0x12/0x20 [usb_storage]
    [] usb_stor_Bulk_transport+0x190/0x3d0 [usb_storage]
    [] usb_stor_invoke_transport+0x1b6/0x320 [usb_storage]
    [] usb_stor_transparent_scsi_command+0x8/0x10 [usb_storage]
    [] usb_stor_control_thread+0x143/0x2c0 [usb_storage]
    [<7813bc02>] kthread+0x42/0x70
    [<78104fab>] kernel_thread_helper+0x7/0x1c
    =======================
    usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries, sg
    00000000
    usb-storage: usb_sg_init returned -22

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, 2008-01-29 at 11:10 -0800, Matthew Dharm wrote:
    > On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > > Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > >> Greg KH wrote:
    > > > >
    > > > > > > > No difference, still just a lot of resets.
    > > > > > > >
    > > > > > > Where you able to figure out which usb storage transport is used?
    > > > > > >
    > > > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > > > > functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > > > > > pinpoint better where to look. Let me research a bit.
    > > > > >
    > > > > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > > > transport is 'Bulk'
    > > > >
    > > > > You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > > That should tell the reason for the resets.
    > > >
    > > > Sure, I'll do that. Will post the results tonight.

    > >
    > > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > in the device, waited 10 seconds or so and pulled it out. These are the
    > > messages.
    > >
    > > It all looks good until the MODE_SENSE command, where it only transfers
    > > 4 of 192 bytes.

    >
    > No, that's not the problem. This is the problem:
    >
    > > usb-storage: *** thread awakened.
    > > usb-storage: Command TEST_UNIT_READY (6 bytes)
    > > usb-storage: 00 00 00 00 00 00
    > > usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > usb-storage: Status code 0; transferred 31/31
    > > usb-storage: -- transfer complete
    > > usb-storage: Bulk command transfer result=0
    > > usb-storage: Attempting to get CSW...
    > > usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > > usb-storage: Status code 0; transferred 13/13
    > > usb-storage: -- transfer complete
    > > usb-storage: Bulk status result = 0
    > > usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > > usb-storage: -- transport indicates command failure
    > > usb-storage: Issuing auto-REQUEST_SENSE
    > > usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > usb-storage: Status code 0; transferred 31/31
    > > usb-storage: -- transfer complete
    > > usb-storage: Bulk command transfer result=0
    > > usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > > usb-storage: usb_sg_init returned -22
    > > usb-storage: Bulk data transfer result 0x4
    > > usb-storage: -- auto-sense failure
    > > usb-storage: storage_pre_reset
    > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > usb-storage: storage_post_reset
    > > usb-storage: usb_reset_composite_device returns 0
    > > usb-storage: scsi cmd done, result=0x70000
    > > usb-storage: *** thread sleeping.

    >
    > For some reason, usb_sg_init is boned during auto-sense.


    OK, that's implicating the scsi_eh_prep_cmnd() in the auto sense
    code ... that was also an update in 2.6.24

    James


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008 at 21:17 +0200, James Bottomley wrote:
    > 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
    >

    Yes you are right this is a bug, but it is a bug that was there before.
    perhaps the stack is just different now then what it used to be.

    Jens could you please try that:

    diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
    index d9f4912..b18a5e6 100644
    --- a/drivers/usb/storage/transport.c
    +++ b/drivers/usb/storage/transport.c
    @@ -465,7 +465,7 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe,
    int usb_stor_bulk_srb(struct us_data* us, unsigned int pipe,
    struct scsi_cmnd* srb)
    {
    - unsigned int partial;
    + unsigned int partial = 0;
    int result = usb_stor_bulk_transfer_sglist(us, pipe, scsi_sglist(srb),
    scsi_sg_count(srb), scsi_bufflen(srb),
    &partial);

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008, James Bottomley wrote:
    > On Tue, 2008-01-29 at 11:10 -0800, Matthew Dharm wrote:
    > > On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > > On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > > > Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > >> Greg KH wrote:
    > > > > >
    > > > > > > > > No difference, still just a lot of resets.
    > > > > > > > >
    > > > > > > > Where you able to figure out which usb storage transport is used?
    > > > > > > >
    > > > > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > > > > > functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > > > > > > pinpoint better where to look. Let me research a bit.
    > > > > > >
    > > > > > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > > > > transport is 'Bulk'
    > > > > >
    > > > > > You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > > > That should tell the reason for the resets.
    > > > >
    > > > > Sure, I'll do that. Will post the results tonight.
    > > >
    > > > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > > in the device, waited 10 seconds or so and pulled it out. These are the
    > > > messages.
    > > >
    > > > It all looks good until the MODE_SENSE command, where it only transfers
    > > > 4 of 192 bytes.

    > >
    > > No, that's not the problem. This is the problem:
    > >
    > > > usb-storage: *** thread awakened.
    > > > usb-storage: Command TEST_UNIT_READY (6 bytes)
    > > > usb-storage: 00 00 00 00 00 00
    > > > usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > usb-storage: Status code 0; transferred 31/31
    > > > usb-storage: -- transfer complete
    > > > usb-storage: Bulk command transfer result=0
    > > > usb-storage: Attempting to get CSW...
    > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > > > usb-storage: Status code 0; transferred 13/13
    > > > usb-storage: -- transfer complete
    > > > usb-storage: Bulk status result = 0
    > > > usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > > > usb-storage: -- transport indicates command failure
    > > > usb-storage: Issuing auto-REQUEST_SENSE
    > > > usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > usb-storage: Status code 0; transferred 31/31
    > > > usb-storage: -- transfer complete
    > > > usb-storage: Bulk command transfer result=0
    > > > usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > > > usb-storage: usb_sg_init returned -22
    > > > usb-storage: Bulk data transfer result 0x4
    > > > usb-storage: -- auto-sense failure
    > > > usb-storage: storage_pre_reset
    > > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > usb-storage: storage_post_reset
    > > > usb-storage: usb_reset_composite_device returns 0
    > > > usb-storage: scsi cmd done, result=0x70000
    > > > usb-storage: *** thread sleeping.

    > >
    > > For some reason, usb_sg_init is boned during auto-sense.

    >
    > OK, that's implicating the scsi_eh_prep_cmnd() in the auto sense
    > code ... that was also an update in 2.6.24


    yeah, already found the bug - it's assuming ->request_buffer holds the
    sglist, oops. Preparing a fix.

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29, 2008 at 08:15:04PM +0100, Jens Axboe wrote:
    > On Tue, Jan 29 2008, Matthew Dharm wrote:
    > > On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > > On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > > > Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > >> Greg KH wrote:
    > > > > >
    > > > > > > > > No difference, still just a lot of resets.
    > > > > > > > >
    > > > > > > > Where you able to figure out which usb storage transport is used?
    > > > > > > >
    > > > > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > > > > > functions. I'm not sure if these get stored in sysfs perhaps.This will
    > > > > > > > pinpoint better where to look. Let me research a bit.
    > > > > > >
    > > > > > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > > > > transport is 'Bulk'
    > > > > >
    > > > > > You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > > > That should tell the reason for the resets.
    > > > >
    > > > > Sure, I'll do that. Will post the results tonight.
    > > >
    > > > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > > in the device, waited 10 seconds or so and pulled it out. These are the
    > > > messages.
    > > >
    > > > It all looks good until the MODE_SENSE command, where it only transfers
    > > > 4 of 192 bytes.

    > >
    > > No, that's not the problem. This is the problem:

    >
    > It's where the problem starts, otherwise there would not be a need to
    > sense :-)


    MODE_SENSE has nothing to do with it. A short MODE_SENSE response is
    perfectly valid, and the log shows it was handled correctly and subsequent
    commands worked just fine. It's not until the TUR fails that we get the
    problem.

    Matt

    --
    Matthew Dharm Home: mdharm-usb@one-eyed-alien.net
    Maintainer, Linux USB Mass Storage Driver

    P: Nine more messages in admin.policy.
    M: I know, I'm typing as fast as I can!
    -- Pitr and Mike
    User Friendly, 11/27/97

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.7 (GNU/Linux)

    iD8DBQFHn4BpHL9iwnUZqnkRAjFhAJ0Zpe7ze1JAMgU+S5cNm3 PM0eDCbACfWm4d
    MV0Ppy6gr3ZNwXlv53gIkOg=
    =CX+K
    -----END PGP SIGNATURE-----


  11. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008, Jens Axboe wrote:
    > On Tue, Jan 29 2008, James Bottomley wrote:
    > > On Tue, 2008-01-29 at 11:10 -0800, Matthew Dharm wrote:
    > > > On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > > > On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > > > > Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > > > > > > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > > > > > > >> Greg KH wrote:
    > > > > > >
    > > > > > > > > > No difference, still just a lot of resets.
    > > > > > > > > >
    > > > > > > > > Where you able to figure out which usb storage transport is used?
    > > > > > > > >
    > > > > > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > > > > > > functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > > > > > > > pinpoint better where to look. Let me research a bit.
    > > > > > > >
    > > > > > > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > > > > > transport is 'Bulk'
    > > > > > >
    > > > > > > You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > > > > That should tell the reason for the resets.
    > > > > >
    > > > > > Sure, I'll do that. Will post the results tonight.
    > > > >
    > > > > OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > > > in the device, waited 10 seconds or so and pulled it out. These are the
    > > > > messages.
    > > > >
    > > > > It all looks good until the MODE_SENSE command, where it only transfers
    > > > > 4 of 192 bytes.
    > > >
    > > > No, that's not the problem. This is the problem:
    > > >
    > > > > usb-storage: *** thread awakened.
    > > > > usb-storage: Command TEST_UNIT_READY (6 bytes)
    > > > > usb-storage: 00 00 00 00 00 00
    > > > > usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > > usb-storage: Status code 0; transferred 31/31
    > > > > usb-storage: -- transfer complete
    > > > > usb-storage: Bulk command transfer result=0
    > > > > usb-storage: Attempting to get CSW...
    > > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > > > > usb-storage: Status code 0; transferred 13/13
    > > > > usb-storage: -- transfer complete
    > > > > usb-storage: Bulk status result = 0
    > > > > usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > > > > usb-storage: -- transport indicates command failure
    > > > > usb-storage: Issuing auto-REQUEST_SENSE
    > > > > usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > > > > usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > > usb-storage: Status code 0; transferred 31/31
    > > > > usb-storage: -- transfer complete
    > > > > usb-storage: Bulk command transfer result=0
    > > > > usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > > > > usb-storage: usb_sg_init returned -22
    > > > > usb-storage: Bulk data transfer result 0x4
    > > > > usb-storage: -- auto-sense failure
    > > > > usb-storage: storage_pre_reset
    > > > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > > usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > > > > ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > > ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > > usb-storage: storage_post_reset
    > > > > usb-storage: usb_reset_composite_device returns 0
    > > > > usb-storage: scsi cmd done, result=0x70000
    > > > > usb-storage: *** thread sleeping.
    > > >
    > > > For some reason, usb_sg_init is boned during auto-sense.

    > >
    > > OK, that's implicating the scsi_eh_prep_cmnd() in the auto sense
    > > code ... that was also an update in 2.6.24

    >
    > yeah, already found the bug - it's assuming ->request_buffer holds the
    > sglist, oops. Preparing a fix.


    ok here goes, this saves and restores the sg table correctly. it also
    fixes the usb bug for me.

    diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
    index 547e85a..12770ef 100644
    --- a/drivers/scsi/scsi_error.c
    +++ b/drivers/scsi/scsi_error.c
    @@ -622,13 +622,15 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
    ses->use_sg = scmd->use_sg;
    ses->resid = scmd->resid;
    ses->result = scmd->result;
    + memcpy(&ses->sense_sgl, &scmd->sg_table, sizeof(ses->sense_sgl));

    if (sense_bytes) {
    scmd->request_bufflen = min_t(unsigned,
    SCSI_SENSE_BUFFERSIZE, sense_bytes);
    sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
    scmd->request_bufflen);
    - scmd->request_buffer = &ses->sense_sgl;
    + scmd->sg_table.sgl = &ses->sense_sgl;
    + scmd->sg_table.nents = scmd->sg_table.orig_nents = 1;
    scmd->sc_data_direction = DMA_FROM_DEVICE;
    scmd->use_sg = 1;
    memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
    @@ -679,6 +681,7 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
    scmd->request_bufflen = ses->bufflen;
    scmd->request_buffer = ses->buffer;
    scmd->use_sg = ses->use_sg;
    + memcpy(&scmd->sg_table, &ses->sg_table, sizeof(scmd->sg_table));
    scmd->resid = ses->resid;
    scmd->result = ses->result;
    }
    diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
    index d21b891..d43dc83 100644
    --- a/include/scsi/scsi_eh.h
    +++ b/include/scsi/scsi_eh.h
    @@ -75,6 +75,7 @@ struct scsi_eh_save {

    void *buffer;
    unsigned bufflen;
    + struct sg_table sg_table;
    unsigned short use_sg;
    int resid;


    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > On Tue, Jan 29 2008 at 21:45 +0200, Jens Axboe wrote:
    > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > >> On Tue, Jan 29 2008, James Bottomley wrote:
    > >>> On Tue, 2008-01-29 at 11:10 -0800, Matthew Dharm wrote:
    > >>>> On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > >>>>> On Tue, Jan 29 2008, Jens Axboe wrote:
    > >>>>>> On Tue, Jan 29 2008, Oliver Neukum wrote:
    > >>>>>>> Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > >>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > >>>>>>>>> On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > >>>>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > >>>>>>>>>>> Greg KH wrote:
    > >>>>>>>
    > >>>>>>>>>> No difference, still just a lot of resets.
    > >>>>>>>>>>
    > >>>>>>>>> Where you able to figure out which usb storage transport is used?
    > >>>>>>>>>
    > >>>>>>>>> in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > >>>>>>>>> functions. I'm not sure if these get stored in sysfs perhaps. This will
    > >>>>>>>>> pinpoint better where to look. Let me research a bit.
    > >>>>>>>> Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > >>>>>>>> transport is 'Bulk'
    > >>>>>>> You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > >>>>>>> That should tell the reason for the resets.
    > >>>>>> Sure, I'll do that. Will post the results tonight.
    > >>>>> OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > >>>>> in the device, waited 10 seconds or so and pulled it out. These are the
    > >>>>> messages.
    > >>>>>
    > >>>>> It all looks good until the MODE_SENSE command, where it only transfers
    > >>>>> 4 of 192 bytes.
    > >>>> No, that's not the problem. This is the problem:
    > >>>>
    > >>>>> usb-storage: *** thread awakened.
    > >>>>> usb-storage: Command TEST_UNIT_READY (6 bytes)
    > >>>>> usb-storage: 00 00 00 00 00 00
    > >>>>> usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > >>>>> usb-storage: Status code 0; transferred 31/31
    > >>>>> usb-storage: -- transfer complete
    > >>>>> usb-storage: Bulk command transfer result=0
    > >>>>> usb-storage: Attempting to get CSW...
    > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > >>>>> usb-storage: Status code 0; transferred 13/13
    > >>>>> usb-storage: -- transfer complete
    > >>>>> usb-storage: Bulk status result = 0
    > >>>>> usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > >>>>> usb-storage: -- transport indicates command failure
    > >>>>> usb-storage: Issuing auto-REQUEST_SENSE
    > >>>>> usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > >>>>> usb-storage: Status code 0; transferred 31/31
    > >>>>> usb-storage: -- transfer complete
    > >>>>> usb-storage: Bulk command transfer result=0
    > >>>>> usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > >>>>> usb-storage: usb_sg_init returned -22
    > >>>>> usb-storage: Bulk data transfer result 0x4
    > >>>>> usb-storage: -- auto-sense failure
    > >>>>> usb-storage: storage_pre_reset
    > >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    > >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > >>>>> usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    > >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > >>>>> usb-storage: storage_post_reset
    > >>>>> usb-storage: usb_reset_composite_device returns 0
    > >>>>> usb-storage: scsi cmd done, result=0x70000
    > >>>>> usb-storage: *** thread sleeping.
    > >>>> For some reason, usb_sg_init is boned during auto-sense.
    > >>> OK, that's implicating the scsi_eh_prep_cmnd() in the auto sense
    > >>> code ... that was also an update in 2.6.24
    > >> yeah, already found the bug - it's assuming ->request_buffer holds the
    > >> sglist, oops. Preparing a fix.

    > >
    > > ok here goes, this saves and restores the sg table correctly. it also
    > > fixes the usb bug for me.
    > >
    > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
    > > index 547e85a..12770ef 100644
    > > --- a/drivers/scsi/scsi_error.c
    > > +++ b/drivers/scsi/scsi_error.c
    > > @@ -622,13 +622,15 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
    > > ses->use_sg = scmd->use_sg;
    > > ses->resid = scmd->resid;
    > > ses->result = scmd->result;
    > > + memcpy(&ses->sense_sgl, &scmd->sg_table, sizeof(ses->sense_sgl));
    > >
    > > if (sense_bytes) {
    > > scmd->request_bufflen = min_t(unsigned,
    > > SCSI_SENSE_BUFFERSIZE, sense_bytes);
    > > sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
    > > scmd->request_bufflen);
    > > - scmd->request_buffer = &ses->sense_sgl;
    > > + scmd->sg_table.sgl = &ses->sense_sgl;
    > > + scmd->sg_table.nents = scmd->sg_table.orig_nents = 1;
    > > scmd->sc_data_direction = DMA_FROM_DEVICE;
    > > scmd->use_sg = 1;
    > > memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
    > > @@ -679,6 +681,7 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
    > > scmd->request_bufflen = ses->bufflen;
    > > scmd->request_buffer = ses->buffer;
    > > scmd->use_sg = ses->use_sg;
    > > + memcpy(&scmd->sg_table, &ses->sg_table, sizeof(scmd->sg_table));
    > > scmd->resid = ses->resid;
    > > scmd->result = ses->result;
    > > }
    > > diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
    > > index d21b891..d43dc83 100644
    > > --- a/include/scsi/scsi_eh.h
    > > +++ b/include/scsi/scsi_eh.h
    > > @@ -75,6 +75,7 @@ struct scsi_eh_save {
    > >
    > > void *buffer;
    > > unsigned bufflen;
    > > + struct sg_table sg_table;
    > > unsigned short use_sg;
    > > int resid;
    > >
    > >

    > Ok this is not in Linus tree is it? Hence I did not have that failure.


    Yes it is, it's in current -git! James, comments on the patch? Will you
    push it or shall I?

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, 2008-01-29 at 21:03 +0100, Jens Axboe wrote:
    > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > On Tue, Jan 29 2008 at 21:45 +0200, Jens Axboe wrote:
    > > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > >> On Tue, Jan 29 2008, James Bottomley wrote:
    > > >>> On Tue, 2008-01-29 at 11:10 -0800, Matthew Dharm wrote:
    > > >>>> On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > >>>>> On Tue, Jan 29 2008, Jens Axboe wrote:
    > > >>>>>> On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > >>>>>>> Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > >>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > >>>>>>>>> On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > >>>>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > >>>>>>>>>>> Greg KH wrote:
    > > >>>>>>>
    > > >>>>>>>>>> No difference, still just a lot of resets.
    > > >>>>>>>>>>
    > > >>>>>>>>> Where you able to figure out which usb storage transport is used?
    > > >>>>>>>>>
    > > >>>>>>>>> in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > >>>>>>>>> functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > >>>>>>>>> pinpoint better where to look. Let me research a bit.
    > > >>>>>>>> Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > >>>>>>>> transport is 'Bulk'
    > > >>>>>>> You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > >>>>>>> That should tell the reason for the resets.
    > > >>>>>> Sure, I'll do that. Will post the results tonight.
    > > >>>>> OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > >>>>> in the device, waited 10 seconds or so and pulled it out. These are the
    > > >>>>> messages.
    > > >>>>>
    > > >>>>> It all looks good until the MODE_SENSE command, where it only transfers
    > > >>>>> 4 of 192 bytes.
    > > >>>> No, that's not the problem. This is the problem:
    > > >>>>
    > > >>>>> usb-storage: *** thread awakened.
    > > >>>>> usb-storage: Command TEST_UNIT_READY (6 bytes)
    > > >>>>> usb-storage: 00 00 00 00 00 00
    > > >>>>> usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > >>>>> usb-storage: Status code 0; transferred 31/31
    > > >>>>> usb-storage: -- transfer complete
    > > >>>>> usb-storage: Bulk command transfer result=0
    > > >>>>> usb-storage: Attempting to get CSW...
    > > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > > >>>>> usb-storage: Status code 0; transferred 13/13
    > > >>>>> usb-storage: -- transfer complete
    > > >>>>> usb-storage: Bulk status result = 0
    > > >>>>> usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > > >>>>> usb-storage: -- transport indicates command failure
    > > >>>>> usb-storage: Issuing auto-REQUEST_SENSE
    > > >>>>> usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > >>>>> usb-storage: Status code 0; transferred 31/31
    > > >>>>> usb-storage: -- transfer complete
    > > >>>>> usb-storage: Bulk command transfer result=0
    > > >>>>> usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > > >>>>> usb-storage: usb_sg_init returned -22
    > > >>>>> usb-storage: Bulk data transfer result 0x4
    > > >>>>> usb-storage: -- auto-sense failure
    > > >>>>> usb-storage: storage_pre_reset
    > > >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    > > >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > >>>>> usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > > >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    > > >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > >>>>> usb-storage: storage_post_reset
    > > >>>>> usb-storage: usb_reset_composite_device returns 0
    > > >>>>> usb-storage: scsi cmd done, result=0x70000
    > > >>>>> usb-storage: *** thread sleeping.
    > > >>>> For some reason, usb_sg_init is boned during auto-sense.
    > > >>> OK, that's implicating the scsi_eh_prep_cmnd() in the auto sense
    > > >>> code ... that was also an update in 2.6.24
    > > >> yeah, already found the bug - it's assuming ->request_buffer holds the
    > > >> sglist, oops. Preparing a fix.
    > > >
    > > > ok here goes, this saves and restores the sg table correctly. it also
    > > > fixes the usb bug for me.
    > > >
    > > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
    > > > index 547e85a..12770ef 100644
    > > > --- a/drivers/scsi/scsi_error.c
    > > > +++ b/drivers/scsi/scsi_error.c
    > > > @@ -622,13 +622,15 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
    > > > ses->use_sg = scmd->use_sg;
    > > > ses->resid = scmd->resid;
    > > > ses->result = scmd->result;
    > > > + memcpy(&ses->sense_sgl, &scmd->sg_table, sizeof(ses->sense_sgl));
    > > >
    > > > if (sense_bytes) {
    > > > scmd->request_bufflen = min_t(unsigned,
    > > > SCSI_SENSE_BUFFERSIZE, sense_bytes);
    > > > sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
    > > > scmd->request_bufflen);
    > > > - scmd->request_buffer = &ses->sense_sgl;
    > > > + scmd->sg_table.sgl = &ses->sense_sgl;
    > > > + scmd->sg_table.nents = scmd->sg_table.orig_nents = 1;
    > > > scmd->sc_data_direction = DMA_FROM_DEVICE;
    > > > scmd->use_sg = 1;
    > > > memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
    > > > @@ -679,6 +681,7 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
    > > > scmd->request_bufflen = ses->bufflen;
    > > > scmd->request_buffer = ses->buffer;
    > > > scmd->use_sg = ses->use_sg;
    > > > + memcpy(&scmd->sg_table, &ses->sg_table, sizeof(scmd->sg_table));
    > > > scmd->resid = ses->resid;
    > > > scmd->result = ses->result;
    > > > }
    > > > diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
    > > > index d21b891..d43dc83 100644
    > > > --- a/include/scsi/scsi_eh.h
    > > > +++ b/include/scsi/scsi_eh.h
    > > > @@ -75,6 +75,7 @@ struct scsi_eh_save {
    > > >
    > > > void *buffer;
    > > > unsigned bufflen;
    > > > + struct sg_table sg_table;
    > > > unsigned short use_sg;
    > > > int resid;
    > > >
    > > >

    > > Ok this is not in Linus tree is it? Hence I did not have that failure.

    >
    > Yes it is, it's in current -git! James, comments on the patch? Will you
    > push it or shall I?


    I will ... but it will cause an explosion in the bidirectional tree
    again. I think the bidi updates also fix this. However, give me time
    to rebase and verify.

    James


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008 at 21:45 +0200, Jens Axboe wrote:
    > On Tue, Jan 29 2008, Jens Axboe wrote:
    >> On Tue, Jan 29 2008, James Bottomley wrote:
    >>> On Tue, 2008-01-29 at 11:10 -0800, Matthew Dharm wrote:
    >>>> On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    >>>>> On Tue, Jan 29 2008, Jens Axboe wrote:
    >>>>>> On Tue, Jan 29 2008, Oliver Neukum wrote:
    >>>>>>> Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    >>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    >>>>>>>>> On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    >>>>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    >>>>>>>>>>> Greg KH wrote:
    >>>>>>>
    >>>>>>>>>> No difference, still just a lot of resets.
    >>>>>>>>>>
    >>>>>>>>> Where you able to figure out which usb storage transport is used?
    >>>>>>>>>
    >>>>>>>>> in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    >>>>>>>>> functions. I'm not sure if these get stored in sysfs perhaps. This will
    >>>>>>>>> pinpoint better where to look. Let me research a bit.
    >>>>>>>> Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    >>>>>>>> transport is 'Bulk'
    >>>>>>> You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    >>>>>>> That should tell the reason for the resets.
    >>>>>> Sure, I'll do that. Will post the results tonight.
    >>>>> OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    >>>>> in the device, waited 10 seconds or so and pulled it out. These are the
    >>>>> messages.
    >>>>>
    >>>>> It all looks good until the MODE_SENSE command, where it only transfers
    >>>>> 4 of 192 bytes.
    >>>> No, that's not the problem. This is the problem:
    >>>>
    >>>>> usb-storage: *** thread awakened.
    >>>>> usb-storage: Command TEST_UNIT_READY (6 bytes)
    >>>>> usb-storage: 00 00 00 00 00 00
    >>>>> usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    >>>>> usb-storage: Status code 0; transferred 31/31
    >>>>> usb-storage: -- transfer complete
    >>>>> usb-storage: Bulk command transfer result=0
    >>>>> usb-storage: Attempting to get CSW...
    >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    >>>>> usb-storage: Status code 0; transferred 13/13
    >>>>> usb-storage: -- transfer complete
    >>>>> usb-storage: Bulk status result = 0
    >>>>> usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    >>>>> usb-storage: -- transport indicates command failure
    >>>>> usb-storage: Issuing auto-REQUEST_SENSE
    >>>>> usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    >>>>> usb-storage: Status code 0; transferred 31/31
    >>>>> usb-storage: -- transfer complete
    >>>>> usb-storage: Bulk command transfer result=0
    >>>>> usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    >>>>> usb-storage: usb_sg_init returned -22
    >>>>> usb-storage: Bulk data transfer result 0x4
    >>>>> usb-storage: -- auto-sense failure
    >>>>> usb-storage: storage_pre_reset
    >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    >>>>> usb 5-1: reset high speed USB device using ehci_hcd and address 2
    >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    >>>>> usb-storage: storage_post_reset
    >>>>> usb-storage: usb_reset_composite_device returns 0
    >>>>> usb-storage: scsi cmd done, result=0x70000
    >>>>> usb-storage: *** thread sleeping.
    >>>> For some reason, usb_sg_init is boned during auto-sense.
    >>> OK, that's implicating the scsi_eh_prep_cmnd() in the auto sense
    >>> code ... that was also an update in 2.6.24

    >> yeah, already found the bug - it's assuming ->request_buffer holds the
    >> sglist, oops. Preparing a fix.

    >
    > ok here goes, this saves and restores the sg table correctly. it also
    > fixes the usb bug for me.
    >
    > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
    > index 547e85a..12770ef 100644
    > --- a/drivers/scsi/scsi_error.c
    > +++ b/drivers/scsi/scsi_error.c
    > @@ -622,13 +622,15 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
    > ses->use_sg = scmd->use_sg;
    > ses->resid = scmd->resid;
    > ses->result = scmd->result;
    > + memcpy(&ses->sense_sgl, &scmd->sg_table, sizeof(ses->sense_sgl));
    >
    > if (sense_bytes) {
    > scmd->request_bufflen = min_t(unsigned,
    > SCSI_SENSE_BUFFERSIZE, sense_bytes);
    > sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
    > scmd->request_bufflen);
    > - scmd->request_buffer = &ses->sense_sgl;
    > + scmd->sg_table.sgl = &ses->sense_sgl;
    > + scmd->sg_table.nents = scmd->sg_table.orig_nents = 1;
    > scmd->sc_data_direction = DMA_FROM_DEVICE;
    > scmd->use_sg = 1;
    > memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
    > @@ -679,6 +681,7 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
    > scmd->request_bufflen = ses->bufflen;
    > scmd->request_buffer = ses->buffer;
    > scmd->use_sg = ses->use_sg;
    > + memcpy(&scmd->sg_table, &ses->sg_table, sizeof(scmd->sg_table));
    > scmd->resid = ses->resid;
    > scmd->result = ses->result;
    > }
    > diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
    > index d21b891..d43dc83 100644
    > --- a/include/scsi/scsi_eh.h
    > +++ b/include/scsi/scsi_eh.h
    > @@ -75,6 +75,7 @@ struct scsi_eh_save {
    >
    > void *buffer;
    > unsigned bufflen;
    > + struct sg_table sg_table;
    > unsigned short use_sg;
    > int resid;
    >
    >

    Ok this is not in Linus tree is it? Hence I did not have that failure.

    Boaz

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008, James Bottomley wrote:
    > On Tue, 2008-01-29 at 21:03 +0100, Jens Axboe wrote:
    > > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > On Tue, Jan 29 2008 at 21:45 +0200, Jens Axboe wrote:
    > > > > On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > >> On Tue, Jan 29 2008, James Bottomley wrote:
    > > > >>> On Tue, 2008-01-29 at 11:10 -0800, Matthew Dharm wrote:
    > > > >>>> On Tue, Jan 29, 2008 at 07:39:11PM +0100, Jens Axboe wrote:
    > > > >>>>> On Tue, Jan 29 2008, Jens Axboe wrote:
    > > > >>>>>> On Tue, Jan 29 2008, Oliver Neukum wrote:
    > > > >>>>>>> Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe:
    > > > >>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > >>>>>>>>> On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe wrote:
    > > > >>>>>>>>>> On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > > > >>>>>>>>>>> Greg KH wrote:
    > > > >>>>>>>
    > > > >>>>>>>>>> No difference, still just a lot of resets.
    > > > >>>>>>>>>>
    > > > >>>>>>>>> Where you able to figure out which usb storage transport is used?
    > > > >>>>>>>>>
    > > > >>>>>>>>> in drivers/usb/storage/usb.c you have get_protocol() and get_transport()
    > > > >>>>>>>>> functions. I'm not sure if these get stored in sysfs perhaps. This will
    > > > >>>>>>>>> pinpoint better where to look. Let me research a bit.
    > > > >>>>>>>> Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and
    > > > >>>>>>>> transport is 'Bulk'
    > > > >>>>>>> You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG
    > > > >>>>>>> That should tell the reason for the resets.
    > > > >>>>>> Sure, I'll do that. Will post the results tonight.
    > > > >>>>> OK, fresh boot with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG. Plugged
    > > > >>>>> in the device, waited 10 seconds or so and pulled it out. These are the
    > > > >>>>> messages.
    > > > >>>>>
    > > > >>>>> It all looks good until the MODE_SENSE command, where it only transfers
    > > > >>>>> 4 of 192 bytes.
    > > > >>>> No, that's not the problem. This is the problem:
    > > > >>>>
    > > > >>>>> usb-storage: *** thread awakened.
    > > > >>>>> usb-storage: Command TEST_UNIT_READY (6 bytes)
    > > > >>>>> usb-storage: 00 00 00 00 00 00
    > > > >>>>> usb-storage: Bulk Command S 0x43425355 T 0xd L 0 F 0 Trg 0 LUN 1 CL 6
    > > > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > >>>>> usb-storage: Status code 0; transferred 31/31
    > > > >>>>> usb-storage: -- transfer complete
    > > > >>>>> usb-storage: Bulk command transfer result=0
    > > > >>>>> usb-storage: Attempting to get CSW...
    > > > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
    > > > >>>>> usb-storage: Status code 0; transferred 13/13
    > > > >>>>> usb-storage: -- transfer complete
    > > > >>>>> usb-storage: Bulk status result = 0
    > > > >>>>> usb-storage: Bulk Status S 0x53425355 T 0xd R 0 Stat 0x1
    > > > >>>>> usb-storage: -- transport indicates command failure
    > > > >>>>> usb-storage: Issuing auto-REQUEST_SENSE
    > > > >>>>> usb-storage: Bulk Command S 0x43425355 T 0xe L 18 F 128 Trg 0 LUN 1 CL 6
    > > > >>>>> usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
    > > > >>>>> usb-storage: Status code 0; transferred 31/31
    > > > >>>>> usb-storage: -- transfer complete
    > > > >>>>> usb-storage: Bulk command transfer result=0
    > > > >>>>> usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
    > > > >>>>> usb-storage: usb_sg_init returned -22
    > > > >>>>> usb-storage: Bulk data transfer result 0x4
    > > > >>>>> usb-storage: -- auto-sense failure
    > > > >>>>> usb-storage: storage_pre_reset
    > > > >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > >>>>> usb 5-1: reset high speed USB device using ehci_hcd and address 2
    > > > >>>>> ehci_hcd 0000:00:1d.7: port 1 high speed
    > > > >>>>> ehci_hcd 0000:00:1d.7: GetStatus port 1 status 001005 POWER sig=se0 PE CONNECT
    > > > >>>>> usb-storage: storage_post_reset
    > > > >>>>> usb-storage: usb_reset_composite_device returns 0
    > > > >>>>> usb-storage: scsi cmd done, result=0x70000
    > > > >>>>> usb-storage: *** thread sleeping.
    > > > >>>> For some reason, usb_sg_init is boned during auto-sense.
    > > > >>> OK, that's implicating the scsi_eh_prep_cmnd() in the auto sense
    > > > >>> code ... that was also an update in 2.6.24
    > > > >> yeah, already found the bug - it's assuming ->request_buffer holds the
    > > > >> sglist, oops. Preparing a fix.
    > > > >
    > > > > ok here goes, this saves and restores the sg table correctly. it also
    > > > > fixes the usb bug for me.
    > > > >
    > > > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
    > > > > index 547e85a..12770ef 100644
    > > > > --- a/drivers/scsi/scsi_error.c
    > > > > +++ b/drivers/scsi/scsi_error.c
    > > > > @@ -622,13 +622,15 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
    > > > > ses->use_sg = scmd->use_sg;
    > > > > ses->resid = scmd->resid;
    > > > > ses->result = scmd->result;
    > > > > + memcpy(&ses->sense_sgl, &scmd->sg_table, sizeof(ses->sense_sgl));
    > > > >
    > > > > if (sense_bytes) {
    > > > > scmd->request_bufflen = min_t(unsigned,
    > > > > SCSI_SENSE_BUFFERSIZE, sense_bytes);
    > > > > sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
    > > > > scmd->request_bufflen);
    > > > > - scmd->request_buffer = &ses->sense_sgl;
    > > > > + scmd->sg_table.sgl = &ses->sense_sgl;
    > > > > + scmd->sg_table.nents = scmd->sg_table.orig_nents = 1;
    > > > > scmd->sc_data_direction = DMA_FROM_DEVICE;
    > > > > scmd->use_sg = 1;
    > > > > memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
    > > > > @@ -679,6 +681,7 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
    > > > > scmd->request_bufflen = ses->bufflen;
    > > > > scmd->request_buffer = ses->buffer;
    > > > > scmd->use_sg = ses->use_sg;
    > > > > + memcpy(&scmd->sg_table, &ses->sg_table, sizeof(scmd->sg_table));
    > > > > scmd->resid = ses->resid;
    > > > > scmd->result = ses->result;
    > > > > }
    > > > > diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
    > > > > index d21b891..d43dc83 100644
    > > > > --- a/include/scsi/scsi_eh.h
    > > > > +++ b/include/scsi/scsi_eh.h
    > > > > @@ -75,6 +75,7 @@ struct scsi_eh_save {
    > > > >
    > > > > void *buffer;
    > > > > unsigned bufflen;
    > > > > + struct sg_table sg_table;
    > > > > unsigned short use_sg;
    > > > > int resid;
    > > > >
    > > > >
    > > > Ok this is not in Linus tree is it? Hence I did not have that failure.

    > >
    > > Yes it is, it's in current -git! James, comments on the patch? Will you
    > > push it or shall I?

    >
    > I will ... but it will cause an explosion in the bidirectional tree
    > again. I think the bidi updates also fix this. However, give me time
    > to rebase and verify.


    OK thanks, just wanted to make sure that we didn't both expect each
    other to handle it :-)

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [BUG] 2.6.24-git usb reset problems

    >>
    > Ok this is not in Linus tree is it? Hence I did not have that failure.
    >
    > Boaz
    >
    >


    actually James bidi tree has a fix for this in the scsi_data_buffer patch.

    what you sent is not enough there are other places. look at this patch I
    sent to the list.

    http://www.spinics.net/lists/linux-scsi/msg21938.html

    Could we take the 2 SG patches and submit them through the scsi
    bidi tree? It is much more natural to have them in one tree as one
    patchset then try coordinate with git-merge. Actually if you look at it,
    the biggest change is to SCSI. So I think it is more natural this way

    Boaz
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008, Boaz Harrosh wrote:
    > >>

    > > Ok this is not in Linus tree is it? Hence I did not have that failure.
    > >
    > > Boaz
    > >
    > >

    >
    > actually James bidi tree has a fix for this in the scsi_data_buffer patch.
    >
    > what you sent is not enough there are other places. look at this patch I
    > sent to the list.
    >
    > http://www.spinics.net/lists/linux-scsi/msg21938.html


    Hard to compare, since its on different bases.

    > Could we take the 2 SG patches and submit them through the scsi
    > bidi tree? It is much more natural to have them in one tree as one
    > patchset then try coordinate with git-merge. Actually if you look at it,
    > the biggest change is to SCSI. So I think it is more natural this way


    What 2 sg patches do you mean?

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008 at 22:13 +0200, Jens Axboe wrote:
    > On Tue, Jan 29 2008, Boaz Harrosh wrote:
    >>> Ok this is not in Linus tree is it? Hence I did not have that failure.
    >>>
    >>> Boaz
    >>>
    >>>

    >> actually James bidi tree has a fix for this in the scsi_data_buffer patch.
    >>
    >> what you sent is not enough there are other places. look at this patch I
    >> sent to the list.
    >>
    >> http://www.spinics.net/lists/linux-scsi/msg21938.html

    >
    > Hard to compare, since its on different bases.

    Yes in this patchset I have taken your sg branch at the time, and
    rebased it ontop of scsi_data_buffer patch. Because I felt that
    it is more natural for this patch to come after the scsi total
    cleanup that is scsi_data_buffer. Then the extraction to sg_table
    is simple and trivial.

    What I meant to point out with this patch is that all the exact same
    places that are touched there should be fixed when moving to sg_table.

    Look at it. It is a revised version of your patch.

    >
    >> Could we take the 2 SG patches and submit them through the scsi
    >> bidi tree? It is much more natural to have them in one tree as one
    >> patchset then try coordinate with git-merge. Actually if you look at it,
    >> the biggest change is to SCSI. So I think it is more natural this way

    >
    > What 2 sg patches do you mean?
    >


    I mean the patches that where in sg branch of the linux-block tree, But
    I see that it is now to late, and that they are in Linus already

    James the most simple is to submit the scsi_data_buff patch that fixes
    all these places. If not do you want that I send in fixes?

    Boaz
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, 2008-01-29 at 21:06 +0100, Jens Axboe wrote:
    > > I will ... but it will cause an explosion in the bidirectional tree
    > > again. I think the bidi updates also fix this. However, give me time
    > > to rebase and verify.

    >
    > OK thanks, just wanted to make sure that we didn't both expect each
    > other to handle it :-)


    Yes, confirm that; I think this is already fixed in scsi-misc-2.6.
    Could you pull from

    master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6.git

    and verify with your device?

    Thanks,

    James


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [BUG] 2.6.24-git usb reset problems

    On Tue, Jan 29 2008 at 22:24 +0200, James Bottomley wrote:
    > On Tue, 2008-01-29 at 21:06 +0100, Jens Axboe wrote:
    >>> I will ... but it will cause an explosion in the bidirectional tree
    >>> again. I think the bidi updates also fix this. However, give me time
    >>> to rebase and verify.

    >> OK thanks, just wanted to make sure that we didn't both expect each
    >> other to handle it :-)

    >
    > Yes, confirm that; I think this is already fixed in scsi-misc-2.6.
    > Could you pull from
    >
    > master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6.git
    >
    > and verify with your device?
    >
    > Thanks,
    >
    > James
    >
    >

    I still don't see these changes, I wanted to check, make sure...
    Are there mirrors on the way to here?

    James I would like to remind you that one small piece is missing
    from the bidi tree, as I saw it from here, it's the few patches from
    scsi-pending. Mainly arm will break which is a grate loss.

    I'm going home now, I'll review all the patches tomorrow and compare
    to what I have, to make sure nothing was forgotten. What a waste of a day
    I pulled from Linus this morning apparently minutes before the merge, and
    chased a none existent bug in my tree. Sigh

    Bye
    Boaz

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast