Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1159784 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH RFC] hwspinlock: Don't take software spinlock before hwspinlock Jeffrey Hugo <jhugo@codeaurora.org> - 2015-06-06 02:00 +0200
    Re: [PATCH RFC] hwspinlock: Don't take software spinlock before hwspinlock Ohad Ben-Cohen <ohad@wizery.com> - 2015-06-06 05:30 +0200

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

On 6/4/2015 7:09 PM, Ohad Ben-Cohen wrote:
> On Tue, May 26, 2015 at 11:36 PM, Lina Iyer <lina.iyer@linaro.org> 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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1159819

On Sat, Jun 6, 2015 at 2:50 AM, Jeffrey Hugo <jhugo@codeaurora.org> wrote:
> 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.

How about:
- add a 'hwcaps' member to hwspinlock_device, and define single cap
called HWL_CAP_ALLOW_RAW
- add new 'hwcaps' parameter to hwspin_lock_register
- omap_hwspinlock.c will pass NULL, qcom_hwspinlock will pass HWL_CAP_ALLOW_RAW
- hwspin_lock_register will set hwcaps accordingly
- before a lock is taken in RAW mode, the capabilities are checked
- document everything nicely

Unless I'm missing something, it should take 5 minutes or less. For
reference, feel free to check out mmc_host's caps member and its
usage.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web