#4785 HIGH Future : hal should not poll usb storage devices every 2s

[Zarro Boogs per Child]

#4785: hal should not poll usb storage devices every 2s
-----------------------+----------------------------------------------------
  Reporter:  dilinger  |       Owner:  jg            
      Type:  defect    |      Status:  new           
  Priority:  high      |   Milestone:  Future Release
 Component:  distro    |     Version:                
Resolution:            |    Keywords:  power         
  Verified:  0         |  
-----------------------+----------------------------------------------------

Comment(by davidz):

 Replying to [comment:6 dilinger]:
 > Replying to [comment:3 davidz]:
 > > > Yikes, talk about a waste.. Hald currently polls usb-storage
 > > > devices every 2 seconds to see if they still exist. This
 > > > seems completely unnecessary, as we can send an event from the
 > > > kernel when a device disappears.
 > >
 > > This is completely inaccurate and linking to random rants won't make
 it more true. May I recommend reading the manual page for hal-disable-
 polling(1)? If you don't have it on your system here's a semi-readable
 copy
 >
 > How is it inaccurate?  Enable CONFIG_USB_STORAGE_DEBUG in the kernel and
 watch as hal polls a usb stick every 2s.  I see nowhere below where you
 say that isn't the case.

 It's inaccurate in the sense that the sole reason we poll is for detecting
 _media removal_, not _device removal_. It would be non-sense to poll for
 device removal as the kernel already delivers such events. The other part
 is that you're saying it's a waste. Without _something_ actually doing the
 polling the kernel will not notice if you insert/remove media.

 > Ok, no need to argue about usb storage devices, then; hal should
 obviously *not* be polling them, as their removal will trigger block
 device removal.  The scenario that hal is working around (cdrom devices)
 is where block device stick around even when the media is removed.  The
 course of action that *I* would take would be to add some sort of
 userspace callback to cdrom_media_changed in the kernel, or to one of its
 friends.  However, assuming we don't want to change the kernel (or we want
 to maintain compatibility w/ older kernels), is there really no way to
 distinguish between a cdrom and every other usb device?  When hal first
 sees the device, doing an ioctl to see if one of the CDROM ioctls (ie,
 CDROM_DRIVE_STATUS) are supported seems like a fairly reliable check.

 The problem is that even the kernel don't know when the media is removed
 except for SATA AN (they didn't add all that just for fun). Also keep in
 mind that USB is a host-initiated protocol; the device itself cannot
 initiate an event.

 Also, you seem to be focused on optical drives; keep in mind there are
 lots of USB card readers around; presumably USB card readers are a lot
 more prevalent than optical drives these days :-)

 Btw, there's talk on LKML of having the kernel itself poll the device and
 just deliver "change" events via netlink/udev. That way we can unify a lot
 of code in user space; e.g. no separate paths for SATA AN vs. non-SATA AN.
 It'll probably save some power but at the end of the day it's just a
 kernel thread having to wake up every two seconds instead of us initiating
 it from user space with the syscall and context overhead that implies.

 > So if doing this check is pointless due to the majority of usb hardware
 out there.. Why are we doing the check?  Why don't we switch to a check
 that actually makes sense?  Hacking around crappy hardware is what Hal and
 the kernel are supposed to be doing..

 The problem is that the hardware _itself_ reports that it supports
 removable storage. There's no chance besides whitelists that you can ever
 work around this. And hardware that isn't SATA AN simply don't deliver the
 events.

 > Thanks for the explanation.  Unfortunately, I'm not convinced that XML
 blob is granular enough for OLPC; I can look into a kernel-side solution,
 or can come up w/ a better test of removable media.

 I'm not sure how you would do this; the problem is that a) the hardware
 itself lies about that it supports removable media; and b) the hardware
 don't report media change.

 So the only course of action is a white-list of known devices that don't
 support removable media but report it regardless. Do you agree?

 You can do such a white list via the XML blob similar to how we tag card
 readers

 http://gitweb.freedesktop.org/?p=hal-
 info.git;a=blob;hb=HEAD;f=fdi/information/10freedesktop/10-usb-card-
 readers.fdi

 to get the right icon on the desktop. You can also put this white-list
 somewhere in the kernel (drivers/usb/storage/unusual_devs.h for example)
 but from where I'm sitting it's easier to maintain such quirk lists in
 user space. YMMV.

 The problem, however, is that there are thousands or tens of thousands USB
 devices out there so the whitelist would be really big. It's not really
 realistic I think...

-- 
Ticket URL: <http://dev.laptop.org/ticket/4785#comment:7>
One Laptop Per Child <http://dev.laptop.org>
OLPC bug tracking system