2006-09-07 23:13:44

by Kristen Carlson Accardi

[permalink] [raw]
Subject: patch [0/2]: acpi: add generic removable drive bay support

Hello,
Ever since I put out the docking station patches people have been asking me
if I could get removable drive bays to work as well. These are devices such
as the IBM/Lenovo Ultrabay, or the Dell Module Bay - basically removable disk
drives. There is an IBM platform specific solution that works with some IBM
laptops, but nothing generic. In addition, the ibm_acpi driver had the
limitation that it would not correctly identify ultrabay devices that had
been inserted after the OS booted. This driver implements bay handling
using ACPI to safely power off the drive. It works similarly
to the ibm_acpi driver, in that it will simply generate an acpi event which
can then be used by udev to unmount or rescan depending on the event. It will
create a proc entry under /proc/acpi/bay for "eject" and for "status". Writing
a 1 to the eject call will cause the drive bays acpi eject method to be called,
and should be done prior to removing the device. Reading the "status" entry
will tell you either "present" or "removed" depending on the state of the
device. User space scripts can use the status entry to determine if a device
has been either inserted or removed in the case of some laptops who generate
the exact same event for either insertion or removal (i.e. the Dell M65 for
example). Same scripts for using these events and udev can be found on the
thinkwiki website:

http://www.thinkwiki.org/wiki/How_to_hotswap_UltraBay_devices#When_using_the_ata
_piix_driver

Note that due to the lack of hotplug PATA, this driver will only work with
SATA devices, or with ata_piix driver. Hopefully if/when hotplug PATA support
gets into libata, we can support that as well.

Note that this driver does not have the same limitations that the ibm_acpi
driver had with regard to device insertion after booting. i.e. - if you boot
the laptop without a drive bay inserted, and then insert the drive bay later
on, this driver will still detect the insertion and send an event to user
space. The bay driver will also register with the dock station driver so that
if the removeable drive bay is on a dock station, an event will be sent to
userspace upon dock/undock that will allow any udev scripts specifically for
bay handling to still be executed.

The driver still needs more testing, and I still have a little bit of funkiness
to sort out with the Lenovo X60 with the SATA controller in compatibility
mode - but I think it is ready for you to experiment with if you like and
I hope that you will give it a try and send me any feedback you have.

patch 1 is just a bug fix to the dock driver that will allow devices who's
parents are listed as being on the dock to also be recognized as being on
a dock station. patch 2 is the driver.

Thanks,
Kristen


2006-09-08 01:34:21

by Nigel Cunningham

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

Hi Kristen.

Great to see this!

Allow me to anticipate a question I'm sure will come: will this play
well with (say) suspending while docked and resuming undocked?

Regards,

Nigel

2006-09-08 17:48:56

by Kristen Carlson Accardi

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

On Fri, 08 Sep 2006 11:34:16 +1000
Nigel Cunningham <[email protected]> wrote:

> Hi Kristen.
>
> Great to see this!
>
> Allow me to anticipate a question I'm sure will come: will this play
> well with (say) suspending while docked and resuming undocked?
>

Hi Nigel,
I don't know. the few experiments that I've tried with suspend/resume are
not inspiring. I don't think that suspend/resume works at all with the
dock station ATM. "warm plug" is something that is on my list of things
to work on after hot plug actually works.

Kristen

2006-09-08 20:03:20

by Matthew Garrett

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

On Thu, Sep 07, 2006 at 04:13:05PM -0700, Kristen Carlson Accardi wrote:

Firstly, thanks for this - I wrote some related code a while ago. A
couple of questions...

> can then be used by udev to unmount or rescan depending on the event. It will
> create a proc entry under /proc/acpi/bay for "eject" and for "status". Writing

Do we really want it under /proc? It would seem to make more sense for
it to be under /sys.

> a 1 to the eject call will cause the drive bays acpi eject method to be called,
> and should be done prior to removing the device. Reading the "status" entry
> will tell you either "present" or "removed" depending on the state of the
> device. User space scripts can use the status entry to determine if a device
> has been either inserted or removed in the case of some laptops who generate
> the exact same event for either insertion or removal (i.e. the Dell M65 for
> example). Same scripts for using these events and udev can be found on the
> thinkwiki website:

What gets generated if I rip a drive out without notifying the system
beforehand? Dell hardware doesn't appear to send any event when poking
the eject lever.

The way I originally implemented this was to tie it into the libata
layer directly. With a small amount of glue it's possible to tie a
hotswap bay to a specific sata port, and use the event to trigger a
hotplug rescan. Doing it in kernel space means that the device can be
destroyed the moment it's removed, reducing the possibility that further
writes will be made. There was some amount of resistance to acpi
integration in the scsi layer, though I think we eventually reached a
compromise about providing support for platform firmware hooks.

Doing it this way also means that the bay itself can be represented in
sysfs as an attribute under the appropriate port. That seems more
natural than having the bay be entirely separate, despite ACPI providing
us with the information for them to be tied together.

--
Matthew Garrett | [email protected]

2006-09-08 20:04:03

by Matthew Garrett

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

On Fri, Sep 08, 2006 at 11:34:16AM +1000, Nigel Cunningham wrote:

> Allow me to anticipate a question I'm sure will come: will this play
> well with (say) suspending while docked and resuming undocked?

Most hardware seems to send a bay event on resume.

--
Matthew Garrett | [email protected]

2006-09-08 20:21:42

by Kristen Carlson Accardi

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

On Fri, 8 Sep 2006 20:58:42 +0100
Matthew Garrett <[email protected]> wrote:

> On Thu, Sep 07, 2006 at 04:13:05PM -0700, Kristen Carlson Accardi wrote:
>
> Firstly, thanks for this - I wrote some related code a while ago. A
> couple of questions...
>
> > can then be used by udev to unmount or rescan depending on the event. It will
> > create a proc entry under /proc/acpi/bay for "eject" and for "status". Writing
>
> Do we really want it under /proc? It would seem to make more sense for
> it to be under /sys.

I agree - this is under proc because this is an acpi driver, and the acpi
subsystem is still using the /proc fs for driver/user space interface. I
thought I would just conform to their standard.

>
> > a 1 to the eject call will cause the drive bays acpi eject method to be called,
> > and should be done prior to removing the device. Reading the "status" entry
> > will tell you either "present" or "removed" depending on the state of the
> > device. User space scripts can use the status entry to determine if a device
> > has been either inserted or removed in the case of some laptops who generate
> > the exact same event for either insertion or removal (i.e. the Dell M65 for
> > example). Same scripts for using these events and udev can be found on the
> > thinkwiki website:
>
> What gets generated if I rip a drive out without notifying the system
> beforehand? Dell hardware doesn't appear to send any event when poking
> the eject lever.

I tested the Dell M65 with this patch, and you are right - it does not
send an event when you press the eject button, but rather when you do the
actual remove. I sent a mail to their BIOS people informing them that we
would find it helpful to have the eject request as well as the removal event,
but, I am not holding my breath. The remove event seems to be "good enough",
although I can definitely forsee issues.

>
> The way I originally implemented this was to tie it into the libata
> layer directly. With a small amount of glue it's possible to tie a
> hotswap bay to a specific sata port, and use the event to trigger a
> hotplug rescan. Doing it in kernel space means that the device can be
> destroyed the moment it's removed, reducing the possibility that further
> writes will be made. There was some amount of resistance to acpi
> integration in the scsi layer, though I think we eventually reached a
> compromise about providing support for platform firmware hooks.

This is the way I was planning on doing it too - and I'd like to do that
for SATA - unfortunately, a lot of the removable drives are actually
PATA, and until hotplug support for this gets integrated into libata,
as a workaround we have to tell userspace to try to unmount the filesystem
first, otherwise the whole system locks up. Even new laptops such as the
Lenovo X60 have IDE bays on them.

I think that it would be appropriate and possible to determine if we
were a SATA removable drive from the bay driver, and then if so -
call into the scsi layer (or vice versa) to do the remove completely
in kernel - this is the best way in my opinion anyway since ATM the
eject event is triggered when the user removes the drive, which means
that if it takes a lot of time to trigger the userspace scripts, you
can wind up with problems.

>
> Doing it this way also means that the bay itself can be represented in
> sysfs as an attribute under the appropriate port. That seems more
> natural than having the bay be entirely separate, despite ACPI providing
> us with the information for them to be tied together.
>
> --
> Matthew Garrett | [email protected]

So what are the firmware hooks that you speak of? I would love to have
this represented under the appropriate scsi layer (assuming SATA) if
possible - it seems more natural to me also.

2006-09-08 20:26:10

by Dave Jones

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

On Fri, Sep 08, 2006 at 01:21:23PM -0700, Kristen Carlson Accardi wrote:
> On Fri, 8 Sep 2006 20:58:42 +0100
> Matthew Garrett <[email protected]> wrote:
>
> > On Thu, Sep 07, 2006 at 04:13:05PM -0700, Kristen Carlson Accardi wrote:
> >
> > Firstly, thanks for this - I wrote some related code a while ago. A
> > couple of questions...
> >
> > > can then be used by udev to unmount or rescan depending on the event. It will
> > > create a proc entry under /proc/acpi/bay for "eject" and for "status". Writing
> >
> > Do we really want it under /proc? It would seem to make more sense for
> > it to be under /sys.
>
> I agree - this is under proc because this is an acpi driver, and the acpi
> subsystem is still using the /proc fs for driver/user space interface. I
> thought I would just conform to their standard.

It's my understanding from talking with Len that he'd like to see /proc/acpi/
go away over time, so adding more to it seems to be at odds with that goal.

Dave

2006-09-08 20:47:17

by Matthew Garrett

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

On Fri, Sep 08, 2006 at 01:21:23PM -0700, Kristen Carlson Accardi wrote:
> On Fri, 8 Sep 2006 20:58:42 +0100
> Matthew Garrett <[email protected]> wrote:
> > Do we really want it under /proc? It would seem to make more sense for
> > it to be under /sys.
>
> I agree - this is under proc because this is an acpi driver, and the acpi
> subsystem is still using the /proc fs for driver/user space interface. I
> thought I would just conform to their standard.

Yeah, that's the current situation. I think pretty much everyone would
like to see it die off, though :)

> > What gets generated if I rip a drive out without notifying the system
> > beforehand? Dell hardware doesn't appear to send any event when poking
> > the eject lever.
>
> I tested the Dell M65 with this patch, and you are right - it does not
> send an event when you press the eject button, but rather when you do the
> actual remove. I sent a mail to their BIOS people informing them that we
> would find it helpful to have the eject request as well as the removal event,
> but, I am not holding my breath. The remove event seems to be "good enough",
> although I can definitely forsee issues.

Right. My prime concern in this case is that userspace ends up seeing
the drive for a significant period of time despite it having been
removed.

> This is the way I was planning on doing it too - and I'd like to do that
> for SATA - unfortunately, a lot of the removable drives are actually
> PATA, and until hotplug support for this gets integrated into libata,
> as a workaround we have to tell userspace to try to unmount the filesystem
> first, otherwise the whole system locks up. Even new laptops such as the
> Lenovo X60 have IDE bays on them.

Really? Ouch. I did hack up some vague support for pata hotswap some
time ago, but dropped it when I realised that there wasn't a terribly
easy way to remove a single device in drivers/ide. Removing an entire
bus isn't helpful when your test machine has the hard drive and CD drive
on the same channel.

> So what are the firmware hooks that you speak of? I would love to have
> this represented under the appropriate scsi layer (assuming SATA) if
> possible - it seems more natural to me also.

The idea was to add something to init_scsi in drivers/scsi/scsi.c along
the lines of scsi_platform_register(). On most platforms this would be a
noop, but on ACPI systems it would do something like pci_acpi_init does
in drivers/pci/pci-acpi.c. That way when SCSI devices (including libata
ones) get registered, there's a callback into the scsi-acpi code which
can associate them with the appropriate ACPI address. When an event is
received that can then be handled by the scsi-acpi code, which has the
corresponding SCSI device structure and can call the relevant hotplug
code.

The added advantage of this is that it would allow fairly clean
integration of the ACPI suspend/resume methods for PATA and SATA
devices, plus any further information that ever gets exposed via ACPI.

http://lkml.org/lkml/2005/12/8/152 is the relevant part of the
discussion, though the patch appears somewhere earlier in the thread. As
Jeff mentions the SCSI layer may not be the ideal place to do this,
though I haven't checked whether it's straightforward to do it in libata
itself yet. Even if not, it wouldn't be hard to swap the code over when
necessary.

--
Matthew Garrett | [email protected]

2006-09-09 14:52:37

by Alan

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

Ar Iau, 2006-09-07 am 16:13 -0700, ysgrifennodd Kristen Carlson Accardi:
> the exact same event for either insertion or removal (i.e. the Dell M65 for
> example). Same scripts for using these events and udev can be found on the
> thinkwiki website:
>
> http://www.thinkwiki.org/wiki/How_to_hotswap_UltraBay_devices#When_using_the_ata
> _piix_driver

drivers/ide/piix does not support hotplug of any kind. drivers/ide does
not support hotplug of any kind. There is an ioctl hack that vaguely
works now and then for some interfaces but it too is completely unsafe
in 2.6 except for RHEL4. If people keep trying to post suggestions to
use them for anything but debugging I'm going to have to send Andrew
patches to remove them entirely before more people lose their data.

RHEL4 has a set of locking changes/patches to support some basic hotplug
cases, they were refused upstream so upstream simply doesn't support it.

Alan

2006-10-14 05:12:20

by Brown, Len

[permalink] [raw]
Subject: Re: patch [0/2]: acpi: add generic removable drive bay support

On Friday 08 September 2006 16:33, Dave Jones wrote:
> On Fri, Sep 08, 2006 at 01:21:23PM -0700, Kristen Carlson Accardi wrote:
> > On Fri, 8 Sep 2006 20:58:42 +0100
> > Matthew Garrett <[email protected]> wrote:
> >
> > > > can then be used by udev to unmount or rescan depending on the event. It will
> > > > create a proc entry under /proc/acpi/bay for "eject" and for "status". Writing
> > >
> > > Do we really want it under /proc? It would seem to make more sense for
> > > it to be under /sys.
> >
> > I agree - this is under proc because this is an acpi driver, and the acpi
> > subsystem is still using the /proc fs for driver/user space interface. I
> > thought I would just conform to their standard.
>
> It's my understanding from talking with Len that he'd like to see /proc/acpi/
> go away over time, so adding more to it seems to be at odds with that goal.

Dave is right. We've had a moratorium on new files under /proc/acpi for some time now.
The reason is that user-space should not know or care that something is supplied
by ACPI -- for on other systems it may be supplied by something else.

So new stuff should have generic names under sys -- even if on a large body of
systems the functionality beneath happens to be supplied via ACPI.

an example of old is the brightness stuff under /proc/acpi and in various platform
specific drivers that scribble under /proc/acpi.
The corresponding example of new is the backlight I/F under /sys.

thanks,
-Len