Re: [RFC PATCH v3 3/3] acpi_memhotplug: Allow eject to proceed onrebind scenario

From: Toshi Kani
Date: Thu Nov 29 2012 - 20:17:55 EST


On Fri, 2012-11-30 at 01:13 +0100, Rafael J. Wysocki wrote:
> On Thursday, November 29, 2012 04:17:19 PM Toshi Kani wrote:
> > On Thu, 2012-11-29 at 23:11 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, November 29, 2012 02:46:44 PM Toshi Kani wrote:
> > > > On Thu, 2012-11-29 at 22:23 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, November 29, 2012 01:38:39 PM Toshi Kani wrote:
> > > > > Now, that need not harm functionality, and that's why I proposed the
> > > > > eject_forbidden flag, so that .remove() can say "I'm not done, please
> > > > > rollback", in which case the device can happily function going forward,
> > > > > even if we don't rebind the driver to it.
> > > >
> > > > A partially trimmed acpi_device is hard to rollback. acpi_device should
> > > > be either trimmed completely or intact.
> > >
> > > I may or may not agree, depending on what you mean by "trimmed". :-)
> > >
> > > > When a function failed to trim
> > > > an acpi_device, it needs to rollback its operation for the device before
> > > > returning an error.
> > >
> > > Unless it is .remove(), because .remove() is supposed to always succeed
> > > (ie. unbind the driver from the device). However, it may signal the caller
> > > that something's fishy, by setting a flag in the device object, for example.
> >
> > Right, .remove() cannot fail. We still need to check if we should
> > continue to use .remove(), though.
> >
> > As for the flag, are you thinking that we call acpi_bus_trim() with
> > rmdevice false first, so that it won't remove acpi_device?
>
> I'm not sure if that's going to help.
>
> Definitely, .remove() should just unbind the driver from the device.
> That's what it's supposed to do. Still, it may leave some information for
> the caller in the device structure itself. For example, "I have unbound
> from the device, but it is not safe to remove it physically".

Right.

> I'm now thinking that we may need to rework the trimming so that
> .remove() is called for all drivers first and the struct acpi_device
> objects are not removed at this stage. Then, if .remove() from one
> driver signals the situation like above, the routine will have to
> rebind the drivers that have been unbound and we're done.
>
> After that stage, when all drivers have been unbound, we should be
> able to go for full eject. First, we can drop all struct acpi_device
> objects in the relevant subtree and then we can run _EJ0.

I agree that such approach is worth pursuing.

> > > > This is because only the failed function has enough
> > > > context to rollback when an error occurred in the middle of its
> > > > procedure.
> > >
> > > Not really. If it actually removes the struct acpi_device then the caller
> > > may run acpi_bus_scan() on that device if necessary. There may be a problem
> > > if the device has an associated physical node (or more of them), but that
> > > requires special care anyway.
> >
> > Well, hot-remove to a device fails when there is a reason to fail. IOW,
> > such reason prevented the device to be removed safely. So, I think we
> > need to put it back to the original state in this case. Removing it by
> > ignoring the cause of failure sounds unsafe to me. Some status/data may
> > be left un-deleted as a result.
>
> Again, I may or may not agree with that, depending on whether you're talking
> about physical devices or about struct acpi_device objects.

Sorry, by "hot-remove a device", I was referring removing struct
acpi_device and off-lining its resource. By "left un-delted", I was
referring its resource left un-deleted, such as memory ranges.

> Anyway, I agree that removing struct acpi_device objects may not be worth the
> effort if we're going to re-create them in a while, because that may be costly.

Thanks,
-Toshi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/