public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marek.vasut@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] usb_test_unit_ready called every block read - performance
Date: Sun, 19 Aug 2012 01:17:15 +0200	[thread overview]
Message-ID: <201208190117.16063.marex@denx.de> (raw)
In-Reply-To: <CAP74FYBAwaFgfpVbSapWLL3ZPnGypN_UpMFbA37dgYN0LUKRJA@mail.gmail.com>

Dear Steve Heckman,

> Oh yeah, forgot about that...;)
> 
> I just downloaded the ext4 branch tarball and built it.
> 
> The test_unit_ready calls were still in there.
> 
> With or without those it took 0m 45s to load a ~150MB image.
> 
> In our original branch (2011.12), the test_unit_ready calls had more of an
> impact. The stock 2011.12 u-boot image took 6m 22s to load the 150MB file.
> Without test_unit_ready, it took 3m 15s. Without test_unit_ready and
> wait_ms(5) in usb_stor_BBB_transport() it took 0m 16s.
> 
> In the ext4 branch, I removed test_unit_ready and the mdelay(5) call
> from usb_stor_BBB_transport() function and was able to load the same file
> in 0m 8s.
> 
> So, removing the mdelay(5) call is the biggest improvement. Removing both
> is the best.
> 
> To recap:
> 
>         with w/o  w/o
>         TUR  TUR  TUR and 5ms wait
> 2011.12 6:25 3:15 0:16
> ext4    0:45 0:45 0:08
> 
> Note: all these time include the 3-4 seconds it takes to do the "usb
> start".

Coolness ! :-) Please just retest by applying those ext4 patches on top of uboot 
usb and see how fast it goes :)

> Regards,
> -Steve
> 
> On Wed, Aug 15, 2012 at 10:19 AM, Jim Shimer <mgi2475@motorola.com> wrote:
> > Hi Marek,
> > 
> > I looked at the ext4 branch.  It looks like he has the patch to remove
> > the usb_test_unit_ready() calls which were not needed. Actually those
> > calls are commented out on that branch:
> > #if 0
> > 
> >         if (usb_test_unit_ready(srb, ss)) {
> >         
> >                 printf("Device NOT ready\n   Request Sense returned %02X
> > 
> > %02X"
> > 
> >                        " %02X\n", srb->sense_buf[2], srb->sense_buf[12],
> >                        
> >                         srb->sense_buf[13]);
> >                 
> >                 return 0;
> >         
> >         }
> > 
> > #endif
> > 
> > In the u-boot-usb.git, this code is removed so at some point there will
> > be a merge conflict.
> > 
> > Also the ext4 branch still has the mdelay(5) always being done in
> > usb_stor_BBB_transport() line 696 which we found to be the largest
> > performance killer.
> > 
> > Regards,
> > Jim
> > 
> > On Sun, Aug 12, 2012 at 7:54 PM, Marek Vasut <marex@denx.de> wrote:
> >> Dear Jim Shimer,
> >> 
> >> > While tuning ext2load, we found that usb_test_unit_ready was being
> >> 
> >> called
> >> 
> >> > every block read.  We compared the usb block storage to the scsi block
> >> > storage cmd_scsi.c, and found that the scsi device was only calling
> >> > its scsi_setup_test_unit_ready() during scsi_can.  It appears that
> >> > usb_test_unit_ready() really only needs to be called once during
> >> > usb_stor_scan(), via usb_stor_get_info().   Is there a particular
> >> > reason usb_test_unit_ready is called for every block read, or do you
> >> > think its
> >> 
> >> ok
> >> 
> >> > to only call during usb_stor_scan()?  We're finding this speeds up
> >> 
> >> ext2load
> >> 
> >> > quite a bit.
> >> 
> >> Jim, did we get anywhere on this one ? Can you try with the new ext4
> >> code in
> >> Wolfgangs' u-boot-master/ext4 branch?
> >> 
> >> > Regards,
> >> > Jim
> >> 
> >> Best regards,
> >> Marek Vasut
> > 
> > --
> > *James H Shimer*
> > Motorola Mobility T3-12-HH72
> > 900 Chelmsford Street
> > Lowell MA 08151
> > 978-614-3550

  reply	other threads:[~2012-08-18 23:17 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-30 19:18 [U-Boot] usb_test_unit_ready called every block read - performance Jim Shimer
2012-07-30 22:54 ` Marek Vasut
2012-07-30 23:35   ` Jim Shimer
2012-07-30 23:41     ` Marek Vasut
2012-07-31  2:17       ` Jim Shimer
2012-07-31  4:43         ` Wolfgang Denk
2012-08-14 17:50         ` Marek Vasut
2012-08-14 17:57           ` Steve Heckman
2012-08-15 14:04             ` Jim Shimer
2012-08-12 23:54 ` Marek Vasut
2012-08-14 13:55   ` Steve Heckman
2012-08-14 17:48     ` Marek Vasut
2012-08-15 14:19   ` Jim Shimer
2012-08-15 16:45     ` Steve Heckman
2012-08-18 23:17       ` Marek Vasut [this message]
2012-08-18 23:16     ` Marek Vasut

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=201208190117.16063.marex@denx.de \
    --to=marek.vasut@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

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

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