linux-kernel - Re: [PATCH 2/5] PM / Runtime: Do not run callbacks under lock for power.irq

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 14 Sep 2011 22:45:14 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Ming Lei <tom.leiming@...il.com>
Cc:	Linux PM mailing list <linux-pm@...ts.linux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"Linux-sh list" <linux-sh@...r.kernel.org>,
	Magnus Damm <magnus.damm@...il.com>,
	Kevin Hilman <khilman@...com>, jean.pihet@...oldbits.com
Subject: Re: [PATCH 2/5] PM / Runtime: Do not run callbacks under lock for power.irq_safe set

On Wednesday, September 14, 2011, Ming Lei wrote:
> Hi,
> 
> On Wed, Sep 14, 2011 at 12:06 AM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> 
> >> >> If power.lock is released, the transition states(resuming or suspending)
> >> >
> >> >> may be observed in rpm_suspend or rpm_resume, then tasks schedule
> >> >
> >> >> will be produced in these two functions,
> >> >
> >> > I don't think so, because the interrupts are still off.
> >>
> >> Yes, the interrupts are still off on local CPU, but the release of spin lock may
> >> cause another CPUs to run into rpm_suspend or rpm_resume and produce
> >> task schedule inside the two functions.
> >
> > Not for the same device, though.
> 
> I think it is probable to happen on the same device in theory, see below:
> 
> - suppose irq_safe is set before calling two pm_runtime_suspend below
> - suppose this patch has been applied
> 
> CPU0						CPU1
> pm_runtime_suspend
> 	acquired power lock
> 	rpm_suspend
> 						pm_runtime_suspend
> 							spining power lock
> 	...
> 	release power lock
> 							acquired power lock
> 	run .runtime_suspend
> 							found the dev suspending
> 							wait for power state and schedule

OK, I see what the problem is.  The second CPU can see the status
being RPM_SUSPENDING in the irq_safe case, which isn't possible
without the patch.

Good catch!

I think in that case rpm_suspend() should just release the lock,
run cpu_relax(), reacquire the lock and go to the "repeat" label.

Thanks,
Rafael
--
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/