[PATCH (resent with the attachment !)] netfilter : zap get_cpu()/put_cpu() calls from ip_tables

[Eric Dumazet <dada1@cosmosbay.com>]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

The 'ip_tables: NUMA-aware allocation' patch 
(http://www.kernel.org/git/?p=linux/kernel/git/davem/net-2.6.16.git;a=commit;h=6d1273850cac8db77c462caaa145b813d93adc55)
introduced a problem with get_cpu()/put_cpu()

The problem is that get_cpu() disables preemption and the thread is not 
allowed to sleep.

As the intent was only to give a hint, relax the enforcement and switch to a 
hint : If the current thread is preempted and migrated to another cpu, this is 
not a problem. Most of the time, thread wont be migrated anyway.

This patch removes get_cpu()/put_cpu() pairs and uses raw_smp_processor_id() 
were appropriate.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

[-- Attachment #2: zap_getput_cpu.patch --]
[-- Type: text/plain, Size: 9865 bytes --]

--- net-2.6.16-orig/net/ipv4/netfilter/ip_tables.c	2005-11-25 10:24:02.000000000 +0100
+++ net-2.6.16/net/ipv4/netfilter/ip_tables.c	2005-11-25 11:44:40.000000000 +0100
@@ -988,11 +988,14 @@
 {
 	unsigned int cpu;
 	unsigned int i;
-	unsigned int curcpu = get_cpu();
+	unsigned int curcpu;
 
 	/* Instead of clearing (by a previous call to memset())
 	 * the counters and using adds, we set the counters
-	 * with data used by current CPU */
+	 * with data used by 'current' CPU
+	 * We dont care about preemption here.
+	 */
+	curcpu = raw_smp_processor_id();
 
 	i = 0;
 	IPT_ENTRY_ITERATE(t->entries[curcpu],
@@ -1011,7 +1014,6 @@
 				  counters,
 				  &i);
 	}
-	put_cpu();
 }
 
 static int
@@ -1029,7 +1031,7 @@
 	   (other than comefrom, which userspace doesn't care
 	   about). */
 	countersize = sizeof(struct ipt_counters) * table->private->number;
-	counters = vmalloc(countersize);
+	counters = vmalloc_node(countersize, numa_node_id());
 
 	if (counters == NULL)
 		return -ENOMEM;
@@ -1039,8 +1041,11 @@
 	get_counters(table->private, counters);
 	write_unlock_bh(&table->lock);
 
-	/* choose the copy that is on our node/cpu, ... */
-	loc_cpu_entry = table->private->entries[get_cpu()];
+	/* choose the copy that is on our node/cpu, ...
+	 * This choice is lazy (because current thread is
+	 * allowed to migrate to another cpu)
+	 */
+	loc_cpu_entry = table->private->entries[raw_smp_processor_id()];
 	/* ... then copy entire thing ... */
 	if (copy_to_user(userptr, loc_cpu_entry, total_size) != 0) {
 		ret = -EFAULT;
@@ -1091,7 +1096,6 @@
 	}
 
  free_counters:
-	put_cpu();
 	vfree(counters);
 	return ret;
 }
@@ -1189,7 +1193,7 @@
 		return -ENOMEM;
 
 	/* choose the copy that is our node/cpu */
-	loc_cpu_entry = newinfo->entries[get_cpu()];
+	loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
 	if (copy_from_user(loc_cpu_entry, user + sizeof(tmp),
 			   tmp.size) != 0) {
 		ret = -EFAULT;
@@ -1242,9 +1246,8 @@
 	/* Get the old counters. */
 	get_counters(oldinfo, counters);
 	/* Decrease module usage counts and free resource */
-	loc_cpu_old_entry = oldinfo->entries[smp_processor_id()];
+	loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
 	IPT_ENTRY_ITERATE(loc_cpu_old_entry, oldinfo->size, cleanup_entry,NULL);
-	put_cpu();
 	free_table_info(oldinfo);
 	if (copy_to_user(tmp.counters, counters,
 			 sizeof(struct ipt_counters) * tmp.num_counters) != 0)
@@ -1261,7 +1264,6 @@
  free_newinfo_counters:
 	vfree(counters);
  free_newinfo:
-	put_cpu();
 	free_table_info(newinfo);
 	return ret;
 }
@@ -1303,7 +1305,7 @@
 	if (len != sizeof(tmp) + tmp.num_counters*sizeof(struct ipt_counters))
 		return -EINVAL;
 
-	paddc = vmalloc(len);
+	paddc = vmalloc_node(len, numa_node_id());
 	if (!paddc)
 		return -ENOMEM;
 
@@ -1326,7 +1328,7 @@
 
 	i = 0;
 	/* Choose the copy that is on our node */
-	loc_cpu_entry = t->private->entries[smp_processor_id()];
+	loc_cpu_entry = t->private->entries[raw_smp_processor_id()];
 	IPT_ENTRY_ITERATE(loc_cpu_entry,
 			  t->private->size,
 			  add_counter_to_entry,
@@ -1525,8 +1527,10 @@
 	if (!newinfo)
 		return -ENOMEM;
 
-	/* choose the copy on our node/cpu */
-	loc_cpu_entry = newinfo->entries[get_cpu()];
+	/* choose the copy on our node/cpu
+	 * but dont care of preemption
+	 */
+	loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
 	memcpy(loc_cpu_entry, repl->entries, repl->size);
 
 	ret = translate_table(table->name, table->valid_hooks,
@@ -1534,7 +1538,6 @@
 			      repl->num_entries,
 			      repl->hook_entry,
 			      repl->underflow);
-	put_cpu();
 	if (ret != 0) {
 		free_table_info(newinfo);
 		return ret;
@@ -1584,10 +1587,9 @@
 	up(&ipt_mutex);
 
 	/* Decrease module usage counts and free resources */
-	loc_cpu_entry = table->private->entries[get_cpu()];
+	loc_cpu_entry = table->private->entries[raw_smp_processor_id()];
 	IPT_ENTRY_ITERATE(loc_cpu_entry, table->private->size,
 			  cleanup_entry, NULL);
-	put_cpu();
 	free_table_info(table->private);
 }
 
--- net-2.6.16-orig/net/ipv6/netfilter/ip6_tables.c	2005-11-25 10:24:02.000000000 +0100
+++ net-2.6.16/net/ipv6/netfilter/ip6_tables.c	2005-11-25 11:23:03.000000000 +0100
@@ -1074,11 +1074,14 @@
 {
 	unsigned int cpu;
 	unsigned int i;
-	unsigned int curcpu = get_cpu();
+	unsigned int curcpu;
 
 	/* Instead of clearing (by a previous call to memset())
 	 * the counters and using adds, we set the counters
-	 * with data used by current CPU */
+	 * with data used by 'current' CPU
+	 * We dont care about preemption here.
+	 */
+	curcpu = raw_smp_processor_id();
 
 	i = 0;
 	IP6T_ENTRY_ITERATE(t->entries[curcpu],
@@ -1097,7 +1100,6 @@
 				  counters,
 				  &i);
 	}
-	put_cpu();
 }
 
 static int
@@ -1126,7 +1128,7 @@
 	write_unlock_bh(&table->lock);
 
 	/* choose the copy that is on ourc node/cpu */
-	loc_cpu_entry = table->private->entries[get_cpu()];
+	loc_cpu_entry = table->private->entries[raw_smp_processor_id()];
 	if (copy_to_user(userptr, loc_cpu_entry, total_size) != 0) {
 		ret = -EFAULT;
 		goto free_counters;
@@ -1176,7 +1178,6 @@
 	}
 
  free_counters:
-	put_cpu();
 	vfree(counters);
 	return ret;
 }
@@ -1271,7 +1272,7 @@
 		return -ENOMEM;
 
 	/* choose the copy that is on our node/cpu */
-	loc_cpu_entry = newinfo->entries[get_cpu()];
+	loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
 	if (copy_from_user(loc_cpu_entry, user + sizeof(tmp),
 			   tmp.size) != 0) {
 		ret = -EFAULT;
@@ -1324,9 +1325,8 @@
 	/* Get the old counters. */
 	get_counters(oldinfo, counters);
 	/* Decrease module usage counts and free resource */
-	loc_cpu_old_entry = oldinfo->entries[smp_processor_id()];
+	loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
 	IP6T_ENTRY_ITERATE(loc_cpu_old_entry, oldinfo->size, cleanup_entry,NULL);
-	put_cpu();
 	free_table_info(oldinfo);
 	if (copy_to_user(tmp.counters, counters,
 			 sizeof(struct ip6t_counters) * tmp.num_counters) != 0)
@@ -1343,7 +1343,6 @@
  free_newinfo_counters:
 	vfree(counters);
  free_newinfo:
-	put_cpu();
 	free_table_info(newinfo);
 	return ret;
 }
@@ -1609,7 +1608,7 @@
 		return -ENOMEM;
 
 	/* choose the copy on our node/cpu */
-	loc_cpu_entry = newinfo->entries[get_cpu()];
+	loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
 	memcpy(loc_cpu_entry, repl->entries, repl->size);
 
 	ret = translate_table(table->name, table->valid_hooks,
@@ -1617,7 +1616,6 @@
 			      repl->num_entries,
 			      repl->hook_entry,
 			      repl->underflow);
-	put_cpu();
 	if (ret != 0) {
 		free_table_info(newinfo);
 		return ret;
@@ -1667,10 +1665,9 @@
 	up(&ip6t_mutex);
 
 	/* Decrease module usage counts and free resources */
-	loc_cpu_entry = table->private->entries[get_cpu()];
+	loc_cpu_entry = table->private->entries[raw_smp_processor_id()];
 	IP6T_ENTRY_ITERATE(loc_cpu_entry, table->private->size,
 			  cleanup_entry, NULL);
-	put_cpu();
 	free_table_info(table->private);
 }
 
--- net-2.6.16-orig/net/ipv4/netfilter/arp_tables.c	2005-11-25 10:24:02.000000000 +0100
+++ net-2.6.16/net/ipv4/netfilter/arp_tables.c	2005-11-25 11:15:50.000000000 +0100
@@ -814,11 +814,14 @@
 {
 	unsigned int cpu;
 	unsigned int i;
-	unsigned int curcpu = get_cpu();
+	unsigned int curcpu;
 
 	/* Instead of clearing (by a previous call to memset())
 	 * the counters and using adds, we set the counters
-	 * with data used by current CPU */
+	 * with data used by 'current' CPU
+	 * We dont care about preemption here.
+	 */
+	curcpu = raw_smp_processor_id();
 
 	i = 0;
 	ARPT_ENTRY_ITERATE(t->entries[curcpu],
@@ -837,7 +840,6 @@
 				   counters,
 				   &i);
 	}
-	put_cpu();
 }
 
 static int copy_entries_to_user(unsigned int total_size,
@@ -865,7 +867,7 @@
 	get_counters(table->private, counters);
 	write_unlock_bh(&table->lock);
 
-	loc_cpu_entry = table->private->entries[get_cpu()];
+	loc_cpu_entry = table->private->entries[raw_smp_processor_id()];
 	/* ... then copy entire thing ... */
 	if (copy_to_user(userptr, loc_cpu_entry, total_size) != 0) {
 		ret = -EFAULT;
@@ -898,7 +900,6 @@
 	}
 
  free_counters:
-	put_cpu();
 	vfree(counters);
 	return ret;
 }
@@ -996,7 +997,7 @@
 		return -ENOMEM;
 
 	/* choose the copy that is on our node/cpu */
-	loc_cpu_entry = newinfo->entries[get_cpu()];
+	loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
 	if (copy_from_user(loc_cpu_entry, user + sizeof(tmp),
 			   tmp.size) != 0) {
 		ret = -EFAULT;
@@ -1049,9 +1050,9 @@
 	/* Get the old counters. */
 	get_counters(oldinfo, counters);
 	/* Decrease module usage counts and free resource */
-	loc_cpu_old_entry = oldinfo->entries[smp_processor_id()];
+	loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
 	ARPT_ENTRY_ITERATE(loc_cpu_old_entry, oldinfo->size, cleanup_entry,NULL);
-	put_cpu();
+
 	free_table_info(oldinfo);
 	if (copy_to_user(tmp.counters, counters,
 			 sizeof(struct arpt_counters) * tmp.num_counters) != 0)
@@ -1068,7 +1069,6 @@
  free_newinfo_counters:
 	vfree(counters);
  free_newinfo:
-	put_cpu();
 	free_table_info(newinfo);
 	return ret;
 }
@@ -1295,7 +1295,7 @@
 	}
 
 	/* choose the copy on our node/cpu */
-	loc_cpu_entry = newinfo->entries[get_cpu()];
+	loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
 	memcpy(loc_cpu_entry, repl->entries, repl->size);
 
 	ret = translate_table(table->name, table->valid_hooks,
@@ -1303,7 +1303,6 @@
 			      repl->num_entries,
 			      repl->hook_entry,
 			      repl->underflow);
-	put_cpu();
 	duprintf("arpt_register_table: translate table gives %d\n", ret);
 	if (ret != 0) {
 		free_table_info(newinfo);
@@ -1354,10 +1353,9 @@
 	up(&arpt_mutex);
 
 	/* Decrease module usage counts and free resources */
-	loc_cpu_entry = table->private->entries[get_cpu()];
+	loc_cpu_entry = table->private->entries[raw_smp_processor_id()];
 	ARPT_ENTRY_ITERATE(loc_cpu_entry, table->private->size,
 			   cleanup_entry, NULL);
-	put_cpu();
 	free_table_info(table->private);
 }