Skip to contentSkip to conversation

Improve cache_reap hotplug cpu support

PATCH In review Dimitri Sivanich 2 participants 2 messages Aug 19, 2004 Memory management

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Improve cache_reap hotplug cpu support

I found that there was room for improvement in the hotplug cpu code in cache_reap. This applies to the code that moved cache_reap out of timer context.

The patch applies over 2.6.8.1-mm2.

mm/slab.c +23 -3
Index: linux/mm/slab.c
===================================================================
@@ -613,12 +613,22 @@ static void __devinit start_cpu_timer(in
 	 * init_workqueues() has already run, so keventd will be setup
 	 * at that time.
 	 */
-	if (keventd_up() && reap_work->func == NULL) {
-		INIT_WORK(reap_work, cache_reap, NULL);
+	if (keventd_up() && reap_work->data == NULL) {
+		INIT_WORK(reap_work, cache_reap, reap_work);
 		schedule_delayed_work_on(cpu, reap_work, HZ + 3 * cpu);
 	}
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+static void stop_cpu_timer(int cpu)
+{
+	struct work_struct *reap_work = &per_cpu(reap_work, cpu);
+
+	/* Null out this otherwise unused pointer for checking in cache_reap */
+	reap_work->data = NULL;
+}
+#endif
+
 static struct array_cache *alloc_arraycache(int cpu, int entries, int batchcount)
 {
 	int memsize = sizeof(void*)*entries+sizeof(struct array_cache);
@@ -670,6 +680,7 @@ static int __devinit cpuup_callback(stru
 		break;
 #ifdef CONFIG_HOTPLUG_CPU
 	case CPU_DEAD:
+		stop_cpu_timer(cpu);
 		/* fall thru */
 	case CPU_UP_CANCELED:
 		down(&cache_chain_sem);
@@ -2731,10 +2742,19 @@ static void drain_array_locked(kmem_cach
  * If we cannot acquire the cache chain semaphore then just give up - we'll
  * try again on the next iteration.
  */
-static void cache_reap(void *unused)
+static void cache_reap(void *work)
 {
 	struct list_head *walk;
 
+#ifdef CONFIG_HOTPLUG_CPU
+	if (unlikely((work==NULL) ||
+			((struct work_struct *)work)->data == NULL)) {
+		/* We've been moved off the original cpu due to HOTPLUG,
+		 * don't resched.
+		 */
+		return;
+	}
+#endif
 	if (down_trylock(&cache_chain_sem)) {
 		/* Give up. Setup the next iteration. */
 		schedule_delayed_work(&__get_cpu_var(reap_work), REAPTIMEOUT_CPUC + smp_processor_id());

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#1 lore raw
Quoted 3 lines · Dimitri Sivanich

I found that there was room for improvement in the hotplug cpu code in cache_reap.

"improvement" is not an adequate description for your change, and it is unobvious from the diff what it is intended to do, and why, and what the expected and observed results were.

Please describe your work more carefully.

The patch adds even more ifdefs. Suggest you open-code this:

mm/slab.c static void __devinit start_cpu_timer(in · line 630

Quoted 9 lines
mm/slab.c +9 -0
+#ifdef CONFIG_HOTPLUG_CPU
+static void stop_cpu_timer(int cpu)
+{
+	struct work_struct *reap_work = &per_cpu(reap_work, cpu);
+
+	/* Null out this otherwise unused pointer for checking in cache_reap */
+	reap_work->data = NULL;
+}
+#endif

in here:

mm/slab.c static int __devinit cpuup_callback(stru · line 686

Quoted 11 lines

static struct array_cache *alloc_arraycache(int cpu, int entries, int batchcount) {

mm/slab.c +1 -0
 	int memsize = sizeof(void*)*entries+sizeof(struct array_cache);
@@ -670,6 +680,7 @@ static int __devinit cpuup_callback(stru
 		break;
 #ifdef CONFIG_HOTPLUG_CPU
 	case CPU_DEAD:
+		stop_cpu_timer(cpu);
 		/* fall thru */
 	case CPU_UP_CANCELED:
 		down(&cache_chain_sem);

thereby removing one of them.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>