Re: [PATCH ver. 2] PM: add synchronous runtime interface for interrupt handlers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 22 Nov 2010, Rafael J. Wysocki wrote:

> > > I didn't like this change before and I still don't like it.  Quite frankly, I'm
> > > not sure I can convince Linus to pull it. :-)
> > > 
> > > Why don't we simply execute the callback under the spinlock in the
> > > IRQ safe case?
> > 
> > Because it wouldn't work.  The job of the runtime_idle callback is to
> > call pm_runtime_suspend when needed.  But if the callback runs under
> > the spinlock then pm_runtime_suspend would hang when it tries to grab
> > the lock.
> 
> Yes, in the _idle case.  I actually should have put my comment under
> the change in rpm_callback(), which is what I really meant.

But the new rpm_callback() _does_ simply execute the callback under the
spinlock in the irq-safe case.  So I don't understand what you mean
here.

> Moreover, I'm not sure if we need an "IRQ safe" version of _idle.  Why do
> we need it, exactly?

Because pm_runtime_put_sync() calls rpm_idle().  If there were no 
irq-safe version of rpm_idle() then drivers wouldn't be able to call 
pm_runtime_put_sync() from within an interrupt handler.


> The problem I have with this change is that switching interrupts off really is
> a part of the locking operation, so using spin_unlock() after spin_lock_irq...()
> is kind of like releasing the lock partially, which I don't think is valid
> (even if we're going to reacquire the lock immediately).

On the contrary; spin_unlock() after spin_lock_irq() doesn't partially
release the lock -- it releases the lock _entirely_!  :-)

Besides which, the existing code already releases the spinlock before 
making callbacks, so there should be no reason to worry about that 
issue.  The new code either:

	releases the spinlock but keeps interrupts disabled (in
	rpm_idle), or

	doesn't release the spinlock (in rpm_callback).

Either way, I should think you'd find the new code _less_ objectionable 
than the existing code, not _more_ objectionable.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux