* Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines
@ 2002-07-25 15:15 Ravikiran G Thirumalai
2002-07-26 6:24 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Ravikiran G Thirumalai @ 2002-07-25 15:15 UTC (permalink / raw)
To: torvalds; +Cc: linux-kernel, trivial
I've noticed that xtime_lock and timerlist_lock ends up on the same
cacheline all the time (atleaset on x86). Not a good thing for
loads with high xxx_timer and do_gettimeofday counts I guess (networking etc).
Here's a 2.5.25 objdump:
c0302780 g O .data 00000004 time_freq
c0302784 g O .data 00000004 timerlist_lock
c0302788 g O .data 00000004 tqueue_lock
c030278c g O .data 00000004 xtime_lock
c0302790 l O .data 00000004 count.3
c0302794 l O .data 00000004 uidhash_lock
c0302798 g O .data 00000018 root_user
Here's a trivial 2.5.28 based fix.
-Kiran
diff -ruN -X dontdiff linux-2.5.28/kernel/timer.c align_locks/kernel/timer.c
--- linux-2.5.28/kernel/timer.c Thu Jul 25 02:33:23 2002
+++ align_locks/kernel/timer.c Thu Jul 25 18:17:46 2002
@@ -169,7 +169,7 @@
}
/* Initialize both explicitly - let's try to have them in the same cache line */
-spinlock_t timerlist_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t timerlist_lock ____cacheline_aligned_in_smp = SPIN_LOCK_UNLOCKED;
#ifdef CONFIG_SMP
volatile struct timer_list * volatile running_timer;
@@ -327,7 +327,7 @@
spin_unlock_irq(&timerlist_lock);
}
-spinlock_t tqueue_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t tqueue_lock __cacheline_aligned_in_smp = SPIN_LOCK_UNLOCKED;
void tqueue_bh(void)
{
@@ -633,7 +633,7 @@
* This read-write spinlock protects us from races in SMP while
* playing with xtime and avenrun.
*/
-rwlock_t xtime_lock = RW_LOCK_UNLOCKED;
+rwlock_t xtime_lock __cacheline_aligned_in_smp = RW_LOCK_UNLOCKED;
unsigned long last_time_offset;
static inline void update_times(void)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines
2002-07-25 15:15 Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines Ravikiran G Thirumalai
@ 2002-07-26 6:24 ` Rusty Russell
2002-07-26 7:26 ` Kiran
0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2002-07-26 6:24 UTC (permalink / raw)
To: Ravikiran G Thirumalai; +Cc: linux-kernel, torvalds
In message <20020725204512.E3594@in.ibm.com> you write:
> I've noticed that xtime_lock and timerlist_lock ends up on the same
> cacheline all the time (atleaset on x86). Not a good thing for
> loads with high xxx_timer and do_gettimeofday counts I guess (networking etc)
..
Better might be to use the x86-64 trick of using sequence counters
around do_gettimeofday, and avoid the xtime lock altogether. That
will improve gettimeofday performance as well. Or you could try
changing xtime lock to a brlock.
FYI: as policy, I don't take optimization patches without
measurements. I'm just not that smart.
Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines
2002-07-26 6:24 ` Rusty Russell
@ 2002-07-26 7:26 ` Kiran
2002-07-26 7:56 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Kiran @ 2002-07-26 7:26 UTC (permalink / raw)
To: Rusty Russell; +Cc: linux-kernel, torvalds
On Fri, Jul 26, 2002 at 04:24:51PM +1000, Rusty Russell wrote:
> In message <20020725204512.E3594@in.ibm.com> you write:
> > I've noticed that xtime_lock and timerlist_lock ends up on the same
> > cacheline all the time (atleaset on x86). Not a good thing for
> > loads with high xxx_timer and do_gettimeofday counts I guess (networking etc)
> ..
>
> Better might be to use the x86-64 trick of using sequence counters
> around do_gettimeofday, and avoid the xtime lock altogether. That
> will improve gettimeofday performance as well. Or you could try
> changing xtime lock to a brlock.
>
Ok, I'll look at the x86-64 code
> FYI: as policy, I don't take optimization patches without
> measurements. I'm just not that smart.
>
This patch was not meant to be a definitive fix for do_gettimeofday.
I thought having diffrent locks on the same cacheline was bad. Atleast,
I don't think there'd be any negative performance impact due to my patch.
Pls correct me if I am wrong.
I want to get some nos too .. and probably will...(still waiting for my
turn to use the 4way here :-) ). But, I decided to post this patch
as a follow up to the 2.5 profiler discussion on lse-tech.
Anywayz, point taken. Next time I submit an optimization patch to you,
I'll post the measuements too.
Thanks,
Kiran
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines
2002-07-26 7:26 ` Kiran
@ 2002-07-26 7:56 ` Rusty Russell
2002-07-26 9:23 ` Kiran
0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2002-07-26 7:56 UTC (permalink / raw)
To: Kiran; +Cc: linux-kernel, torvalds
In message <20020726125605.A2822@phreaker.net> you write:
> This patch was not meant to be a definitive fix for do_gettimeofday.
> I thought having diffrent locks on the same cacheline was bad. Atleast,
> I don't think there'd be any negative performance impact due to my patch.
> Pls correct me if I am wrong.
Did you ever wonder why we don't declare spinlock to be ____cacheline_aligned?
While it's probably justified in this case, you pay for it in a slight
increase in size...
> I want to get some nos too .. and probably will...(still waiting for my
> turn to use the 4way here :-) ). But, I decided to post this patch
> as a follow up to the 2.5 profiler discussion on lse-tech.
> Anywayz, point taken. Next time I submit an optimization patch to you,
> I'll post the measuements too.
Sure!
Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines
2002-07-26 7:56 ` Rusty Russell
@ 2002-07-26 9:23 ` Kiran
2002-07-27 4:17 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Kiran @ 2002-07-26 9:23 UTC (permalink / raw)
To: Rusty Russell; +Cc: linux-kernel, torvalds
On Fri, Jul 26, 2002 at 05:56:19PM +1000, Rusty Russell wrote:
> In message <20020726125605.A2822@phreaker.net> you write:
> > This patch was not meant to be a definitive fix for do_gettimeofday.
> > I thought having diffrent locks on the same cacheline was bad. Atleast,
> > I don't think there'd be any negative performance impact due to my patch.
> > Pls correct me if I am wrong.
>
> Did you ever wonder why we don't declare spinlock to be ____cacheline_aligned?
Yep...and for long enough...(I think..)..or there'd have been an RFC
or a stupid question to lkml sometime from me :)
> While it's probably justified in this case, you pay for it in a slight
> increase in size...
>
I thought you were of the opinion that "memory is cheap" ;-)
-Kiran
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines
2002-07-26 9:23 ` Kiran
@ 2002-07-27 4:17 ` Rusty Russell
0 siblings, 0 replies; 6+ messages in thread
From: Rusty Russell @ 2002-07-27 4:17 UTC (permalink / raw)
To: Kiran; +Cc: linux-kernel
In message <20020726145344.A18568@phreaker.net> you write:
> On Fri, Jul 26, 2002 at 05:56:19PM +1000, Rusty Russell wrote:
> > While it's probably justified in this case, you pay for it in a slight
> > increase in size...
>
> I thought you were of the opinion that "memory is cheap" ;-)
Absolutely: if you're trading it for programming time.
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-07-27 4:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-07-25 15:15 Patch 2.5.25: Ensure xtime_lock and timerlist_lock are on difft cachelines Ravikiran G Thirumalai
2002-07-26 6:24 ` Rusty Russell
2002-07-26 7:26 ` Kiran
2002-07-26 7:56 ` Rusty Russell
2002-07-26 9:23 ` Kiran
2002-07-27 4:17 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox