Re: [Xen-devel] [PATCH v1 6/9] spinlock: Introduce spin_lock_cb()

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 03/29/2017 09:47 AM, Boris Ostrovsky wrote:
> On 03/29/2017 06:28 AM, Wei Liu wrote:
>> On Fri, Mar 24, 2017 at 01:05:01PM -0400, Boris Ostrovsky wrote:
>>> While waiting for a lock we may want to periodically run some
>>> code. We could use spin_trylock() but since it doesn't take lock
>>> ticket it may take a long time until the lock is taken.
>>>
>>> Add spin_lock_cb() that allows us to execute a callback while waiting.
>>> Also add spin_lock_kick() that will wake up the waiters.
>>>
>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>>> ---
>>>  xen/common/spinlock.c      |   20 ++++++++++++++++++++
>>>  xen/include/xen/spinlock.h |    3 +++
>>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
>>> index 2a06406..d1de3ca 100644
>>> --- a/xen/common/spinlock.c
>>> +++ b/xen/common/spinlock.c
>>> @@ -129,6 +129,26 @@ static always_inline u16 
>>> observe_head(spinlock_tickets_t *t)
>>>      return read_atomic(&t->head);
>>>  }
>>>  
>>> +void _spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data)
>>> +{
>>> +    spinlock_tickets_t tickets = SPINLOCK_TICKET_INC;
>>> +    LOCK_PROFILE_VAR;
>>> +
>>> +    check_lock(&lock->debug);
>>> +    tickets.head_tail = arch_fetch_and_add(&lock->tickets.head_tail,
>>> +                                           tickets.head_tail);
>>> +    while ( tickets.tail != observe_head(&lock->tickets) )
>>> +    {
>>> +        LOCK_PROFILE_BLOCK;
>>> +        if ( cb )
>>> +            cb(data);
>>> +        arch_lock_relax();
>>> +    }
>>> +    LOCK_PROFILE_GOT;
>>> +    preempt_disable();
>>> +    arch_lock_acquire_barrier();
>>> +}
>>> +
>>>  void _spin_lock(spinlock_t *lock)
>> You should be able to use _spin_lock_cb to implement _spin_lock, right?
>
> I did consider this but decided not to do it because we'd be adding a
> few extra instructions and a call on potentially hot path.
>
> (And doing it as a #define of a spin_lock() would make things even worse).

Although declaring _spin_lock_cb() as an inline makes generated assembly
look essentially the same as with current _spin_lock() (I don't care
about the extra check inside the loop since that's a slow path).

So maybe I can indeed do this.

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel