RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Adrian Bunk]

I've already seen people crippling their usb-storage driver with 
enabling BLK_DEV_UB - and I doubt the warning in the help text added 
after 2.6.9 will fix all such problems.

Is there except for kernel size any good reason for using BLK_DEV_UB 
instead of USB_STORAGE?

If not, I'd suggest the patch below to let BLK_DEV_UB depend
on EMBEDDED.


Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.10-rc3-mm1-full/drivers/block/Kconfig.old	2004-12-20 00:52:22.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/drivers/block/Kconfig	2004-12-20 00:52:39.000000000 +0100
@@ -303,7 +303,7 @@
 
 config BLK_DEV_UB
 	tristate "Low Performance USB Block driver"
-	depends on USB
+	depends on USB && EMBEDDED
 	help
 	  This driver supports certain USB attached storage devices
 	  such as flash keys.

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Pete Zaitcev]

On Mon, 20 Dec 2004 01:16:44 +0100, Adrian Bunk <bunk@stusta.de> wrote:

> Is there except for kernel size any good reason for using BLK_DEV_UB 
> instead of USB_STORAGE?

It depends. I like it.

>  config BLK_DEV_UB
>  	tristate "Low Performance USB Block driver"
> -	depends on USB
> +	depends on USB && EMBEDDED

The ub has nothing to do with embedded systems, most of which do not run
with CONFIG_EMBEDDED, BTW.

-- Pete

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Greg KH]

On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> I've already seen people crippling their usb-storage driver with 
> enabling BLK_DEV_UB - and I doubt the warning in the help text added 
> after 2.6.9 will fix all such problems.
> 
> Is there except for kernel size any good reason for using BLK_DEV_UB 
> instead of USB_STORAGE?

You don't want to use the scsi layer?  You like the stability of it at
times?  :)

> If not, I'd suggest the patch below to let BLK_DEV_UB depend
> on EMBEDDED.

No, it's good for non-embedded boxes too.

thanks,

greg k-h

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Adrian Bunk]

On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > I've already seen people crippling their usb-storage driver with 
> > enabling BLK_DEV_UB - and I doubt the warning in the help text added 
> > after 2.6.9 will fix all such problems.
> > 
> > Is there except for kernel size any good reason for using BLK_DEV_UB 
> > instead of USB_STORAGE?
> 
> You don't want to use the scsi layer?  You like the stability of it at
> times?  :)
>...

What about a dependency of BLK_DEV_UB on USB_STORAGE=n ?

> thanks,
> 
> greg k-h

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Pete Zaitcev]

On Mon, 20 Dec 2004 02:35:42 +0100, Adrian Bunk <bunk@stusta.de> wrote:

> What about a dependency of BLK_DEV_UB on USB_STORAGE=n ?

I have them both as 'm' in my configuration, works like a charm.

-- Pete

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Randy.Dunlap]

Pete Zaitcev wrote:
> On Mon, 20 Dec 2004 02:35:42 +0100, Adrian Bunk <bunk@stusta.de> wrote:
> 
> 
>>What about a dependency of BLK_DEV_UB on USB_STORAGE=n ?
> 
> 
> I have them both as 'm' in my configuration, works like a charm.

ub can work like that, but it makes it darned difficult to
use usb-storage like that.  ub wants to bind to the devices,
not usb-storage, and if ub is unloaded, usb-storage doesn't
bind to them.  at least that's been my experience with it.

-- 
~Randy

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

On Sun, Dec 19, 2004 at 09:09:52PM -0800, Randy.Dunlap wrote:
> Pete Zaitcev wrote:
> >On Mon, 20 Dec 2004 02:35:42 +0100, Adrian Bunk <bunk@stusta.de> wrote:
> >
> >
> >>What about a dependency of BLK_DEV_UB on USB_STORAGE=n ?
> >
> >
> >I have them both as 'm' in my configuration, works like a charm.
> 
> ub can work like that, but it makes it darned difficult to
> use usb-storage like that.  ub wants to bind to the devices,
> not usb-storage, and if ub is unloaded, usb-storage doesn't
> bind to them.  at least that's been my experience with it.

Enabling CONFIG_BLK_DEV_UB actually disables usb-storage from attaching to
certain devices, regardless of what's loaded or not.

I, personally, don't like this.  But I wasn't consulted on that particular
feature.  I'm given to understand that some bad things can happen when two
drivers can bind to the same device, but I haven't had time to experiment
with it.

I can tell you that this has turned into the single largest source of bug
reports/complaints about usb-storage.  Something has to be done.  I just
don't know what.

Matt

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

C:  They kicked your ass, didn't they?
S:  They were cheating!
					-- The Chief and Stef
User Friendly, 11/19/1997

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Pete Zaitcev]

On Sun, 19 Dec 2004 21:09:52 -0800, "Randy.Dunlap" <rddunlap@osdl.org> wrote:

> Pete Zaitcev wrote:
> > On Mon, 20 Dec 2004 02:35:42 +0100, Adrian Bunk <bunk@stusta.de> wrote:
> > 
> >>What about a dependency of BLK_DEV_UB on USB_STORAGE=n ?
> > 
> > I have them both as 'm' in my configuration, works like a charm.
> 
> ub can work like that, but it makes it darned difficult to
> use usb-storage like that.  ub wants to bind to the devices,
> not usb-storage, and if ub is unloaded, usb-storage doesn't
> bind to them.  at least that's been my experience with it.

There is no asymmetry in lists of devices in either of them, however
currently there aren't any devices which usb-storage cannot handle
and ub can. Thus it makes sense to conditionalize part of usb-storage
list on ub. Otherwise, it would be a separate configuration item,
I suppose. We'll when we get there.

I don't quite understand why it matters for you if a certain module
is loaded or not loaded. The hotplug acts upon the contents of
modules.usbmap which does not change when you modprobe or rmmod things.
So, the match lists are made non-conflicting between
ub and usb-storage. It looks as if Adrian has the same broken mental
model of the way things work. Once again, what is loaded does not
matter (not in ideal world, but in reality we still have conflicts such
as e100 and eepro100).

-- Pete

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Pete Zaitcev]

On Sun, 19 Dec 2004 22:20:55 -0800, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:

> I can tell you that this has turned into the single largest source of bug
> reports/complaints about usb-storage.  Something has to be done.  I just
> don't know what.

Is it that bad, really? Honestly, I could not imagine users can be so dumb.
The option defaults to off. There is a warning in the Kconfig. And yet they
first enable it and then complain about it. I don't know what to do about
it, either.

-- Pete

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[David Brownell]

On Sunday 19 December 2004 10:20 pm, Matthew Dharm wrote:
> 
> Enabling CONFIG_BLK_DEV_UB actually disables usb-storage from attaching to
> certain devices, regardless of what's loaded or not.

It also seems to mean significantly slower access (at high speed)
for the most standard devices.  That doesn't seem like a win,
though I suspect fixing it should be as simple as switching over
to use the USB scatterlist calls (which usb-storage uses) ...

- Davve


> I can tell you that this has turned into the single largest source of bug
> reports/complaints about usb-storage.  Something has to be done.  I just
> don't know what.

 

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Pete Zaitcev]

On Sun, 19 Dec 2004 22:43:05 -0800, David Brownell <david-b@pacbell.net> wrote:

> It also seems to mean significantly slower access (at high speed)
> for the most standard devices.  That doesn't seem like a win,
> though I suspect fixing it should be as simple as switching over
> to use the USB scatterlist calls (which usb-storage uses) ...

They do not allow asynchronous operation, last time I checked.

-- Pete

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Phil Dibowitz]

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

Pete Zaitcev wrote:
> On Sun, 19 Dec 2004 22:20:55 -0800, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:
> 
> 
>>I can tell you that this has turned into the single largest source of bug
>>reports/complaints about usb-storage.  Something has to be done.  I just
>>don't know what.
> 
> 
> Is it that bad, really? Honestly, I could not imagine users can be so dumb.
> The option defaults to off. There is a warning in the Kconfig. And yet they
> first enable it and then complain about it. I don't know what to do about
> it, either.

I was told this was enabled in the mandrake 2.6.9 kernel. I haven't 
confirmed that.

Yes, users are very stupid. Ever worked in tech support?

-- 
Phil Dibowitz                             phil@ipom.com
Freeware and Technical Pages              Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"They that can give up essential liberty to obtain a little temporary
safety deserve neither liberty nor safety."
  - Benjamin Franklin, 1759


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1650 bytes --]

On Sun, Dec 19, 2004 at 10:37:23PM -0800, Pete Zaitcev wrote:
> On Sun, 19 Dec 2004 22:20:55 -0800, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:
> 
> > I can tell you that this has turned into the single largest source of bug
> > reports/complaints about usb-storage.  Something has to be done.  I just
> > don't know what.
> 
> Is it that bad, really? Honestly, I could not imagine users can be so dumb.

'There are more things in Heaven and Earth, Horatio...."

Yes, users can be so dumb.  Commonly, the exchange goes like this:

USER: My device used to work in 2.6.7, and now it's busted.
US: Please turn on debugging and send us the logs.
USER: (sends dmesg which shows ub in use)
US: Turn off CONFIG_BLK_DEV_UB
USER: It works!  You should make it more clear that UB breaks things...

And that's the optimal case.  Often times they cut the dmesg so we can't
see what's wrong.  And this usually takes 3 days for the entire exchange.

> The option defaults to off. There is a warning in the Kconfig. And yet they
> first enable it and then complain about it. I don't know what to do about
> it, either.

The best idea I have is to hide CONFIG_BLK_DEV_UB behind
CONFIG_EXPERIMENTAL.

The next-best idea I have is to make UB print out some sort of warning
message at startup.

Neither of these ideas is very good, I'll admit.

Matt

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

A:  The most ironic oxymoron wins ...
DP: "Microsoft Works"
A:  Uh, okay, you win.
					-- A.J. & Dust Puppy
User Friendly, 1/18/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Phil Dibowitz]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

Matthew Dharm wrote:
> 
> The best idea I have is to hide CONFIG_BLK_DEV_UB behind
> CONFIG_EXPERIMENTAL.

As a long-term solution, this obviously won't work, but I think it's a 
_very_ good solution for now. UB _is_ expiramental at this point. It 
still has it's issues, and per Pete is still not to be used unless you 
really know what you're doing.

That makes it expiramental to me.

> The next-best idea I have is to make UB print out some sort of warning
> message at startup.
> 
> Neither of these ideas is very good, I'll admit.

This is decent  idea. Some "Warning: UB enabled - your usb-storage 
devices may not work properly!" type thing on loading can't hurt.

Not the cleanest thing in the world, but look how well it worked for the 
unusual_devs cleanups.

-- 
Phil Dibowitz                             phil@ipom.com
Freeware and Technical Pages              Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"They that can give up essential liberty to obtain a little temporary
safety deserve neither liberty nor safety."
  - Benjamin Franklin, 1759


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Pete Zaitcev]

On Mon, 20 Dec 2004 00:09:51 -0800, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:

> The best idea I have is to hide CONFIG_BLK_DEV_UB behind
> CONFIG_EXPERIMENTAL.

I thought about it, but I do not like CONFIG_EXPERIMENTAL as a concept.
I seem to recall a few instances when it was practically required, because
some necessary driver was covered by it, and so users ran it always-on.
AFAIK, both Fedora and Red Hat Enterprise Linux 4 Beta have it enabled
for that reason. We can try it, certainly, to see if it helps.

> The next-best idea I have is to make UB print out some sort of warning
> message at startup.

This is probably a cure worse than the disease.

-- Pete

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Phil Dibowitz]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

Pete Zaitcev wrote:
> On Mon, 20 Dec 2004 00:09:51 -0800, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:
> 
> 
>>The best idea I have is to hide CONFIG_BLK_DEV_UB behind
>>CONFIG_EXPERIMENTAL.
> 
> 
> I thought about it, but I do not like CONFIG_EXPERIMENTAL as a concept.
> I seem to recall a few instances when it was practically required, because
> some necessary driver was covered by it, and so users ran it always-on.
> AFAIK, both Fedora and Red Hat Enterprise Linux 4 Beta have it enabled
> for that reason. We can try it, certainly, to see if it helps.

Just because other people use it wrong doesn't mean we can't use it right...

-- 
Phil Dibowitz                             phil@ipom.com
Freeware and Technical Pages              Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"They that can give up essential liberty to obtain a little temporary
safety deserve neither liberty nor safety."
  - Benjamin Franklin, 1759


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Ed Tomlinson]

On Monday 20 December 2004 01:37, Pete Zaitcev wrote:
> On Sun, 19 Dec 2004 22:20:55 -0800, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:
> 
> > I can tell you that this has turned into the single largest source of bug
> > reports/complaints about usb-storage.  Something has to be done.  I just
> > don't know what.
> 
> Is it that bad, really? Honestly, I could not imagine users can be so dumb.
> The option defaults to off. There is a warning in the Kconfig. And yet they
> first enable it and then complain about it. I don't know what to do about
> it, either.

Its not that they just enable it.  Its that it has side effects.  I enable it to support
one device - it then 'devnaps' other devices that usbstorage supports _much_
better.  Is there some way it could work in reverse.  eg. let ub bind only if 
usbstorage does not, possibly making usbstorage a _little_ more conservative
if ub is present?

Ed Tomlinson

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Alan Stern]

On Sun, 19 Dec 2004, Pete Zaitcev wrote:

> I don't quite understand why it matters for you if a certain module
> is loaded or not loaded. The hotplug acts upon the contents of
> modules.usbmap which does not change when you modprobe or rmmod things.
> So, the match lists are made non-conflicting between
> ub and usb-storage. It looks as if Adrian has the same broken mental
> model of the way things work. Once again, what is loaded does not
> matter (not in ideal world, but in reality we still have conflicts such
> as e100 and eepro100).

No one has posted a reply to this point, so here's one even if it is 
unnecessary.

What matters is not which modules are loaded, but rather which modules 
bind to which devices.  If ub is configured then usb-storage will not bind 
to certain devices, regardless of what modules are loaded.

Problems start arising when devices that ub binds to and usb-storage 
doesn't (when ub is configured) fail to work with ub, or work much more 
slowly than they do with usb-storage.  And since some distributions enable 
ub in their shipping configurations, users often don't realize what has 
happened -- they only know that things don't work as well as they used to.

Maybe a reasonable answer would be to ask distributors not to enable ub,
and leave it up to individual users to configure it when they want to.  
That is the default setting in Kconfig, after all.

Alan Stern

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Alan Stern]

On Mon, 20 Dec 2004, Ed Tomlinson wrote:

> Its not that they just enable it.  Its that it has side effects.  I enable it to support
> one device - it then 'devnaps' other devices that usbstorage supports _much_
> better.  Is there some way it could work in reverse.  eg. let ub bind only if 
> usbstorage does not, possibly making usbstorage a _little_ more conservative
> if ub is present?

Unfortunately there isn't any way to define which driver should bind to a 
device, if they are both capable of controlling it.  Maybe there should 
be.  It might not be too hard to add a sysfs interface for that sort of 
thing.

Alan Stern

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Greg KH]

On Mon, Dec 20, 2004 at 10:28:05AM -0500, Alan Stern wrote:
> On Mon, 20 Dec 2004, Ed Tomlinson wrote:
> 
> > Its not that they just enable it.  Its that it has side effects.  I enable it to support
> > one device - it then 'devnaps' other devices that usbstorage supports _much_
> > better.  Is there some way it could work in reverse.  eg. let ub bind only if 
> > usbstorage does not, possibly making usbstorage a _little_ more conservative
> > if ub is present?
> 
> Unfortunately there isn't any way to define which driver should bind to a 
> device, if they are both capable of controlling it.  Maybe there should 
> be.  It might not be too hard to add a sysfs interface for that sort of 
> thing.

We are working on it...

thanks,

greg k-h

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[David Brownell]

On Sunday 19 December 2004 11:06 pm, Pete Zaitcev wrote:
> On Sun, 19 Dec 2004 22:43:05 -0800, David Brownell <david-b@pacbell.net> wrote:
> 
> > It also seems to mean significantly slower access (at high speed)
> > for the most standard devices.  That doesn't seem like a win,
> > though I suspect fixing it should be as simple as switching over
> > to use the USB scatterlist calls (which usb-storage uses) ...
> 
> They do not allow asynchronous operation, last time I checked.

You could add an async mode ... heck, even the hack of dedicating
a kernel thread to that task ("psuedo-async" within "ub") would
give much more reasonable throughput. 

- Dave

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Lee Revell]

On Mon, 2004-12-20 at 10:28 -0500, Alan Stern wrote:
> On Mon, 20 Dec 2004, Ed Tomlinson wrote:
> 
> > Its not that they just enable it.  Its that it has side effects.  I enable it to support
> > one device - it then 'devnaps' other devices that usbstorage supports _much_
> > better.  Is there some way it could work in reverse.  eg. let ub bind only if 
> > usbstorage does not, possibly making usbstorage a _little_ more conservative
> > if ub is present?
> 
> Unfortunately there isn't any way to define which driver should bind to a 
> device, if they are both capable of controlling it.  Maybe there should 
> be.  It might not be too hard to add a sysfs interface for that sort of 
> thing.

This is a neverending battle with ALSA and OSS modules claiming the same
device, resulting in bizarre behavior.  You could argue that it's user
or vendor error but that doesn't change the flood of bug reports on
alsa-user.

I am sure the ALSA developers would welcome a generic solution for this
problem.

Lee

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Rob Browning]

Pete Zaitcev <zaitcev@redhat.com> writes:

> Is it that bad, really? Honestly, I could not imagine users can be
> so dumb.  The option defaults to off. There is a warning in the
> Kconfig. And yet they first enable it and then complain about it. I
> don't know what to do about it, either.

Well, I presume you know this, but at least in 2.6.9, there's no
warning.  When I read it, it said:

   CONFIG_BLK_DEV_UB:

   This driver supports certain USB attached storage devices
   such as flash keys.

   If unsure, say N.

which sounded potentially useful, and certainly didn't give the
impression that the driver was likely to perform terribly in common
cases (i.e. when using an external drive).

The sample Kconfig warnings I saw posted later in this thread would
certainly have given enough information to know to avoid the driver,
though if true, this might be even clearer:

  Note: this driver does not coexist well with usb-storage, and
  usb-storage is is often the best driver for common devices like
  external drive enclosures.  At the moment, usb-storage may peform
  dramatically better for those devices.

  If you're not certain you need this driver, you should probably
  say 'N' here, and choose usb-storage instead.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4

Re: [linux-usb-devel] Re: RFC: [2.6 patch] let BLK_DEV_UB depend on EMBEDDED

[Theodore Ts'o]

On Wed, Dec 22, 2004 at 02:10:00AM -0600, Rob Browning wrote:
> The sample Kconfig warnings I saw posted later in this thread would
> certainly have given enough information to know to avoid the driver,
> though if true, this might be even clearer:
> 
>   Note: this driver does not coexist well with usb-storage, and
>   usb-storage is is often the best driver for common devices like
>   external drive enclosures.  At the moment, usb-storage may peform
>   dramatically better for those devices.
> 
>   If you're not certain you need this driver, you should probably
>   say 'N' here, and choose usb-storage instead.

The other caveat which is worth adding is that currently, the UB
device only supports a single LUN.  Some devices, most notably USB
readers that support multiple types of compact flash/secure
digital/smart media/et.al., and the PalmOne T5 PDA export multiple
LUN's.  

(I was scratching my head for a while trying to figure out why the T5
documentation claimed that you could access both the internal flash
memory as well as the Secure Digital external memory via the USB
interface until I realized it was because I was using the UB driver,
and it didn't support multiple LUN's.)

						- Ted

RFC: [2.6 patch] let BLK_DEV_UB depend on USB_STORAGE=n

[Adrian Bunk]

On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > I've already seen people crippling their usb-storage driver with 
> > enabling BLK_DEV_UB - and I doubt the warning in the help text added 
> > after 2.6.9 will fix all such problems.
> > 
> > Is there except for kernel size any good reason for using BLK_DEV_UB 
> > instead of USB_STORAGE?
> 
> You don't want to use the scsi layer?  You like the stability of it at
> times?  :)
> 
> > If not, I'd suggest the patch below to let BLK_DEV_UB depend
> > on EMBEDDED.
> 
> No, it's good for non-embedded boxes too.


My current understanding is:
- BLK_DEV_UB supports a subset of what USB_STORAGE can support
- for an average user, there's no reason to enable BLK_DEV_UB
- if you really know what you are doing, there might be several reasons
  why you might want to use BLK_DEV_UB


What about the patch below to let BLK_DEV_UB depend on USB_STORAGE=n?

If you really know what you are doing, you'll also know that you have to
set USB_STORAGE=n before you can enable BLK_DEV_UB.


diffstat output:
 drivers/block/Kconfig              |    2 +-
 drivers/usb/storage/unusual_devs.h |    2 --
 drivers/usb/storage/usb.c          |    4 ----
 3 files changed, 1 insertion(+), 7 deletions(-)


Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.10-rc3-mm1-full/drivers/block/Kconfig.old	2004-12-20 00:52:22.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/drivers/block/Kconfig	2004-12-23 03:09:14.000000000 +0100
@@ -303,7 +303,7 @@
 
 config BLK_DEV_UB
 	tristate "Low Performance USB Block driver"
-	depends on USB
+	depends on USB && USB_STORAGE=n
 	help
 	  This driver supports certain USB attached storage devices
 	  such as flash keys.
--- linux-2.6.10-rc3-mm1-full/drivers/usb/storage/unusual_devs.h.old	2004-12-23 03:09:42.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/drivers/usb/storage/unusual_devs.h	2004-12-23 03:09:53.000000000 +0100
@@ -549,13 +549,11 @@
 		US_SC_SCSI, US_PR_CB, NULL,
 		US_FL_SINGLE_LUN ),
 
-#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
 UNUSUAL_DEV(  0x0781, 0x0002, 0x0009, 0x0009, 
 		"Sandisk",
 		"ImageMate SDDR-31",
 		US_SC_DEVICE, US_PR_DEVICE, NULL,
 		US_FL_IGNORE_SER ),
-#endif
 
 UNUSUAL_DEV(  0x0781, 0x0100, 0x0100, 0x0100,
 		"Sandisk",
--- linux-2.6.10-rc3-mm1-full/drivers/usb/storage/usb.c.old	2004-12-23 03:10:00.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/drivers/usb/storage/usb.c	2004-12-23 03:10:13.000000000 +0100
@@ -144,9 +144,7 @@
 	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_QIC, US_PR_BULK) },
 	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_UFI, US_PR_BULK) },
 	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_8070, US_PR_BULK) },
-#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
 	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_SCSI, US_PR_BULK) },
-#endif
 
 	/* Terminating entry */
 	{ }
@@ -220,10 +218,8 @@
 	  .useTransport = US_PR_BULK},
 	{ .useProtocol = US_SC_8070,
 	  .useTransport = US_PR_BULK},
-#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
 	{ .useProtocol = US_SC_SCSI,
 	  .useTransport = US_PR_BULK},
-#endif
 
 	/* Terminating entry */
 	{ NULL }

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on USB_STORAGE=n

[Greg KH]

On Thu, Dec 23, 2004 at 03:40:31AM +0100, Adrian Bunk wrote:
> On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> > On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > > I've already seen people crippling their usb-storage driver with 
> > > enabling BLK_DEV_UB - and I doubt the warning in the help text added 
> > > after 2.6.9 will fix all such problems.
> > > 
> > > Is there except for kernel size any good reason for using BLK_DEV_UB 
> > > instead of USB_STORAGE?
> > 
> > You don't want to use the scsi layer?  You like the stability of it at
> > times?  :)
> > 
> > > If not, I'd suggest the patch below to let BLK_DEV_UB depend
> > > on EMBEDDED.
> > 
> > No, it's good for non-embedded boxes too.
> 
> 
> My current understanding is:
> - BLK_DEV_UB supports a subset of what USB_STORAGE can support
> - for an average user, there's no reason to enable BLK_DEV_UB
> - if you really know what you are doing, there might be several reasons
>   why you might want to use BLK_DEV_UB

I have been running with just the code portion of this patch for a while
now, with good results (no Kconfig changes.)

Pete and Matt, do you mind me applying the following portion of the
patch to the kernel tree?

thanks,

greg k-h
> --- linux-2.6.10-rc3-mm1-full/drivers/usb/storage/unusual_devs.h.old	2004-12-23 03:09:42.000000000 +0100
> +++ linux-2.6.10-rc3-mm1-full/drivers/usb/storage/unusual_devs.h	2004-12-23 03:09:53.000000000 +0100
> @@ -549,13 +549,11 @@
>  		US_SC_SCSI, US_PR_CB, NULL,
>  		US_FL_SINGLE_LUN ),
>  
> -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
>  UNUSUAL_DEV(  0x0781, 0x0002, 0x0009, 0x0009, 
>  		"Sandisk",
>  		"ImageMate SDDR-31",
>  		US_SC_DEVICE, US_PR_DEVICE, NULL,
>  		US_FL_IGNORE_SER ),
> -#endif
>  
>  UNUSUAL_DEV(  0x0781, 0x0100, 0x0100, 0x0100,
>  		"Sandisk",
> --- linux-2.6.10-rc3-mm1-full/drivers/usb/storage/usb.c.old	2004-12-23 03:10:00.000000000 +0100
> +++ linux-2.6.10-rc3-mm1-full/drivers/usb/storage/usb.c	2004-12-23 03:10:13.000000000 +0100
> @@ -144,9 +144,7 @@
>  	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_QIC, US_PR_BULK) },
>  	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_UFI, US_PR_BULK) },
>  	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_8070, US_PR_BULK) },
> -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
>  	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_SCSI, US_PR_BULK) },
> -#endif
>  
>  	/* Terminating entry */
>  	{ }
> @@ -220,10 +218,8 @@
>  	  .useTransport = US_PR_BULK},
>  	{ .useProtocol = US_SC_8070,
>  	  .useTransport = US_PR_BULK},
> -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
>  	{ .useProtocol = US_SC_SCSI,
>  	  .useTransport = US_PR_BULK},
> -#endif
>  
>  	/* Terminating entry */
>  	{ NULL }

-- 

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on USB_STORAGE=n

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]

On Wed, Jan 19, 2005 at 02:07:07PM -0800, Greg KH wrote:
> On Thu, Dec 23, 2004 at 03:40:31AM +0100, Adrian Bunk wrote:
> > On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> > > On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > > > I've already seen people crippling their usb-storage driver with 
> > > > enabling BLK_DEV_UB - and I doubt the warning in the help text added 
> > > > after 2.6.9 will fix all such problems.
> > > > 
> > > > Is there except for kernel size any good reason for using BLK_DEV_UB 
> > > > instead of USB_STORAGE?
> > > 
> > > You don't want to use the scsi layer?  You like the stability of it at
> > > times?  :)
> > > 
> > > > If not, I'd suggest the patch below to let BLK_DEV_UB depend
> > > > on EMBEDDED.
> > > 
> > > No, it's good for non-embedded boxes too.
> > 
> > 
> > My current understanding is:
> > - BLK_DEV_UB supports a subset of what USB_STORAGE can support
> > - for an average user, there's no reason to enable BLK_DEV_UB
> > - if you really know what you are doing, there might be several reasons
> >   why you might want to use BLK_DEV_UB
> 
> I have been running with just the code portion of this patch for a while
> now, with good results (no Kconfig changes.)
> 
> Pete and Matt, do you mind me applying the following portion of the
> patch to the kernel tree?

I have no objection.

Matt

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

E:  You run this ship with Windows?!  YOU IDIOT!
L:  Give me a break, it came bundled with the computer!
					-- ESR and Lan Solaris
User Friendly, 12/8/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on USB_STORAGE=n

[Greg KH]

On Wed, Jan 19, 2005 at 06:49:00PM -0800, Matthew Dharm wrote:
> On Wed, Jan 19, 2005 at 02:07:07PM -0800, Greg KH wrote:
> > On Thu, Dec 23, 2004 at 03:40:31AM +0100, Adrian Bunk wrote:
> > > On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> > > > On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > > > > I've already seen people crippling their usb-storage driver with 
> > > > > enabling BLK_DEV_UB - and I doubt the warning in the help text added 
> > > > > after 2.6.9 will fix all such problems.
> > > > > 
> > > > > Is there except for kernel size any good reason for using BLK_DEV_UB 
> > > > > instead of USB_STORAGE?
> > > > 
> > > > You don't want to use the scsi layer?  You like the stability of it at
> > > > times?  :)
> > > > 
> > > > > If not, I'd suggest the patch below to let BLK_DEV_UB depend
> > > > > on EMBEDDED.
> > > > 
> > > > No, it's good for non-embedded boxes too.
> > > 
> > > 
> > > My current understanding is:
> > > - BLK_DEV_UB supports a subset of what USB_STORAGE can support
> > > - for an average user, there's no reason to enable BLK_DEV_UB
> > > - if you really know what you are doing, there might be several reasons
> > >   why you might want to use BLK_DEV_UB
> > 
> > I have been running with just the code portion of this patch for a while
> > now, with good results (no Kconfig changes.)
> > 
> > Pete and Matt, do you mind me applying the following portion of the
> > patch to the kernel tree?
> 
> I have no objection.

Ok, I've commited the change to my trees, thanks.

greg k-h

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on USB_STORAGE=n

[David Woodhouse]

On Wed, 2005-01-19 at 14:07 -0800, Greg KH wrote:
> I have been running with just the code portion of this patch for a while
> now, with good results (no Kconfig changes.)
> 
> Pete and Matt, do you mind me applying the following portion of the
> patch to the kernel tree?
> 
> > -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
> >  UNUSUAL_DEV(  0x0781, 0x0002, 0x0009, 0x0009, 
> >  		"Sandisk",
> >  		"ImageMate SDDR-31",
> >  		US_SC_DEVICE, US_PR_DEVICE, NULL,
> >  		US_FL_IGNORE_SER ),
> > -#endif

Urgh. Please do. Code which may compile differently in the kernel image
according to which _modules_ are configured at the time is horrid, and
should be avoided.

-- 
dwmw2

Re: RFC: [2.6 patch] let BLK_DEV_UB depend on USB_STORAGE=n

[Pete Zaitcev]

On Mon, 24 Jan 2005 11:48:51 +0000, David Woodhouse <dwmw2@infradead.org> wrote:

> On Wed, 2005-01-19 at 14:07 -0800, Greg KH wrote:
> > I have been running with just the code portion of this patch for a while
> > now, with good results (no Kconfig changes.)
> > 
> > Pete and Matt, do you mind me applying the following portion of the
> > patch to the kernel tree?
> > 
> > > -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
> > >  UNUSUAL_DEV(  0x0781, 0x0002, 0x0009, 0x0009, 
> > >  		"Sandisk",
> > >  		"ImageMate SDDR-31",
> > >  		US_SC_DEVICE, US_PR_DEVICE, NULL,
> > >  		US_FL_IGNORE_SER ),
> > > -#endif
> 
> Urgh. Please do. Code which may compile differently in the kernel image
> according to which _modules_ are configured at the time is horrid, and
> should be avoided.

The fallacy of this "urgh" is easy to demonstrate when you consider usb-storage
and ub the one and the same driver. Initially, ub was just a mode for
usb-storage ("threadless"). I only factored them separate for reasons
of clarity. Horrid, indeed. There's no reason to build one statically
and another as a module.

Mind, I didn't disagree with the backout patch as such, but not because
it was a good idea, but because it may help to shut up a few stupid users
(provided that our scripts preserve the link order from drivers/usb/Makefile
in modules.usbmap, or have other way to make sure that usb-storage entries
are ahead of ub entires; did anyone actually check it? if those scripts
sort by name, ub still pops ahead, and the backout is utterly ineffectual).

When we reintroduce ub in Fedora, I'll just put this patch right back,
it's not a problem. But please think about this issue a little more,
you might want to take the Urgh back.

-- Pete