All of lore.kernel.org
 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 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.