linux-kernel - Re: [PATCH 1/3] ACPI / dock: Rework the handling of notifications

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1494881.y5W1zydDGl@vostro.rjw.lan>
Date:	Sun, 30 Jun 2013 16:18:42 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jiang Liu <liuj97@...il.com>,
	"Alexander E. Patrakov" <patrakov@...il.com>
Subject: Re: [PATCH 1/3] ACPI / dock: Rework the handling of notifications

On Saturday, June 29, 2013 09:12:59 AM Yinghai Lu wrote:
> On Sat, Jun 29, 2013 at 4:16 AM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> > On Friday, June 28, 2013 04:34:21 PM Yinghai Lu wrote:
> >> On Fri, Jun 28, 2013 at 3:53 PM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> >> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >> >
> >> > The ACPI dock driver uses register_acpi_bus_notifier() which
> >> > installs a notifier triggered globally for all system notifications.
> >> > That first of all is inefficient, because the dock driver is only
> >> > interested in notifications associated with the devices it handles,
> >> > but it has to handle all system notifies for all devices.  Moreover,
> >> > it does that even if no docking stations are present in the system
> >> > (CONFIG_ACPI_DOCK set is sufficient for that to happen).  Besides,
> >> > that is inconvenient, because it requires the driver to do extra work
> >> > for each notification to find the target dock station object.
> >> >
> >> > For these reasons, rework the dock driver to install a notify
> >> > handler individually for each dock station in the system using
> >> > acpi_install_notify_handler().  This allows the dock station
> >> > object to be passed directly to the notify handler and makes it
> >> > possible to simplify the dock driver quite a bit.  It also
> >> > reduces the overhead related to the handling of all system
> >> > notifies when CONFIG_ACPI_DOCK is set.
> >>
> >> original change to use register_acpi_bus_notifier, have two assumption
> >> 1.  two dock_station will have same handle.
> >
> > Well, that would mean that dock_add() might be called twice for the same handle
> > and I don't see how that's possible.
> >
> > Moreover, even if that were possible, the loop in acpi_dock_notifier_call()
> > would break after finding the *first* matching handle anyway, so
> > acpi_dock_deferred_cb() wouldn't be called for the second dock station with
> > the same handle, if there were two.
> 
> related commit:
> commit 6bd00a61ab63d4ceb635ae0316353c11c900b8d8
> Author: Shaohua Li <shaohua.li@...el.com>
> Date:   Thu Aug 28 10:04:29 2008 +0800
> 
>     ACPI: introduce notifier change to avoid duplicates
> 
>     The battery driver already registers notification handler.
>     To avoid registering notification handler again,
>     introduce a notifier chain in global system notifier handler
>     and use it in dock driver.
> 
> so it is not two dock station have same handle. it is battery acpi driver.
> 
> but notitifer installing is changed
> 
> commit d94066910943837558d2a461c6766da981260bf0
> Author: Bjorn Helgaas <bjorn.helgaas@...com>
> Date:   Thu Apr 30 09:35:47 2009 -0600
> 
>     ACPI: battery: use .notify method instead of installing handler directly
> 
>     This patch adds a .notify() method.  The presence of .notify() causes
>     Linux/ACPI to manage event handlers and notify handlers on our behalf,
>     so we don't have to install and remove them ourselves.
> 
>     This driver apparently relies on seeing ALL notify events, not just
>     device-specific ones (because it used ACPI_ALL_NOTIFY).  We use the
>     ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag to request all events.
> 
>     Signed-off-by: Bjorn Helgaas <bjorn.helgaas@...com>
>     CC: Alexey Starikovskiy <alexey.y.starikovskiy@...ux.intel.com>
>     Signed-off-by: Len Brown <len.brown@...el.com>
> 
> 
> >
> >> 2. acpi subsystem: non root acpi device only can have one system
> >> notifier installed.
> >
> > No, that limitation is long gone.  We removed it when we were working on ACPI
> > wakeup support for runtime PM.
> 
> looks like i misread that...
> 
>  * NOTES:       The Root namespace object may have only one handler for each
>  *              type of notify (System/Device). Device/Thermal/Processor objects
>  *              may have one device notify handler, and multiple system notify
>  *              handlers.
> 
> device could have multiple system notify handlers.
> 
> So
> 
> Acked-by: Yinghai Lu <yinghai@...nel.org>

Thanks (and for the other ACKs too)!

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