linux-kernel - Re: [linux-pm] [PATCH] PERF(kernel): Cleanup power events V2

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-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1010271014280.1650-100000@iolanthe.rowland.org>
Date:	Wed, 27 Oct 2010 10:32:10 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	Linux-pm mailing list <linux-pm@...ts.linux-foundation.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [linux-pm] [PATCH] PERF(kernel): Cleanup power events V2

[CC: list trimmed drastically, on the assumption that most of the 
people on it aren't very interested in the details of the PM runtime 
memory barriers.]

On Wed, 27 Oct 2010, Mathieu Desnoyers wrote:

> grep -r pm_runtime_get_noresume drivers/    hands out very interesting info.
> 
> e.g.:
> 
> drivers/usb/core/drivers.c: usb_autopm_get_interface_async()
> 
>         pm_runtime_get_noresume(&intf->dev);
>         s = ACCESS_ONCE(intf->dev.power.runtime_status);
>         if (s == RPM_SUSPENDING || s == RPM_SUSPENDED)
>                 status = pm_request_resume(&intf->dev);
> 
> How is this supposed to work ?

It's worth pointing out that this code is going to be removed during
the 2.6.38 development cycle, due to ongoing changes in the runtime PM
core.  It would have been removed already if not for the difficulty of
coordinating cross-subsystem changes.

But it's legitimate to ask how the code _was_ supposed to work...

> If the ACCESS_ONCE can be reordered before the atomic_inc(), then I fear the
> device can be suspended even after the check.

You are correct; the code as written may sometimes fail.  It was a
hack from the beginning; the kind of test it performs should not be
done outside the PM core.  However at the time it was the easiest way 
to do what I wanted.

> My point is that a get/put semantic should imply memory barriers, especially if
> these are exported APIs.

As far as I am aware, apart from the hack above,
pm_runtime_get_noresume is called only in places where either:

	it is purely advisory (e.g., we know that we will use the
	device in the near future so we would prefer to prevent it from
	being suspended, but we don't really care because we're going
	to call pm_runtime_resume_sync before using it anyway);

	or we already know that the usage_count is > 0.

No memory barrier is required for either of these cases.

Alan Stern

--
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/