public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sarah Sharp <sarah.a.sharp@linux.intel.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Tim Vlaar <Tvlaar@ptgrey.com>, Greg KH <greg@kroah.com>,
	Markus Rechberger <mrechberger@gmail.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	USB list <linux-usb@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [Patch] Increase USBFS Bulk Transfer size
Date: Mon, 7 Nov 2011 12:18:12 -0800	[thread overview]
Message-ID: <20111107201812.GA12822@xanatos> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1111071403290.2004-100000@iolanthe.rowland.org>

On Mon, Nov 07, 2011 at 02:12:16PM -0500, Alan Stern wrote:
> On Mon, 7 Nov 2011, Sarah Sharp wrote:
> 
> > On Fri, Oct 14, 2011 at 08:33:29AM -0600, Greg KH wrote:
> > > On Fri, Oct 14, 2011 at 10:05:41AM -0400, Alan Stern wrote:
> > > > No, a much better approach is to remove all limits on individual
> > > > transfer sizes and instead have a global limit on the total amount of
> > > > all usbfs buffers in use at any time.  Maybe something like 16 MB; at 
> > > > SuperSpeed, that's about about 30 ms worth of data.
> > > 
> > > That sounds quite reasonable.
> > 
> > Alan, won't this global limit on the usbfs URB buffer size effect
> > userspace drivers that are currently allocating large amounts of
> > buffers, but still respecting individual buffer limit of 16KB?  It seems
> > like the patch has the potential to break userspace drivers.
> 
> It might indeed.  A further enhancement would replace that 16-MB global
> constant with a sysfs attribute (a writable module parameter for
> usbcore).  Do you have any better suggestions?

No, I don't have any better suggestions, except take out the limit. ;)

I do understand why we don't want userspace to DoS the system by using
up too much DMA'able memory.  However, as I understand it, the usbfs
files are created by udev with root access only by default, and distros
may choose to install rules that have more permissive privileges.  A
device vendor may not be ensured that a udev rule with permissive access
will be present for their device, so I think they're likely to write
programs that require root access.  Or require root privileges to
install said udev rule.

At that point, the same userspace program that has root privileges in
order to access usbfs or create the udev rule can just load and unload
the usbcore module with an arbitrarily large global limit, and the
global limit doesn't really add any security.  So why add the extra
barrier?

> > I think that Point Grey's USB 3.0 webcam will be attempting to queue a
> > series of bulk URBs that will be bigger than your 16MB global limit.
> 
> For SuperSpeed, 16 MB is rather on the low side.  For high speed it
> amounts to about 1/3-second worth of data, which arguably is also a bit
> low.  Increasing the default is easy enough, but the best choice isn't
> obvious.

Yeah, the choice is not obvious and we're probably going to get it
wrong, but as Tim said, he does need ~600MB in flight at once, so I knew
16MB was too small.  I guess the question really should be not "What is
the smallest limit we need?" but "When will the system start breaking
down due to memory pressure?" and set the limit somewhere pretty close
to there.

Do other subsystems have these issues as well?  Does the layer SCSI ever
limit the number of outstanding READ requests (aside from hardware
limitations)?  Or does the networking layer have a limit to the buffers
it keeps pending transfers for userspace to read?

Sarah Sharp

  reply	other threads:[~2011-11-07 20:18 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-12 12:36 [Patch] Increase USBFS Bulk Transfer size Markus Rechberger
2011-10-12 12:46 ` Markus Rechberger
2011-10-12 13:48 ` Sergei Shtylyov
2011-10-12 14:17 ` Greg KH
2011-10-12 16:59   ` Markus Rechberger
2011-10-12 20:33     ` Greg KH
2011-10-12 21:48       ` Markus Rechberger
2011-10-12 22:09         ` Markus Rechberger
2011-10-13  4:03           ` Manu Abraham
2011-10-13  4:59             ` Markus Rechberger
2011-10-13  5:46               ` Manu Abraham
2011-10-13  8:37                 ` Markus Rechberger
2011-10-13  9:29                   ` Markus Rechberger
2011-10-16  9:22                     ` James Courtier-Dutton
2011-10-13  9:34                   ` Manu Abraham
2011-10-13  9:39                     ` Markus Rechberger
2011-10-13 14:58           ` Alan Stern
2011-10-13 15:19             ` Markus Rechberger
2011-10-13 16:01               ` Chris Friesen
2011-10-13 16:12                 ` Markus Rechberger
2011-10-13 16:25                   ` Chris Friesen
2011-10-13 18:27                     ` Markus Rechberger
2011-10-13 20:07                       ` Alan Stern
2011-10-13 20:17                         ` Markus Rechberger
2011-10-13 18:21               ` Alan Stern
2011-10-13 19:05                 ` Alan Cox
2011-10-14 19:21             ` Johannes Stezenbach
2011-10-14 20:19               ` Alan Stern
2011-10-14 22:45                 ` Johannes Stezenbach
2011-10-15 11:45                   ` Markus Rechberger
2011-10-15 17:47                     ` Valdis.Kletnieks
2011-10-15 19:08                     ` Alan Stern
2011-10-15 19:04                   ` Alan Stern
2011-10-16  9:10                     ` Johannes Stezenbach
2011-10-16 14:18                       ` Alan Stern
2011-10-17 18:11                     ` Johannes Stezenbach
2011-10-17 18:22                       ` Alan Stern
     [not found]           ` <CAAMvbhFNTQeuJBgsDB9Y5ODc_b2O0X=oP_3uwRpWUREFS9qufA@mail.gmail.com>
2011-10-14  2:47             ` Markus Rechberger
2011-10-14  3:42               ` Markus Rechberger
2011-10-14  3:48                 ` Markus Rechberger
2011-10-14  5:47                 ` Valdis.Kletnieks
2011-10-14  6:23                   ` Markus Rechberger
2011-10-14  8:51                     ` James Courtier-Dutton
2011-10-14 15:38                       ` Markus Rechberger
2011-10-14 14:05                 ` Alan Stern
2011-10-14 14:33                   ` Greg KH
2011-11-07 18:52                     ` Sarah Sharp
2011-11-07 19:12                       ` Alan Stern
2011-11-07 20:18                         ` Sarah Sharp [this message]
2011-11-07 20:37                           ` Brink, Peter
2011-11-07 20:53                           ` Alan Stern
2011-11-07 21:49                             ` Greg KH
2011-11-07 23:07                             ` Sarah Sharp
2011-11-08  1:44                               ` Alan Stern
2011-11-07 19:16                       ` Tim Vlaar
2011-11-07 19:55                         ` Alan Stern
2011-11-07 20:13                           ` Tim Vlaar
2011-10-17 18:38                 ` Alan Stern
2011-10-17 19:07                   ` Markus Rechberger
2011-10-12 18:00   ` Mihai Moldovan
2011-10-12 20:36     ` Greg KH

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=20111107201812.GA12822@xanatos \
    --to=sarah.a.sharp@linux.intel.com \
    --cc=Tvlaar@ptgrey.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mrechberger@gmail.com \
    --cc=stern@rowland.harvard.edu \
    /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