Skip to contentSkip to conversation

Move mostly read variables to __read_mostly section.

PATCH In review Eial Czerwacki 2 participants 3 messages Oct 23, 2011 – Oct 25, 2011

Subsystems (2)
VFS Timekeeping, clocksource core, NTP, alarmtimer
lore
Share & tools
lore

Eial Czerwacki

author
#0 lore raw

PATCH 1/1 Move mostly read variables to __read_mostly section.

Move mostly read variables to __read_mostly section.

Signed-off-by Eial Czerwacki<[email protected]>
Signed-off-by Shai Fultheim<[email protected]>

---

fs/buffer.c +1 -1
@@ -3233,7 +3233,7 @@ SYSCALL_DEFINE2(bdflush, int, func, long
 /*
  * Buffer-head allocation
  */
-static struct kmem_cache *bh_cachep;
+static struct kmem_cache __read_mostly *bh_cachep;
 
 /*
  * Once the number of bh's in the machine exceeds this level, we start
kernel/time/timekeeping.c +1 -1
@@ -154,7 +154,7 @@ __cacheline_aligned_in_smp DEFINE_SEQLOC
  * used instead.
  */
 static struct timespec xtime __attribute__ ((aligned (16)));
-static struct timespec wall_to_monotonic __attribute__ ((aligned (16)));
+static struct timespec wall_to_monotonic __attribute__ ((aligned (16))) __read_mostly;
 static struct timespec total_sleep_time;
 
 /*
#1 lore raw

kernel/time/timekeeping.c __cacheline_aligned_in_smp DEFINE_SEQLOC · line 158

Quoted 10 lines · Eial Czerwacki

* Once the number of bh's in the machine exceeds this level, we start

kernel/time/timekeeping.c +1 -1
@@ -154,7 +154,7 @@ __cacheline_aligned_in_smp DEFINE_SEQLOC
  * used instead.
  */
 static struct timespec xtime __attribute__ ((aligned (16)));
-static struct timespec wall_to_monotonic __attribute__ ((aligned (16)));
+static struct timespec wall_to_monotonic __attribute__ ((aligned
  (16))) __read_mostly;

That means that a common time fetch/update call will usually have to fetch two cachelines now instead of one, because it needs xtime and wall_to_monotonic. Usually those end up nearby.

On the other hand it's rare to fetch wall_to_monotonic without xtime.

So if anything all those variables should be grouped together in a single cacheline, but definitely not split like you do.

The other changes in your patch are fine for me.

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only
author
#2 lore raw
Quoted 16 lines · Andi Kleen

kernel/time/timekeeping.c __cacheline_aligned_in_smp DEFINE_SEQLOC · line 158

Quoted 10 lines · Eial Czerwacki

* Once the number of bh's in the machine exceeds this level, we start

kernel/time/timekeeping.c +1 -1
@@ -154,7 +154,7 @@ __cacheline_aligned_in_smp DEFINE_SEQLOC
   * used instead.
   */
  static struct timespec xtime __attribute__ ((aligned (16)));
-static struct timespec wall_to_monotonic __attribute__ ((aligned (16)));
+static struct timespec wall_to_monotonic __attribute__ ((aligned
   (16))) __read_mostly;

That means that a common time fetch/update call will usually have to fetch two cachelines now instead of one, because it needs xtime and wall_to_monotonic. Usually those end up nearby.

Quoted 8 lines

On the other hand it's rare to fetch wall_to_monotonic without xtime.

So if anything all those variables should be grouped together in a single cacheline, but definitely not split like you do.

The other changes in your patch are fine for me.

-Andi

Hello Andi,

Thanks for the input, we've removed the mentioned above section from the patch and it will be handled in a different patch.

the new patch is as follows:

Signed-off-by Eial Czerwacki<[email protected]>
Signed-off-by Shai Fultheim<[email protected]>

---

fs/buffer.c +1 -1
@@ -3233,7 +3233,7 @@ SYSCALL_DEFINE2(bdflush, int, func, long
 /*
  * Buffer-head allocation
  */
-static struct kmem_cache *bh_cachep;
+static struct kmem_cache __read_mostly *bh_cachep;
 
 /*
  * Once the number of bh's in the machine exceeds this level, we start