Prev: fs: icache RCU free inodes
Next: [Bugfix] unregister_trace_probe needs to be called under mutex
From: Jan Beulich on 30 Jun 2010 05:00
>>> On 30.06.10 at 10:05, Peter Zijlstra <peterz(a)infradead.org> wrote:
> On Tue, 2010-06-29 at 15:32 +0100, Jan Beulich wrote:
>> +static DEFINE_PER_CPU(arch_rwlock_t, spinning_rm_lock) =
> __ARCH_RW_LOCK_UNLOCKED;
>
> why is that an arch_ lock?

Because I don't think it is appropriate to use anything higher level
in the callouts from the lock/unlock inline functions. The alternative
would be an open coded lock, which seems much less desirable to
me.

> why is that a rwlock?, those things are useless.

Because potentially each CPU's lock gets acquired for reading during
unlock, while only the locking CPU's one needs to be acquired for
writing during lock.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jan Beulich on 30 Jun 2010 05:10
>>> On 30.06.10 at 10:56, Peter Zijlstra <peterz(a)infradead.org> wrote:
> On Wed, 2010-06-30 at 09:52 +0100, Jan Beulich wrote:
>> > why is that a rwlock?, those things are useless.
>>
>> Because potentially each CPU's lock gets acquired for reading during
>> unlock, while only the locking CPU's one needs to be acquired for
>> writing during lock.
>
> Can you say: scalability nightmare? but then its Xen code so who cares..

Yes, this is a scalability problem. And yes, I'm open to suggestions.
But no, I didn't have any better idea myself. And for the time being
I don't foresee this to be a problem, as VMs tend to have much fewer
CPUs than physical machines. And I think a performance win of 25%
justifies a sub-optimal implementation (with the perspective of people
smarter than me replacing it with a better one over time).

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jan Beulich on 30 Jun 2010 10:10
>>> On 30.06.10 at 15:23, Jeremy Fitzhardinge <jeremy(a)goop.org> wrote:
> For spinlocks, the pvop calls should only be in the slow case: when a
> spinlock has been spinning for long enough, and on unlock when there's
> someone waiting for the lock. The fastpath (no contention lock and
> unlock) should have no extra calls.

Then what was all that performance regression noise concerning
pvops spinlocks about, leading to CONFIG_PARAVIRT_SPINLOCKS
being separated from the base CONFIG_PARAVIRT?

Afaics the unlock still involves a function call *in all cases* with
pvops spinlocks, whereas it's a single inline instruction without.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jan Beulich on 30 Jun 2010 10:40
>>> On 30.06.10 at 16:25, Jeremy Fitzhardinge <jeremy(a)goop.org> wrote:
> On 06/30/2010 04:03 PM, Jan Beulich wrote:
>> Afaics the unlock still involves a function call *in all cases* with
>> pvops spinlocks, whereas it's a single inline instruction without.
>>
>
> No. The unlock path can see if there are any further waiters by looking
> at the ticket in the, and only do the kick call if there are some.

Are we perhaps talking about different things? I'm referring to

static __always_inline void arch_spin_unlock(struct arch_spinlock *lock)
{
PVOP_VCALL1(pv_lock_ops.spin_unlock, lock);
}

which is an indirect call which, as I understand it, gets replaced
with a direct one at runtime. But it remains to be a call (as opposed
to being a single inc instructions without CONFIG_PARAVIRT_SPINLOCKS).

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: fs: icache RCU free inodes
Next: [Bugfix] unregister_trace_probe needs to be called under mutex