Re: [PATCH RFC] hwspinlock: Don't take software spinlock before hwspinlock

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

 



On 6/4/2015 7:09 PM, Ohad Ben-Cohen wrote:
On Tue, May 26, 2015 at 11:36 PM, Lina Iyer <lina.iyer@xxxxxxxxxx> wrote:
Just to make sure I understand, is this how your scenario is solved?

- c1 goes down
- c0 goes down, carries information about shared resources
- c1 takes HWLOCK and calls into SCM, stuck handling FIQs
- c0 wants to call into SCM but is waiting spinning on HWLOCK
- c1 completes handling FIQs, goes idle, HWLOCK is released by secure monitor
- c0 takes HWLOCK, calls into SCM, shared resources handled correctly,

HWLOCK in this example is a single shared hwspinlock accessible by c0,
c1 and secure monitor.

That is correct.

Ok, thanks.

If we adopt the proposed approach in your patch, I'm thinking maybe we
should restrict it only to hardware implementations that explicitly
allow it, using some hardware capability flag published by the
hwspinlock driver.

In OMAP, e.g., it is prohibited to spin on this hwlock for a long
period of time, so such a hw cap flag would allow you guys to enable
this behaviour specifically for your driver.

What do you think?

Lina and I talked about this today.

Lina's current approach of adding a flag (HWLOCK_NOLOCK) seems to flow with the current framework considering the framework already has several flags to control the software spinlock behavior. The NOLOCK flag is not a default option, so current OMAP code is not affected. If you like, we could label the option with comments in the header as an advanced option. Users better be aware of what they are doing when they use it. OMAP code should never use the option (based on what you appear to be saying above), and if someone attempts to use it in OMAP code, well there is plenty of rope around the kernel to get one into trouble. This doesn't seem to be an exception. The default behavior that 99% of clients will use remains the same simple interface as it is today, but "power users" who require more control are given that flexibility, along with additional responsibility.

In short, Lina's solution is simple and uses an existing mechanism to satisfy the requirement.

Your capability proposal could be made to work, but it seems to introduce additional questions. Can a capability be applied to a specific lock? Can a capability be applied to an entire bank of locks? Can capabilities be changed during the lifetime of the system? What would the API look like? Since this mechanism appears to require a new API, what other things would that API cover? Is it appropriate to put usecase specific logic into a driver that is responsible for managing the hardware? IE policy vs mechanism. From "prototyping" an implementation of this in my head, it seems more complicated and heavy handed for a very simple need that can be easily handled in another way. As far as I am aware, there is no other need at this point in time for such a mechanism, so it seems to be a lot of effort and work, for limited use.

In the interest of keeping things simple, Lina and I think Lina's current proposal is the preferred way forward at this point in time given the information available today.

If you still wish to scope out a capability based alternative, would you please provide some details about how you envision it working? An example of the API, how it would be used, future usecases that might be covered by it, etc. That would give us specifics we can discuss and weigh the merits of.

Thanks.

--
Jeffrey Hugo
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux