linux-kernel - Re: [linux-pm] [PATCH 1/2] PM / Runtime: Introduce pm_runtime_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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <201108212009.49550.rjw@sisk.pl>
Date:	Sun, 21 Aug 2011 20:09:49 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	linux-sh@...r.kernel.org,
	Linux PM mailing list <linux-pm@...ts.linux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [linux-pm] [PATCH 1/2] PM / Runtime: Introduce pm_runtime_irq_unsafe()

On Sunday, August 21, 2011, Alan Stern wrote:
> On Sat, 20 Aug 2011, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@...k.pl>
> > 
> > Add a helper function allowing drivers and subsystems to clear
> > the power.irq_safe device flag.
> 
> > --- linux.orig/drivers/base/power/runtime.c
> > +++ linux/drivers/base/power/runtime.c
> > @@ -1109,22 +1109,23 @@ void pm_runtime_no_callbacks(struct devi
> >  EXPORT_SYMBOL_GPL(pm_runtime_no_callbacks);
> >  
> >  /**
> > - * pm_runtime_irq_safe - Leave interrupts disabled during callbacks.
> > + * __pm_runtime_irq_safe - Manipulate a device's power.irq_safe flag.
> >   * @dev: Device to handle
> > + * @irq_safe: Whether or not to leave interrupts disabled during callbacks.
> >   *
> > - * Set the power.irq_safe flag, which tells the PM core that the
> > + * Set or unset the power.irq_safe flag, which tells the PM core that the
> >   * ->runtime_suspend() and ->runtime_resume() callbacks for this device should
> >   * always be invoked with the spinlock held and interrupts disabled.  It also
> >   * causes the parent's usage counter to be permanently incremented, preventing
> >   * the parent from runtime suspending -- otherwise an irq-safe child might have
> >   * to wait for a non-irq-safe parent.
> >   */
> > -void pm_runtime_irq_safe(struct device *dev)
> > +void __pm_runtime_irq_safe(struct device *dev, bool irq_safe)
> >  {
> >  	if (dev->parent)
> >  		pm_runtime_get_sync(dev->parent);
> >  	spin_lock_irq(&dev->power.lock);
> > -	dev->power.irq_safe = 1;
> > +	dev->power.irq_safe = irq_safe;
> >  	spin_unlock_irq(&dev->power.lock);
> 
> It's not quite this easy.  There are two important aspects that must be
> considered.
> 
> Firstly, I originally envisioned pm_runtime_irq_safe() being called
> just once, before the device is enabled for runtime PM.  If you allow
> the flag to be turned on and off like this, you raise the possibility
> of races with runtime PM callbacks.  (That is, if a callback occurs at
> about the same time as the irq_safe flag is changed, nobody can predict
> whether the callback will be invoked with interrupts enabled.)  Maybe
> that's something the driver needs to take care of, but it should at
> least be mentioned in the documentation.

Good point.  Perhaps I should make it possible only if runtime PM is
disabled.

> Secondly, this doesn't manage the parent's usage counter correctly.

Right, I forgot about that.

> Do the pm_runtime_get_sync(dev->parent) at the beginning only when the
> irq_safe flag was off and is being turned on.  And at the end, if the
> irq_safe flag was on and is being turned off, do
> pm_runtime_put_sync(dev->parent).  See pm_runtime_remove() for why this
> matters.  (Also update the documentation; the change to the parent
> isn't necessarily permanent any more.)

I'll try to figure out an alternative approach without the $subject change
first.  If I can't, I'll revisit this idea.

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/