[PATCH] don't use ld.bias

[PATCH] don't use ld.bias

[Jesse Barnes]

Patch to fix spin_lock to not use ld.bias since it causes a bunch of bus
activity that hurts scalability when a lock is contended.

Thanks,
Jesse

=== include/asm-ia64/spinlock.h 1.13 vs edited ==--- 1.13/include/asm-ia64/spinlock.h	Fri May 30 19:59:40 2003
+++ edited/include/asm-ia64/spinlock.h	Mon Jul 21 16:54:15 2003
@@ -93,13 +93,16 @@
  * Streamlined test_and_set_bit(0, (x)).  We use test-and-test-and-set
  * rather than a simple xchg to avoid writing the cache-line when
  * there is contention.
+ *
+ * Don't use ld.bias since it generates unnecessary bus activity on
+ * multiprocessor systems when the lock is contended.
  */
 #define _raw_spin_lock(x) __asm__ __volatile__ (		\
 	"mov ar.ccv = r0\n"					\
 	"mov r29 = 1\n"						\
 	";;\n"							\
 	"1:\n"							\
-	"ld4.bias r2 = [%0]\n"					\
+	"ld4 r2 = [%0]\n"					\
 	";;\n"							\
 	"cmp4.eq p0,p7 = r0,r2\n"				\
 	"(p7) br.cond.spnt.few 1b \n"				\

RE: [PATCH] don't use ld.bias

[Chen, Kenneth W]

> Patch to fix spin_lock to not use ld.bias since it causes a bunch of
bus
> activity that hurts scalability when a lock is contended.

It's dangerous to say something without knowing the whole context, but I
can't resist asking why not fix the contended lock area instead?  Sounds
like you have some serious contention.

Re: [PATCH] don't use ld.bias

[Jesse Barnes]

On Mon, Jul 21, 2003 at 05:15:39PM -0700, Chen, Kenneth W wrote:
> > Patch to fix spin_lock to not use ld.bias since it causes a bunch of
> bus
> > activity that hurts scalability when a lock is contended.
> 
> It's dangerous to say something without knowing the whole context, but I
> can't resist asking why not fix the contended lock area instead?  Sounds
> like you have some serious contention.

Maybe I shouldn't have said 'contended'.  This causes problems even when
a lock is nearly uncontended.  I agree that fixing lock contention is a
good idea, but as I read this code, it'll only make the very best case
(totally uncontended) only a little better than if the ld.bias were just
an ld and it makes the average and contended cases _much_ worse.

I can get some data if you're not convinced by reading the code :)

Thanks,
Jesse

RE: [PATCH] don't use ld.bias

[Mallick, Asit K]

It does make sense to not use the ld.bias in the spinlock, it will make
more cacheline bouncing in a larger SMP system.
Thanks,
Asit


> -----Original Message-----
> From: Jesse Barnes [mailto:jbarnes@sgi.com] 
> Sent: Tuesday, July 22, 2003 6:12 AM
> To: Chen, Kenneth W
> Cc: linux-ia64@vger.kernel.org; davidm@hpl.hp.com
> Subject: Re: [PATCH] don't use ld.bias
> 
> 
> On Mon, Jul 21, 2003 at 05:15:39PM -0700, Chen, Kenneth W wrote:
> > > Patch to fix spin_lock to not use ld.bias since it causes 
> a bunch of
> > bus
> > > activity that hurts scalability when a lock is contended.
> > 
> > It's dangerous to say something without knowing the whole 
> context, but I
> > can't resist asking why not fix the contended lock area 
> instead?  Sounds
> > like you have some serious contention.
> 
> Maybe I shouldn't have said 'contended'.  This causes 
> problems even when
> a lock is nearly uncontended.  I agree that fixing lock 
> contention is a
> good idea, but as I read this code, it'll only make the very best case
> (totally uncontended) only a little better than if the 
> ld.bias were just
> an ld and it makes the average and contended cases _much_ worse.
> 
> I can get some data if you're not convinced by reading the code :)
> 
> Thanks,
> Jesse
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] don't use ld.bias

[Matthew Wilcox]

On Tue, Jul 22, 2003 at 09:40:02PM -0700, Mallick, Asit K wrote:
> It does make sense to not use the ld.bias in the spinlock, it will make
> more cacheline bouncing in a larger SMP system.

it certainly makes a mockery of the comment:

 * Streamlined test_and_set_bit(0, (x)).  We use test-and-test-and-set
 * rather than a simple xchg to avoid writing the cache-line when
 * there is contention.

since there's no real advantage (AIUI) to have the cacheline
clean-exclusive vs dirty-exclusive.

but why not delete this code?  it's under #else /* !NEW_LOCK */ and
NEW_LOCK looks to be the better code as well as the default in 2.5.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: [PATCH] don't use ld.bias

[David Mosberger]

>>>>> On Mon, 21 Jul 2003 16:55:59 -0700, jbarnes@sgi.com (Jesse Barnes) said:

  Jesse> Patch to fix spin_lock to not use ld.bias since it causes a
  Jesse> bunch of bus activity that hurts scalability when a lock is
  Jesse> contended.

Yes, using ld.bias really was wrong there.  However, instead of
applying your patch, I dropped the old lock code (which was
effectively dead code) and instead removed the .bias from the
ia64_spinlock_contention() routine in head.S.

Thanks,

	--david

Re: [PATCH] don't use ld.bias

[Jesse Barnes]

On Thu, Jul 24, 2003 at 06:10:13PM -0700, David Mosberger wrote:
> >>>>> On Mon, 21 Jul 2003 16:55:59 -0700, jbarnes@sgi.com (Jesse Barnes) said:
> 
>   Jesse> Patch to fix spin_lock to not use ld.bias since it causes a
>   Jesse> bunch of bus activity that hurts scalability when a lock is
>   Jesse> contended.
> 
> Yes, using ld.bias really was wrong there.  However, instead of
> applying your patch, I dropped the old lock code (which was
> effectively dead code) and instead removed the .bias from the
> ia64_spinlock_contention() routine in head.S.

Even better.  Thanks.

Jesse