From: Toshi Kani <toshi.kani@hp.com>
To: Hanjun Guo <guohanjun@huawei.com>
Cc: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>,
linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com,
wency@cn.fujitsu.com, rjw@sisk.pl, lenb@kernel.org,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, Tang Chen <tangchen@cn.fujitsu.com>,
Liujiang <jiang.liu@huawei.com>, Huxinwei <huxinwei@huawei.com>
Subject: Re: [RFC PATCH v3 0/3] acpi: Introduce prepare_remove device operation
Date: Tue, 04 Dec 2012 16:23:31 -0700 [thread overview]
Message-ID: <1354663411.21585.135.camel@misato.fc.hp.com> (raw)
In-Reply-To: <50BDBF5A.8040407@huawei.com>
On Tue, 2012-12-04 at 17:16 +0800, Hanjun Guo wrote:
> On 2012/12/4 8:10, Toshi Kani wrote:
> > On Mon, 2012-12-03 at 12:25 +0800, Hanjun Guo wrote:
> >> On 2012/11/30 6:27, Toshi Kani wrote:
> >>> On Thu, 2012-11-29 at 12:48 +0800, Hanjun Guo wrote:
> >>>> On 2012/11/29 2:41, Toshi Kani wrote:
> >>>>> On Wed, 2012-11-28 at 19:05 +0800, Hanjun Guo wrote:
> >>>>>> On 2012/11/24 1:50, Vasilis Liaskovitis wrote:
> >>>>>> As you may know, the ACPI based hotplug framework we are working on already addressed
> >>>>>> this problem, and the way we slove this problem is a bit like yours.
> >>>>>>
> >>>>>> We introduce hp_ops in struct acpi_device_ops:
> >>>>>> struct acpi_device_ops {
> >>>>>> acpi_op_add add;
> >>>>>> acpi_op_remove remove;
> >>>>>> acpi_op_start start;
> >>>>>> acpi_op_bind bind;
> >>>>>> acpi_op_unbind unbind;
> >>>>>> acpi_op_notify notify;
> >>>>>> #ifdef CONFIG_ACPI_HOTPLUG
> >>>>>> struct acpihp_dev_ops *hp_ops;
> >>>>>> #endif /* CONFIG_ACPI_HOTPLUG */
> >>>>>> };
> >>>>>>
> >>>>>> in hp_ops, we divide the prepare_remove into six small steps, that is:
> >>>>>> 1) pre_release(): optional step to mark device going to be removed/busy
> >>>>>> 2) release(): reclaim device from running system
> >>>>>> 3) post_release(): rollback if cancelled by user or error happened
> >>>>>> 4) pre_unconfigure(): optional step to solve possible dependency issue
> >>>>>> 5) unconfigure(): remove devices from running system
> >>>>>> 6) post_unconfigure(): free resources used by devices
> >>>>>>
> >>>>>> In this way, we can easily rollback if error happens.
> >>>>>> How do you think of this solution, any suggestion ? I think we can achieve
> >>>>>> a better way for sharing ideas. :)
> >>>>>
> >>>>> Yes, sharing idea is good. :) I do not know if we need all 6 steps (I
> >>>>> have not looked at all your changes yet..), but in my mind, a hot-plug
> >>>>> operation should be composed with the following 3 phases.
> >>>>
> >>>> Good idea ! we also implement a hot-plug operation in 3 phases:
> >>>> 1) acpihp_drv_pre_execute
> >>>> 2) acpihp_drv_execute
> >>>> 3) acpihp_drv_post_execute
> >>>> you may refer to :
> >>>> https://lkml.org/lkml/2012/11/4/79
> >>>
> >>> Great. Yes, I will take a look.
> >>
> >> Thanks, any comments are welcomed :)
> >
> > If I read the code right, the framework calls ACPI drivers differently
> > at boot-time and hot-add as follows. That is, the new entry points are
> > called at hot-add only, but .add() is called at both cases. This
> > requires .add() to work differently.
>
> Hi Toshi,
> Thanks for your comments!
>
> >
> > Boot : .add()
>
> Actually, at boot time: .add(), .start()
Right.
> > Hot-Add : .add(), .pre_configure(), configure(), etc.
>
> Yes, we did it as you said in the framework. We use .pre_configure(), configure(),
> and post_configure() to instead of .start() for better error handling and recovery.
I think we should have hot-plug interfaces at the module level, not at
the ACPI-internal level. In this way, the interfaces can be
platform-neutral and allow any modules to register, which makes it more
consistent with the boot-up sequence. It can also allow ordering of the
sequence among the registered modules. Right now, we initiate all
procedures from ACPI during hot-plug, which I think is inflexible and
steps into other module's role.
I am also concerned about the slot handling, which is the core piece of
the infrastructure and only allows hot-plug operations on ACPI objects
where slot objects are previously created by checking _EJ0. The
infrastructure should allow hot-plug operations on any objects, and it
should not be dependent on the slot design.
I have some rough idea, and it may be easier to review / explain if I
make some code changes. So, let me prototype it, and send it you all if
that works out. Hopefully, it won't take too long.
> > I think the boot-time and hot-add initialization should be done
> > consistently. While there is difficulty with the current boot sequence,
> > the framework should be designed to allow them consistent, not make them
> > diverged.
> >
> >>>>> 1. Validate phase - Verify if the request is a supported operation. All
> >>>>> known restrictions are verified at this phase. For instance, if a
> >>>>> hot-remove request involves kernel memory, it is failed in this phase.
> >>>>> Since this phase makes no change, no rollback is necessary to fail.
> >>>>
> >>>> Yes, we have done this in acpihp_drv_pre_execute, and check following things:
> >>>>
> >>>> 1) Hot-plugble or not. the instance kernel memory you mentioned is also checked
> >>>> when memory device remove;
> >>>
> >>> Agreed.
> >>>
> >>>> 2) Dependency check involved. For instance, if hot-add a memory device,
> >>>> processor should be added first, otherwise it's not valid to this operation.
> >>>
> >>> I think FW should be the one that assures such dependency. That is,
> >>> when a memory device object is marked as present/enabled/functioning, it
> >>> should be ready for the OS to use.
> >>
> >> Yes, BIOS should do something for the dependency, because BIOS knows the
> >> actual hardware topology.
> >
> > Right.
> >
> >> The ACPI specification provides _EDL method to
> >> tell OS the eject device list, but still has no method to tell OS the add device
> >> list now.
> >
> > Yes, but I do not think the OS needs special handling for add...
>
> Hmm, how about trigger a hot add operation by OS ? we have eject interface for OS, but
> have no add interface now, do you think this feature is useful? If it is, I think OS
> should analyze the dependency first and tell the user.
The OS can eject an ACPI device because a target device is owned by the
OS (i.e. enabled). For hot-add, a target ACPI device is not owned by
the OS (i.e. disabled). Therefore, the OS is not supposed to change its
state. So, I do not think we should support a hot-add operation by the
OS.
> >> For some cases, OS should analyze the dependency in the validate phase. For example,
> >> when hot remove a node (container device), OS should analyze the dependency to get
> >> the remove order as following:
> >> 1) Host bridge;
> >> 2) Memory devices;
> >> 3) Processor devices;
> >> 4) Container device itself;
> >
> > This may be off-topic, but how do you plan to delete I/O devices under a
> > node? Are you planning to delete all I/O devices along with the node?
>
> Yes, we delete all I/O devices under the node. we delete I/O devices as
> following steps:
> 1) Offline PCI devices;
> 2) Offline IOAPIC and IOMMU;
> and offline I/O devices no matter in use or not.
Oh, off-lining no matter what would be problematic for enterprise
customers...
> > On other OS, we made a separate step called I/O chassis delete, which
> > off-lines all I/O devices under the node, and is required before a node
> > hot-remove. It basically triggers PCIe hot-remove to detach drivers
> > from all devices. It does not eject the devices so that they do not
> > have to be on hot-plug slots. This step runs user-space scripts to
> > verify if the devices can be off-lined without disrupting user's
> > applications, and provides comprehensive reports if any of them are in
>
> Great! we also have a plan to implement this feature.
That's great!
> > use. Not sure if Linux's PCI hot-remove has such check, but I thought
> > I'd mention it. :)
>
> Have no such check, I'm sure :)
>
> >
> >> In this way, we can check that all the devices are hot-plugble or not under the
> >> container device before execute phase, and further more, we can remove devices
> >> in order to avoid some crash problems.
> >
> > Yes, we should check if all the resources under the node can be
> > off-lined at validate phase. (note, all the devices do not have to have
> > _EJ0 if that's what you meant by hot-pluggable.)
>
> Yes, agreed. For node hotplug, no need for all the devices have _EJ0 method.
Right.
Thanks,
-Toshi
next prev parent reply other threads:[~2012-12-04 23:32 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-23 17:50 [RFC PATCH v3 0/3] acpi: Introduce prepare_remove device operation Vasilis Liaskovitis
2012-11-23 17:50 ` [RFC PATCH v3 1/3] acpi: Introduce prepare_remove operation in acpi_device_ops Vasilis Liaskovitis
2012-11-27 0:10 ` Toshi Kani
2012-11-27 18:36 ` Vasilis Liaskovitis
2012-11-27 23:18 ` Rafael J. Wysocki
2012-11-23 17:50 ` [RFC PATCH v3 2/3] acpi_memhotplug: Add prepare_remove operation Vasilis Liaskovitis
2012-11-24 16:23 ` Wen Congyang
2012-11-23 17:50 ` [RFC PATCH v3 3/3] acpi_memhotplug: Allow eject to proceed on rebind scenario Vasilis Liaskovitis
2012-11-24 16:20 ` Wen Congyang
2012-11-26 8:36 ` Vasilis Liaskovitis
2012-11-26 9:11 ` Wen Congyang
2012-11-27 0:19 ` Toshi Kani
2012-11-27 18:32 ` Vasilis Liaskovitis
2012-11-27 22:03 ` Toshi Kani
2012-11-27 23:41 ` Rafael J. Wysocki
2012-11-28 16:01 ` Toshi Kani
2012-11-28 18:40 ` Rafael J. Wysocki
2012-11-28 21:02 ` Toshi Kani
2012-11-28 21:40 ` Rafael J. Wysocki
2012-11-28 21:40 ` Toshi Kani
2012-11-28 22:01 ` Rafael J. Wysocki
2012-11-28 22:04 ` Toshi Kani
2012-11-28 22:21 ` Rafael J. Wysocki
2012-11-28 22:16 ` Toshi Kani
2012-11-28 22:39 ` Rafael J. Wysocki
2012-11-28 22:46 ` Greg KH
2012-11-28 23:05 ` Rafael J. Wysocki
2012-11-28 23:10 ` Greg KH
2012-11-28 23:31 ` Rafael J. Wysocki
2012-11-28 23:49 ` Rafael J. Wysocki
2012-11-29 1:02 ` Toshi Kani
2012-11-29 1:15 ` Toshi Kani
2012-11-29 10:03 ` Rafael J. Wysocki
2012-11-29 11:30 ` Vasilis Liaskovitis
2012-11-29 16:57 ` Rafael J. Wysocki
2012-11-29 17:56 ` Toshi Kani
2012-11-29 20:25 ` Rafael J. Wysocki
2012-11-29 20:38 ` Toshi Kani
2012-11-29 21:23 ` Rafael J. Wysocki
2012-11-29 21:46 ` Toshi Kani
2012-11-29 22:11 ` Rafael J. Wysocki
2012-11-29 23:17 ` Toshi Kani
2012-11-30 0:13 ` Rafael J. Wysocki
2012-11-30 1:09 ` Toshi Kani
2012-11-29 16:43 ` Toshi Kani
2012-11-29 11:04 ` Vasilis Liaskovitis
2012-11-29 17:44 ` Toshi Kani
2012-12-06 9:30 ` Vasilis Liaskovitis
2012-12-06 12:50 ` Rafael J. Wysocki
2012-12-06 15:41 ` Toshi Kani
2012-12-06 20:32 ` Rafael J. Wysocki
2012-11-28 11:05 ` [RFC PATCH v3 0/3] acpi: Introduce prepare_remove device operation Hanjun Guo
2012-11-28 18:41 ` Toshi Kani
2012-11-29 4:48 ` Hanjun Guo
2012-11-29 22:27 ` Toshi Kani
2012-12-03 4:25 ` Hanjun Guo
2012-12-04 0:10 ` Toshi Kani
2012-12-04 9:16 ` Hanjun Guo
2012-12-04 23:23 ` Toshi Kani [this message]
2012-12-05 12:10 ` Hanjun Guo
2012-12-05 22:31 ` Toshi Kani
2012-12-06 16:47 ` Jiang Liu
2012-12-07 2:25 ` Toshi Kani
2012-12-06 16:40 ` Jiang Liu
2012-12-06 20:30 ` Rafael J. Wysocki
2012-12-07 2:57 ` Toshi Kani
2012-12-07 5:57 ` Jiang Liu
2012-12-08 1:08 ` Toshi Kani
2012-12-11 14:34 ` Jiang Liu
2012-12-13 14:42 ` Toshi Kani
2012-12-13 15:15 ` Jiang Liu
2012-12-15 1:19 ` Toshi Kani
2012-11-29 10:15 ` Rafael J. Wysocki
2012-11-29 11:36 ` Vasilis Liaskovitis
2012-12-06 16:59 ` Jiang Liu
2012-11-29 17:03 ` Toshi Kani
2012-11-29 20:30 ` Rafael J. Wysocki
2012-11-29 20:39 ` Toshi Kani
2012-11-29 20:56 ` Toshi Kani
2012-11-29 21:25 ` Rafael J. Wysocki
2012-12-06 17:10 ` Jiang Liu
2012-12-06 17:07 ` Jiang Liu
2012-12-06 17:01 ` Jiang Liu
2012-12-06 16:56 ` Jiang Liu
2012-12-06 16:00 ` Jiang Liu
2012-12-06 16:03 ` Toshi Kani
2012-12-06 16:25 ` Jiang Liu
2012-12-06 16:31 ` Toshi Kani
2012-12-06 16:52 ` Jiang Liu
2012-12-06 17:09 ` Toshi Kani
2012-12-06 17:30 ` Jiang Liu
2012-12-06 17:28 ` Toshi Kani
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=1354663411.21585.135.camel@misato.fc.hp.com \
--to=toshi.kani@hp.com \
--cc=gregkh@linuxfoundation.org \
--cc=guohanjun@huawei.com \
--cc=huxinwei@huawei.com \
--cc=isimatu.yasuaki@jp.fujitsu.com \
--cc=jiang.liu@huawei.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=rjw@sisk.pl \
--cc=tangchen@cn.fujitsu.com \
--cc=vasilis.liaskovitis@profitbricks.com \
--cc=wency@cn.fujitsu.com \
/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;
as well as URLs for NNTP newsgroup(s).