Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1625755 > unrolled thread

Re: [PATCH] ACPI: emits change uevents to all physical companion devices of container's children

Started by"Rafael J. Wysocki" <rafael@kernel.org>
First post2017-04-19 02:40 +0200
Last post2017-04-19 11:00 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] ACPI: emits change uevents to all physical companion  devices of container's children "Rafael J. Wysocki" <rafael@kernel.org> - 2017-04-19 02:40 +0200
    Re: [PATCH] ACPI: emits change uevents to all physical companion  devices of container's children joeyli <jlee@suse.com> - 2017-04-19 11:00 +0200

#1625755 — Re: [PATCH] ACPI: emits change uevents to all physical companion devices of container's children

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2017-04-19 02:40 +0200
SubjectRe: [PATCH] ACPI: emits change uevents to all physical companion devices of container's children
Message-ID<txLP4-4F9-9@gated-at.bofh.it>
On Mon, Apr 3, 2017 at 5:55 PM, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> The caa73ea1 patch, "ACPI / hotplug / driver core: Handle containers
> in a special way", introduced the offline callback of acpi container.
> In the patch description, it mentions:
>
>     For ACPI containers that callback simply walks the list of ACPI
>     device objects right below the container object (its children) and
>     checks if all of their physical companion devices are offline.  If
>     that's not the case, it returns -EBUSY and the container system
>     devivce cannot be put offline.  Consequently, to put the container
>     system device offline, it is necessary to put all of the physical
>     devices depending on its ACPI companion object offline beforehand.
>
> Looks that it means acpi_container_offline() should walks all physical
> companion devices of container's children and checks their offline
> state. And, the comment in source code is "Check all of the dependent
> devices' physical companions", which means it should checks _all_
> physical companions.
>
> But, the checking code just stops at the first not-offlined physical
> companion device of the first not-offlined child, then kernel only
> emits KOBJ_CHANGE uevent to the one device. It doesn't really walk
> all children's all physical companion devices and doesn't send change
> uevent to them.

It is unclear to me from the description whether or not this is a
practical issue.

Also there is an alternative, which is not to send KOBJ_CHANGE uevents
to any children at all.

Why is the approach you chose better?

Thanks,
Rafael

[toc] | [next] | [standalone]


#1625942

Fromjoeyli <jlee@suse.com>
Date2017-04-19 11:00 +0200
Message-ID<txTCW-1gk-25@gated-at.bofh.it>
In reply to#1625755
Hi, 

On Wed, Apr 19, 2017 at 02:30:18AM +0200, Rafael J. Wysocki wrote:
> On Mon, Apr 3, 2017 at 5:55 PM, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > The caa73ea1 patch, "ACPI / hotplug / driver core: Handle containers
> > in a special way", introduced the offline callback of acpi container.
> > In the patch description, it mentions:
> >
> >     For ACPI containers that callback simply walks the list of ACPI
> >     device objects right below the container object (its children) and
> >     checks if all of their physical companion devices are offline.  If
> >     that's not the case, it returns -EBUSY and the container system
> >     devivce cannot be put offline.  Consequently, to put the container
> >     system device offline, it is necessary to put all of the physical
> >     devices depending on its ACPI companion object offline beforehand.
> >
> > Looks that it means acpi_container_offline() should walks all physical
> > companion devices of container's children and checks their offline
> > state. And, the comment in source code is "Check all of the dependent
> > devices' physical companions", which means it should checks _all_
> > physical companions.
> >
> > But, the checking code just stops at the first not-offlined physical
> > companion device of the first not-offlined child, then kernel only
> > emits KOBJ_CHANGE uevent to the one device. It doesn't really walk
> > all children's all physical companion devices and doesn't send change
> > uevent to them.
> 
> It is unclear to me from the description whether or not this is a
> practical issue.
> 
> Also there is an alternative, which is not to send KOBJ_CHANGE uevents
> to any children at all.
> 
> Why is the approach you chose better?
>

Please ignore this patch!

Thanks for your review. I'd say sorry for that I confused with
the code in acpi_scan_hot_remove() when I was sending this patch.
At that time I didn't aware that the acpi_container_offline() does
not need to send uevent to not-offline-yet devices.  

Before two weeks ago (around Apr. 5), I sent a mail to linux-acpi
for reminding to ignore this patch. I don't know why the mail got
filtered...

Thanks a lot!
Joey Lee

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web