lkml.org 
[lkml]   [2013]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH RFC 1/2] qspinlock: Introducing a 4-byte queue spinlock implementation
On 08/01/2013 04:23 PM, Raghavendra K T wrote:
> On 08/01/2013 08:07 AM, Waiman Long wrote:
>>
>> +}
>> +/**
>> + * queue_spin_trylock - try to acquire the queue spinlock
>> + * @lock : Pointer to queue spinlock structure
>> + * Return: 1 if lock acquired, 0 if failed
>> + */
>> +static __always_inline int queue_spin_trylock(struct qspinlock *lock)
>> +{
>> + if (!queue_spin_is_contended(lock) && (xchg(&lock->locked, 1) ==
>> 0))
>> + return 1;
>> + return 0;
>> +}
>> +
>> +/**
>> + * queue_spin_lock - acquire a queue spinlock
>> + * @lock: Pointer to queue spinlock structure
>> + */
>> +static __always_inline void queue_spin_lock(struct qspinlock *lock)
>> +{
>> + if (likely(queue_spin_trylock(lock)))
>> + return;
>> + queue_spin_lock_slowpath(lock);
>> +}
>
> quickly falling into slowpath may hurt performance in some cases. no?

Failing the trylock means that the process is likely to wait. I do retry
one more time in the slowpath before waiting in the queue.

> Instead, I tried something like this:
>
> #define SPIN_THRESHOLD 64
>
> static __always_inline void queue_spin_lock(struct qspinlock *lock)
> {
> unsigned count = SPIN_THRESHOLD;
> do {
> if (likely(queue_spin_trylock(lock)))
> return;
> cpu_relax();
> } while (count--);
> queue_spin_lock_slowpath(lock);
> }
>
> Though I could see some gains in overcommit, but it hurted undercommit
> in some workloads :(.

The gcc 4.4.7 compiler that I used in my test machine has the tendency
of allocating stack space for variables instead of using registers when
a loop is present. So I try to avoid having loop in the fast path. Also
the count itself is rather arbitrary. For the first pass, I would like
to make thing simple. We can always enhance it once it is accepted and
merged.

>
>>
>> +/**
>> + * queue_trylock - try to acquire the lock bit ignoring the qcode in
>> lock
>> + * @lock: Pointer to queue spinlock structure
>> + * Return: 1 if lock acquired, 0 if failed
>> + */
>> +static __always_inline int queue_trylock(struct qspinlock *lock)
>> +{
>> + if (!ACCESS_ONCE(lock->locked) && (xchg(&lock->locked, 1) == 0))
>> + return 1;
>> + return 0;
>> +}
>
> It took long time for me to confirm myself that,
> this is being used when we exhaust all the nodes. But not sure of
> any better name so that it does not confuse with queue_spin_trylock.
> anyway, they are in different files :).
>

Yes, I know it is confusing. I will change the name to make it more
explicit.

>
> Result:
> sandybridge 32 cpu/ 16 core (HT on) 2 node machine with 16 vcpu kvm
> guests.
>
> In general, I am seeing undercommit loads are getting benefited by the
> patches.
>
> base = 3.11-rc1
> patched = base + qlock
> +----+-----------+-----------+-----------+------------+-----------+
> hackbench (time in sec lower is better)
> +----+-----------+-----------+-----------+------------+-----------+
> oc base stdev patched stdev %improvement
> +----+-----------+-----------+-----------+------------+-----------+
> 0.5x 18.9326 1.6072 20.0686 2.9968 -6.00023
> 1.0x 34.0585 5.5120 33.2230 1.6119 2.45313
> +----+-----------+-----------+-----------+------------+-----------+
> +----+-----------+-----------+-----------+------------+-----------+
> ebizzy (records/sec higher is better)
> +----+-----------+-----------+-----------+------------+-----------+
> oc base stdev patched stdev %improvement
> +----+-----------+-----------+-----------+------------+-----------+
> 0.5x 20499.3750 466.7756 22257.8750 884.8308 8.57831
> 1.0x 15903.5000 271.7126 17993.5000 682.5095 13.14176
> 1.5x 1883.2222 166.3714 1742.8889 135.2271 -7.45177
> 2.5x 829.1250 44.3957 803.6250 78.8034 -3.07553
> +----+-----------+-----------+-----------+------------+-----------+
> +----+-----------+-----------+-----------+------------+-----------+
> dbench (Throughput in MB/sec higher is better)
> +----+-----------+-----------+-----------+------------+-----------+
> oc base stdev patched stdev %improvement
> +----+-----------+-----------+-----------+------------+-----------+
> 0.5x 11623.5000 34.2764 11667.0250 47.1122 0.37446
> 1.0x 6945.3675 79.0642 6798.4950 161.9431 -2.11468
> 1.5x 3950.4367 27.3828 3910.3122 45.4275 -1.01570
> 2.0x 2588.2063 35.2058 2520.3412 51.7138 -2.62209
> +----+-----------+-----------+-----------+------------+-----------+
>
> I saw dbench results improving to 0.3529, -2.9459, 3.2423, 4.8027
> respectively after delaying entering to slowpath above.
> [...]
>
> I have not yet tested on bigger machine. I hope that bigger machine will
> see significant undercommit improvements.
>

Thank for running the test. I am a bit confused about the terminology.
What exactly do undercommit and overcommit mean?

Regards,
Longman



\
 
 \ /
  Last update: 2013-08-01 23:41    [W:0.087 / U:1.740 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site