[my_cpu_ptr 0/5] Introduce my_cpu_ptr/__my_cpu_ptr

[my_cpu_ptr 0/5] Introduce my_cpu_ptr/__my_cpu_ptr

[cl]

The problem with per_cpu_ptr(x, smp_processor_id) is that it requires
an array lookup to find the offset for the cpu. Processors typically
have the offset for the current cpu area already in some kind of
(arch dependent) efficiently accessible register or memory location.

We can use the per cpu offset instead of doing the array lookup to speed up the
determination of the address of the percpu variable. This is particularly
significant because these lookups occur in performance critical paths
of the core kernel.

This optimization is a prerequiste to the introduction of per processor
atomic operations for the core code. Atomic per processor operations
implicitly do the offset calculation to the current per cpu area in a
single instruction. The locations touched by this patchset are potential
candidates for atomic per cpu operations.

The patchset does not cover arch code nor exotic cases where the current
cpu is determined before the call into a function.

-- 

[my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[cl]

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


my_cpu_ptr(xx) = per_cpu_ptr(xx, smp_processor_id).


The problem with per_cpu_ptr(x, smp_processor_id) is that it requires
an array lookup to find the offset for the cpu. Processors typically
have the offset for the current cpu area in some kind of (arch dependent)
efficiently accessible register or memory location.

We can use that instead of doing the array lookup to speed up the
determination of the addressof the percpu variable. This is particularly
significant because these lookups occur in performance critical paths
of the core kernel.

This optimization is a prerequiste to the introduction of per processor
atomic operations for the core code. Atomic per processor operations
implicitly do the offset calculation to the current per cpu area in a
single instruction. All the locations touched by this patchset are potential
candidates for atomic per cpu operations.

my_cpu_ptr comes in two flavors. The preemption context matters since we
are referring the the currently executing processor. In many cases we must
insure that the processor does not change while a code segment is executed.

__my_cpu_ptr 	-> Do not check for preemption context
my_cpu_ptr	-> Check preemption context


Cc: David Howells <dhowells@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Eric Dumazet <dada1@cosmosbay.com>
Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 include/linux/percpu.h |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: linux-2.6/include/linux/percpu.h
===================================================================
--- linux-2.6.orig/include/linux/percpu.h	2009-05-27 11:32:36.000000000 -0500
+++ linux-2.6/include/linux/percpu.h	2009-05-27 11:33:32.000000000 -0500
@@ -78,6 +78,9 @@ extern ssize_t __init pcpu_embed_first_c
  */
 #define per_cpu_ptr(ptr, cpu)	SHIFT_PERCPU_PTR((ptr), per_cpu_offset((cpu)))
 
+#define my_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, my_cpu_offset)
+#define __my_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
+
 extern void *__alloc_reserved_percpu(size_t size, size_t align);
 
 #else /* CONFIG_HAVE_DYNAMIC_PER_CPU_AREA */
@@ -94,6 +97,12 @@ struct percpu_data {
         (__typeof__(ptr))__p->ptrs[(cpu)];				\
 })
 
+#define my_cpu_ptr(ptr) \
+		per_cpu_ptr(ptr, smp_processor_id())
+
+#define __my_cpu_ptr(ptr) \
+		per_cpu_ptr(ptr, raw_smp_processor_id())
+
 #endif /* CONFIG_HAVE_DYNAMIC_PER_CPU_AREA */
 
 extern void *__alloc_percpu(size_t size, size_t align);
@@ -102,6 +111,8 @@ extern void free_percpu(void *__pdata);
 #else /* CONFIG_SMP */
 
 #define per_cpu_ptr(ptr, cpu) ({ (void)(cpu); (ptr); })
+#define my_cpu_ptr(ptr) per_cpu_ptr(ptr, 0)
+#define __my_cpu_ptr(ptr) my_cpu_ptr(ptr)
 
 static inline void *__alloc_percpu(size_t size, size_t align)
 {

-- 

[my_cpu_ptr 2/5] Straight transformations

[cl]

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

Use my_cpu_ptr and __my_cpu_ptr in locations where straight
transformations are possible because per_cpu_ptr is used with
either smp_processor_id() or raw_smp_processor_id().

Cc: David Howells <dhowells@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Eric Dumazet <dada1@cosmosbay.com>
Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 drivers/infiniband/hw/ehca/ehca_irq.c       |    3 +--
 drivers/net/chelsio/sge.c                   |    5 ++---
 drivers/net/loopback.c                      |    2 +-
 fs/ext4/mballoc.c                           |    2 +-
 include/net/neighbour.h                     |    2 +-
 include/net/netfilter/nf_conntrack.h        |    4 ++--
 include/net/netfilter/nf_conntrack_ecache.h |    2 +-
 include/net/snmp.h                          |    4 ++--
 8 files changed, 11 insertions(+), 13 deletions(-)

Index: linux-2.6/drivers/net/chelsio/sge.c
===================================================================
--- linux-2.6.orig/drivers/net/chelsio/sge.c	2009-05-27 11:39:44.000000000 -0500
+++ linux-2.6/drivers/net/chelsio/sge.c	2009-05-27 11:40:25.000000000 -0500
@@ -1378,7 +1378,7 @@ static void sge_rx(struct sge *sge, stru
 	}
 	__skb_pull(skb, sizeof(*p));
 
-	st = per_cpu_ptr(sge->port_stats[p->iff], smp_processor_id());
+	st = my_cpu_ptr(sge->port_stats[p->iff]);
 
 	skb->protocol = eth_type_trans(skb, adapter->port[p->iff].dev);
 	if ((adapter->flags & RX_CSUM_ENABLED) && p->csum == 0xffff &&
@@ -1780,8 +1780,7 @@ int t1_start_xmit(struct sk_buff *skb, s
 {
 	struct adapter *adapter = dev->ml_priv;
 	struct sge *sge = adapter->sge;
-	struct sge_port_stats *st = per_cpu_ptr(sge->port_stats[dev->if_port],
-						smp_processor_id());
+	struct sge_port_stats *st = my_cpu_ptr(sge->port_stats[dev->if_port]);
 	struct cpl_tx_pkt *cpl;
 	struct sk_buff *orig_skb = skb;
 	int ret;
Index: linux-2.6/drivers/net/loopback.c
===================================================================
--- linux-2.6.orig/drivers/net/loopback.c	2009-05-27 11:39:44.000000000 -0500
+++ linux-2.6/drivers/net/loopback.c	2009-05-27 11:40:12.000000000 -0500
@@ -78,7 +78,7 @@ static int loopback_xmit(struct sk_buff 
 
 	/* it's OK to use per_cpu_ptr() because BHs are off */
 	pcpu_lstats = dev->ml_priv;
-	lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
+	lb_stats = my_cpu_ptr(pcpu_lstats);
 	lb_stats->bytes += skb->len;
 	lb_stats->packets++;
 
Index: linux-2.6/fs/ext4/mballoc.c
===================================================================
--- linux-2.6.orig/fs/ext4/mballoc.c	2009-05-27 11:39:44.000000000 -0500
+++ linux-2.6/fs/ext4/mballoc.c	2009-05-27 11:40:12.000000000 -0500
@@ -4210,7 +4210,7 @@ static void ext4_mb_group_or_file(struct
 	 * per cpu locality group is to reduce the contention between block
 	 * request from multiple CPUs.
 	 */
-	ac->ac_lg = per_cpu_ptr(sbi->s_locality_groups, raw_smp_processor_id());
+	ac->ac_lg = __my_cpu_ptr(sbi->s_locality_groups);
 
 	/* we're going to use group allocation */
 	ac->ac_flags |= EXT4_MB_HINT_GROUP_ALLOC;
Index: linux-2.6/include/net/neighbour.h
===================================================================
--- linux-2.6.orig/include/net/neighbour.h	2009-05-27 11:39:44.000000000 -0500
+++ linux-2.6/include/net/neighbour.h	2009-05-27 11:40:12.000000000 -0500
@@ -92,7 +92,7 @@ struct neigh_statistics
 #define NEIGH_CACHE_STAT_INC(tbl, field)				\
 	do {								\
 		preempt_disable();					\
-		(per_cpu_ptr((tbl)->stats, smp_processor_id())->field)++; \
+		(my_cpu_ptr((tbl)->stats)->field)++;			\
 		preempt_enable();					\
 	} while (0)
 
Index: linux-2.6/include/net/netfilter/nf_conntrack.h
===================================================================
--- linux-2.6.orig/include/net/netfilter/nf_conntrack.h	2009-05-27 11:39:44.000000000 -0500
+++ linux-2.6/include/net/netfilter/nf_conntrack.h	2009-05-27 11:40:12.000000000 -0500
@@ -292,11 +292,11 @@ extern unsigned int nf_conntrack_htable_
 extern unsigned int nf_conntrack_max;
 
 #define NF_CT_STAT_INC(net, count)	\
-	(per_cpu_ptr((net)->ct.stat, raw_smp_processor_id())->count++)
+	(__my_cpu_ptr((net)->ct.stat)->count++)
 #define NF_CT_STAT_INC_ATOMIC(net, count)		\
 do {							\
 	local_bh_disable();				\
-	per_cpu_ptr((net)->ct.stat, raw_smp_processor_id())->count++;	\
+	__my_cpu_ptr((net)->ct.stat)->count++;		\
 	local_bh_enable();				\
 } while (0)
 
Index: linux-2.6/include/net/netfilter/nf_conntrack_ecache.h
===================================================================
--- linux-2.6.orig/include/net/netfilter/nf_conntrack_ecache.h	2009-05-27 11:39:44.000000000 -0500
+++ linux-2.6/include/net/netfilter/nf_conntrack_ecache.h	2009-05-27 11:40:12.000000000 -0500
@@ -39,7 +39,7 @@ nf_conntrack_event_cache(enum ip_conntra
 	struct nf_conntrack_ecache *ecache;
 
 	local_bh_disable();
-	ecache = per_cpu_ptr(net->ct.ecache, raw_smp_processor_id());
+	ecache = __my_cpu_ptr(net->ct.ecache);
 	if (ct != ecache->ct)
 		__nf_ct_event_cache_init(ct);
 	ecache->events |= event;
Index: linux-2.6/drivers/infiniband/hw/ehca/ehca_irq.c
===================================================================
--- linux-2.6.orig/drivers/infiniband/hw/ehca/ehca_irq.c	2009-05-27 11:40:23.000000000 -0500
+++ linux-2.6/drivers/infiniband/hw/ehca/ehca_irq.c	2009-05-27 11:40:25.000000000 -0500
@@ -827,8 +827,7 @@ static void __cpuinit take_over_work(str
 		cq = list_entry(cct->cq_list.next, struct ehca_cq, entry);
 
 		list_del(&cq->entry);
-		__queue_comp_task(cq, per_cpu_ptr(pool->cpu_comp_tasks,
-						  smp_processor_id()));
+		__queue_comp_task(cq, my_cpu_ptr(pool->cpu_comp_tasks));
 	}
 
 	spin_unlock_irqrestore(&cct->task_lock, flags_cct);
Index: linux-2.6/include/net/snmp.h
===================================================================
--- linux-2.6.orig/include/net/snmp.h	2009-05-27 11:40:23.000000000 -0500
+++ linux-2.6/include/net/snmp.h	2009-05-27 11:40:25.000000000 -0500
@@ -137,7 +137,7 @@ struct linux_xfrm_mib {
 #define SNMP_STAT_USRPTR(name)	(name[1])
 
 #define SNMP_INC_STATS_BH(mib, field) 	\
-	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field]++)
+	(__my_cpu_ptr(mib[0])->mibs[field]++)
 #define SNMP_INC_STATS_USER(mib, field) \
 	do { \
 		per_cpu_ptr(mib[1], get_cpu())->mibs[field]++; \
@@ -154,7 +154,7 @@ struct linux_xfrm_mib {
 		put_cpu(); \
 	} while (0)
 #define SNMP_ADD_STATS_BH(mib, field, addend) 	\
-	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field] += addend)
+	(__my_cpu_ptr(mib[0])->mibs[field] += addend)
 #define SNMP_ADD_STATS_USER(mib, field, addend) 	\
 	do { \
 		per_cpu_ptr(mib[1], get_cpu())->mibs[field] += addend; \

-- 

[my_cpu_ptr 3/5] Elimninate get/put_cpu

[cl]

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

There are cases where we can use my_cpu_ptr and as the result
of using my_cpu_ptr we no longer need to determine the
current executing cpu.

In those places no get/put_cpu combination is needed anymore.
The local cpu variable can be eliminated.

Preemption still needs to be disabled and enabled since the
modifications of the per cpu variables is not atomic. There may
be multiple per cpu variables modified and those must all
be from the same processor.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Eric Biederman <ebiederm@aristanetworks.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: David L Stevens <dlstevens@us.ibm.com>
Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 drivers/dma/dmaengine.c |   34 +++++++++++++++-------------------
 drivers/net/veth.c      |    7 +++----
 fs/nfs/iostat.h         |   21 +++++++++------------
 include/net/snmp.h      |   31 +++++++++++++++++--------------
 4 files changed, 44 insertions(+), 49 deletions(-)

Index: linux-2.6/drivers/dma/dmaengine.c
===================================================================
--- linux-2.6.orig/drivers/dma/dmaengine.c	2009-05-27 11:47:09.000000000 -0500
+++ linux-2.6/drivers/dma/dmaengine.c	2009-05-27 11:48:42.000000000 -0500
@@ -327,11 +327,10 @@ arch_initcall(dma_channel_table_init);
 struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type)
 {
 	struct dma_chan *chan;
-	int cpu;
 
-	cpu = get_cpu();
-	chan = per_cpu_ptr(channel_table[tx_type], cpu)->chan;
-	put_cpu();
+	preempt_disable()
+	chan = my_cpu_ptr(channel_table[tx_type])->chan;
+	preempt_enable();
 
 	return chan;
 }
@@ -803,7 +802,6 @@ dma_async_memcpy_buf_to_buf(struct dma_c
 	struct dma_async_tx_descriptor *tx;
 	dma_addr_t dma_dest, dma_src;
 	dma_cookie_t cookie;
-	int cpu;
 	unsigned long flags;
 
 	dma_src = dma_map_single(dev->dev, src, len, DMA_TO_DEVICE);
@@ -822,10 +820,10 @@ dma_async_memcpy_buf_to_buf(struct dma_c
 	tx->callback = NULL;
 	cookie = tx->tx_submit(tx);
 
-	cpu = get_cpu();
-	per_cpu_ptr(chan->local, cpu)->bytes_transferred += len;
-	per_cpu_ptr(chan->local, cpu)->memcpy_count++;
-	put_cpu();
+	preempt_disable();
+	my_cpu_ptr(chan->local)->bytes_transferred += len;
+	my_cpu_ptr(chan->local)->memcpy_count++;
+	preempt_enable();
 
 	return cookie;
 }
@@ -852,7 +850,6 @@ dma_async_memcpy_buf_to_pg(struct dma_ch
 	struct dma_async_tx_descriptor *tx;
 	dma_addr_t dma_dest, dma_src;
 	dma_cookie_t cookie;
-	int cpu;
 	unsigned long flags;
 
 	dma_src = dma_map_single(dev->dev, kdata, len, DMA_TO_DEVICE);
@@ -869,10 +866,10 @@ dma_async_memcpy_buf_to_pg(struct dma_ch
 	tx->callback = NULL;
 	cookie = tx->tx_submit(tx);
 
-	cpu = get_cpu();
-	per_cpu_ptr(chan->local, cpu)->bytes_transferred += len;
-	per_cpu_ptr(chan->local, cpu)->memcpy_count++;
-	put_cpu();
+	preempt_disable();
+	my_cpu_ptr(chan->local)->bytes_transferred += len;
+	my_cpu_ptr(chan->local)->memcpy_count++;
+	preempt_enable();
 
 	return cookie;
 }
@@ -901,7 +898,6 @@ dma_async_memcpy_pg_to_pg(struct dma_cha
 	struct dma_async_tx_descriptor *tx;
 	dma_addr_t dma_dest, dma_src;
 	dma_cookie_t cookie;
-	int cpu;
 	unsigned long flags;
 
 	dma_src = dma_map_page(dev->dev, src_pg, src_off, len, DMA_TO_DEVICE);
@@ -919,10 +915,10 @@ dma_async_memcpy_pg_to_pg(struct dma_cha
 	tx->callback = NULL;
 	cookie = tx->tx_submit(tx);
 
-	cpu = get_cpu();
-	per_cpu_ptr(chan->local, cpu)->bytes_transferred += len;
-	per_cpu_ptr(chan->local, cpu)->memcpy_count++;
-	put_cpu();
+	preempt_disable();
+	my_cpu_ptr(chan->local)->bytes_transferred += len;
+	my_cpu_ptr(chan->local)->memcpy_count++;
+	preempt_disable();
 
 	return cookie;
 }
Index: linux-2.6/drivers/net/veth.c
===================================================================
--- linux-2.6.orig/drivers/net/veth.c	2009-05-27 11:47:09.000000000 -0500
+++ linux-2.6/drivers/net/veth.c	2009-05-27 11:48:42.000000000 -0500
@@ -153,7 +153,7 @@ static int veth_xmit(struct sk_buff *skb
 	struct net_device *rcv = NULL;
 	struct veth_priv *priv, *rcv_priv;
 	struct veth_net_stats *stats, *rcv_stats;
-	int length, cpu;
+	int length;
 
 	skb_orphan(skb);
 
@@ -161,9 +161,8 @@ static int veth_xmit(struct sk_buff *skb
 	rcv = priv->peer;
 	rcv_priv = netdev_priv(rcv);
 
-	cpu = smp_processor_id();
-	stats = per_cpu_ptr(priv->stats, cpu);
-	rcv_stats = per_cpu_ptr(rcv_priv->stats, cpu);
+	stats = my_cpu_ptr(priv->stats);
+	rcv_stats = my_cpu_ptr(rcv_priv->stats);
 
 	if (!(rcv->flags & IFF_UP))
 		goto tx_drop;
Index: linux-2.6/fs/nfs/iostat.h
===================================================================
--- linux-2.6.orig/fs/nfs/iostat.h	2009-05-27 11:47:09.000000000 -0500
+++ linux-2.6/fs/nfs/iostat.h	2009-05-27 11:48:42.000000000 -0500
@@ -26,12 +26,11 @@ static inline void nfs_inc_server_stats(
 					enum nfs_stat_eventcounters stat)
 {
 	struct nfs_iostats *iostats;
-	int cpu;
 
-	cpu = get_cpu();
-	iostats = per_cpu_ptr(server->io_stats, cpu);
+	preempt_disable();
+	iostats = my_cpu_ptr(server->io_stats);
 	iostats->events[stat]++;
-	put_cpu_no_resched();
+	preempt_enable_no_resched();
 }
 
 static inline void nfs_inc_stats(const struct inode *inode,
@@ -45,12 +44,11 @@ static inline void nfs_add_server_stats(
 					unsigned long addend)
 {
 	struct nfs_iostats *iostats;
-	int cpu;
 
-	cpu = get_cpu();
-	iostats = per_cpu_ptr(server->io_stats, cpu);
+	preempt_disable();
+	iostats = my_cpu_ptr(server->io_stats);
 	iostats->bytes[stat] += addend;
-	put_cpu_no_resched();
+	preempt_enable_no_resched();
 }
 
 static inline void nfs_add_stats(const struct inode *inode,
@@ -66,12 +64,11 @@ static inline void nfs_add_fscache_stats
 					 unsigned long addend)
 {
 	struct nfs_iostats *iostats;
-	int cpu;
 
-	cpu = get_cpu();
-	iostats = per_cpu_ptr(NFS_SERVER(inode)->io_stats, cpu);
+	preempt_disable();
+	iostats = my_cpu_ptr(NFS_SERVER(inode)->io_stats);
 	iostats->fscache[stat] += addend;
-	put_cpu_no_resched();
+	preempt_enable_no_resched();
 }
 #endif
 
Index: linux-2.6/include/net/snmp.h
===================================================================
--- linux-2.6.orig/include/net/snmp.h	2009-05-27 11:47:09.000000000 -0500
+++ linux-2.6/include/net/snmp.h	2009-05-27 11:48:42.000000000 -0500
@@ -139,26 +139,29 @@ struct linux_xfrm_mib {
 #define SNMP_INC_STATS_BH(mib, field) 	\
 	(__my_cpu_ptr(mib[0])->mibs[field]++)
 #define SNMP_INC_STATS_USER(mib, field) \
-	do { \
-		per_cpu_ptr(mib[1], get_cpu())->mibs[field]++; \
-		put_cpu(); \
-	} while (0)
+ 	do { \
+		preempt_disable();				\
+		my_cpu_ptr(mib[1])->mibs[field]++;		\
+		preempt_enable();				\
+  	} while (0)
 #define SNMP_INC_STATS(mib, field) 	\
-	do { \
-		per_cpu_ptr(mib[!in_softirq()], get_cpu())->mibs[field]++; \
-		put_cpu(); \
-	} while (0)
+ 	do { \
+		preempt_disable();				\
+		my_cpu_ptr(mib[!in_softirq()])->mibs[field]++;	\
+		preempt_enable(); \
+ 	} while (0)
 #define SNMP_DEC_STATS(mib, field) 	\
 	do { \
-		per_cpu_ptr(mib[!in_softirq()], get_cpu())->mibs[field]--; \
-		put_cpu(); \
+		preempt_disable();				\
+		my_cpu_ptr(mib[!in_softirq()])->mibs[field]--;	\
+		preempt_enable();				\
 	} while (0)
 #define SNMP_ADD_STATS_BH(mib, field, addend) 	\
 	(__my_cpu_ptr(mib[0])->mibs[field] += addend)
 #define SNMP_ADD_STATS_USER(mib, field, addend) 	\
-	do { \
-		per_cpu_ptr(mib[1], get_cpu())->mibs[field] += addend; \
-		put_cpu(); \
+ 	do { \
+		preempt_disable();				\
+		my_cpu_ptr(mib[1])->mibs[field] += addend;	\
+		preempt_enable();				\
 	} while (0)
-
 #endif

-- 

[my_cpu_ptr 4/5] sda_icsb_modify_counters() does not need a "cpu" variable

[cl]

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

The xfs_icsb_modify_counters() function no longer needs the cpu variable
if we use my_cpu_ptr() and we can get rid of get/put_cpu().

Cc: Christoph Hellwig <hch@lst.de>
Cc: Olaf Weber <olaf@sgi.com>
Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 fs/xfs/xfs_mount.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Index: linux-2.6/fs/xfs/xfs_mount.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_mount.c	2009-05-27 11:54:37.000000000 -0500
+++ linux-2.6/fs/xfs/xfs_mount.c	2009-05-27 11:55:10.000000000 -0500
@@ -2320,12 +2320,12 @@ xfs_icsb_modify_counters(
 {
 	xfs_icsb_cnts_t	*icsbp;
 	long long	lcounter;	/* long counter for 64 bit fields */
-	int		cpu, ret = 0;
+	int		ret = 0;
 
 	might_sleep();
 again:
-	cpu = get_cpu();
-	icsbp = (xfs_icsb_cnts_t *)per_cpu_ptr(mp->m_sb_cnts, cpu);
+	preempt_disable();
+	icsbp = (xfs_icsb_cnts_t *)my_cpu_ptr(mp->m_sb_cnts);
 
 	/*
 	 * if the counter is disabled, go to slow path
@@ -2369,11 +2369,11 @@ again:
 		break;
 	}
 	xfs_icsb_unlock_cntr(icsbp);
-	put_cpu();
+	preempt_enable();
 	return 0;
 
 slow_path:
-	put_cpu();
+	preempt_enable();
 
 	/*
 	 * serialise with a mutex so we don't burn lots of cpu on
@@ -2421,7 +2421,7 @@ slow_path:
 
 balance_counter:
 	xfs_icsb_unlock_cntr(icsbp);
-	put_cpu();
+	preempt_enable();
 
 	/*
 	 * We may have multiple threads here if multiple per-cpu

-- 

[my_cpu_ptr 5/5] Use my_cpu_ptr in crypto subsystem

[cl]

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

Just a slight optimization that removes one array lookup.
The processor number is needed for other things as well so the
get/put_cpu cannot be removed.

Cc: Huang Ying <ying.huang@intel.com>
Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 crypto/cryptd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/crypto/cryptd.c
===================================================================
--- linux-2.6.orig/crypto/cryptd.c	2009-05-27 11:55:20.000000000 -0500
+++ linux-2.6/crypto/cryptd.c	2009-05-27 11:56:55.000000000 -0500
@@ -93,7 +93,7 @@ static int cryptd_enqueue_request(struct
 	struct cryptd_cpu_queue *cpu_queue;
 
 	cpu = get_cpu();
-	cpu_queue = per_cpu_ptr(queue->cpu_queue, cpu);
+	cpu_queue = my_cpu_ptr(queue->cpu_queue);
 	err = crypto_enqueue_request(&cpu_queue->queue, request);
 	queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
 	put_cpu();

-- 

Re: [my_cpu_ptr 3/5] Elimninate get/put_cpu

[Christoph Lameter]

dmaengine.c was not compiled.... Fixes:

Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 drivers/dma/dmaengine.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/dma/dmaengine.c
===================================================================
--- linux-2.6.orig/drivers/dma/dmaengine.c	2009-05-27 14:30:32.000000000 -0500
+++ linux-2.6/drivers/dma/dmaengine.c	2009-05-27 14:31:19.000000000 -0500
@@ -328,7 +328,7 @@ struct dma_chan *dma_find_channel(enum d
 {
 	struct dma_chan *chan;

-	preempt_disable()
+	preempt_disable();
 	chan = my_cpu_ptr(channel_table[tx_type])->chan;
 	preempt_enable();

@@ -918,7 +918,7 @@ dma_async_memcpy_pg_to_pg(struct dma_cha
 	preempt_disable();
 	my_cpu_ptr(chan->local)->bytes_transferred += len;
 	my_cpu_ptr(chan->local)->memcpy_count++;
-	preempt_disable();
+	preempt_enable();

 	return cookie;
 }

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Rusty Russell]

On Thu, 28 May 2009 03:16:59 am cl@linux-foundation.org wrote:
> my_cpu_ptr(xx) = per_cpu_ptr(xx, smp_processor_id).

I had this implemented earlier as as get_cpu_ptr()/__get_cpu_ptr(), to match 
get_cpu_var() / __get_cpu_var().

But other than that nomenclature quibble, it looks fine!

Thanks,
Rusty.

Re: [my_cpu_ptr 4/5] sda_icsb_modify_counters() does not need a "cpu" variable

[Olaf Weber]

cl  writes:

> The xfs_icsb_modify_counters() function no longer needs the cpu variable
> if we use my_cpu_ptr() and we can get rid of get/put_cpu().

> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Olaf Weber <olaf@sgi.com>
> Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

Looks fine to me.

Acked-By: Olaf Weber <olaf@sgi.com>

-- 
Olaf Weber                 SGI               Phone:  +31(0)30-6696752
                           Veldzigt 2b       Fax:    +31(0)30-6696799
Technical Lead             3454 PW de Meern  Vnet:   955-7151
Storage Software           The Netherlands   Email:  olaf@sgi.com

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Christoph Lameter]

On Thu, 28 May 2009, Rusty Russell wrote:

> On Thu, 28 May 2009 03:16:59 am cl@linux-foundation.org wrote:
> > my_cpu_ptr(xx) = per_cpu_ptr(xx, smp_processor_id).
>
> I had this implemented earlier as as get_cpu_ptr()/__get_cpu_ptr(), to match
> get_cpu_var() / __get_cpu_var().

Have not seen it but it would be a bit confusing since
we already have get_cpu* which must be paired with put_cpu*
because of the refcount taking (get_cpu_var and get_cpu).
get_cpu_ptr() would not have to be paired.

Better use a different name.

	my_cpu_ptr came from my_cpu_offset:

#define my_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, my_cpu_offset)
#define __my_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Christoph Hellwig]

On Thu, May 28, 2009 at 01:16:01PM +0930, Rusty Russell wrote:
> On Thu, 28 May 2009 03:16:59 am cl@linux-foundation.org wrote:
> > my_cpu_ptr(xx) = per_cpu_ptr(xx, smp_processor_id).
> 
> I had this implemented earlier as as get_cpu_ptr()/__get_cpu_ptr(), to match 
> get_cpu_var() / __get_cpu_var().
> 
> But other than that nomenclature quibble, it looks fine!

my_ seems a very odd naming.  We have a lot of this_cpu naming for the
current cpu i nthe ctree, so I would suggest sticking to that.

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Christoph Lameter]

On Thu, 28 May 2009, Christoph Hellwig wrote:

> On Thu, May 28, 2009 at 01:16:01PM +0930, Rusty Russell wrote:
> > On Thu, 28 May 2009 03:16:59 am cl@linux-foundation.org wrote:
> > > my_cpu_ptr(xx) = per_cpu_ptr(xx, smp_processor_id).
> >
> > I had this implemented earlier as as get_cpu_ptr()/__get_cpu_ptr(), to match
> > get_cpu_var() / __get_cpu_var().
> >
> > But other than that nomenclature quibble, it looks fine!
>
> my_ seems a very odd naming.  We have a lot of this_cpu naming for the
> current cpu i nthe ctree, so I would suggest sticking to that.

So this_cpu is taken. I used THIS_CPU in earlier version
but got complaints about uppercase use. Is

this_cpu_ptr()

and

__this_cpu_ptr()

ok?

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Rusty Russell]

On Fri, 29 May 2009 01:29:31 am Christoph Lameter wrote:
> On Thu, 28 May 2009, Rusty Russell wrote:
> > On Thu, 28 May 2009 03:16:59 am cl@linux-foundation.org wrote:
> > > my_cpu_ptr(xx) = per_cpu_ptr(xx, smp_processor_id).
> >
> > I had this implemented earlier as as get_cpu_ptr()/__get_cpu_ptr(), to
> > match get_cpu_var() / __get_cpu_var().
>
> Have not seen it but it would be a bit confusing since
> we already have get_cpu* which must be paired with put_cpu*
> because of the refcount taking (get_cpu_var and get_cpu).
> get_cpu_ptr() would not have to be paired.

To clarify, get_cpu_ptr() would be paired with put_cpu_ptr().  __get_cpu_ptr() 
would be the "raw" one:

#define get_cpu_ptr(xx) per_cpu_ptr(xx, get_cpu())
#define __get_cpu_ptr(xx) per_cpu_ptr(xx, smp_processor_id())

> Better use a different name.
>
> 	my_cpu_ptr came from my_cpu_offset:

Yep, but that's not as widely exposed as get_cpu & get_cpu_var.

Cheers,
Rusty.

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Tejun Heo]

Christoph Lameter wrote:
> On Thu, 28 May 2009, Christoph Hellwig wrote:
> 
>> On Thu, May 28, 2009 at 01:16:01PM +0930, Rusty Russell wrote:
>>> On Thu, 28 May 2009 03:16:59 am cl@linux-foundation.org wrote:
>>>> my_cpu_ptr(xx) = per_cpu_ptr(xx, smp_processor_id).
>>> I had this implemented earlier as as get_cpu_ptr()/__get_cpu_ptr(), to match
>>> get_cpu_var() / __get_cpu_var().
>>>
>>> But other than that nomenclature quibble, it looks fine!
>> my_ seems a very odd naming.  We have a lot of this_cpu naming for the
>> current cpu i nthe ctree, so I would suggest sticking to that.
> 
> So this_cpu is taken. I used THIS_CPU in earlier version
> but got complaints about uppercase use. Is
> 
> this_cpu_ptr()
> 
> and
> 
> __this_cpu_ptr()
> 
> ok?

Yeap, those look fine to me.  Sans the renaming, the series generally
looks good but I didn't review each conversion.

For the first patch, Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Christoph Lameter]

On Fri, 29 May 2009, Rusty Russell wrote:

> > Have not seen it but it would be a bit confusing since
> > we already have get_cpu* which must be paired with put_cpu*
> > because of the refcount taking (get_cpu_var and get_cpu).
> > get_cpu_ptr() would not have to be paired.
>
> To clarify, get_cpu_ptr() would be paired with put_cpu_ptr().  __get_cpu_ptr()
> would be the "raw" one:
>
> #define get_cpu_ptr(xx) per_cpu_ptr(xx, get_cpu())
> #define __get_cpu_ptr(xx) per_cpu_ptr(xx, smp_processor_id())

Hmmm.. That would be a major change in semantics. I tend to
think about these ops more as atomic like operations rather than preempt
sections. The operation to obtain a the current instance for a given
processor is an operation done in a given preemption context. The
get_cpu_ptr() approach establishes the preemption context during the same
operation.

That would mean use

	*get_cpu_ptr(per_cpu_ptr1)++
	*__get_cpu_ptr(per_cpu_ptr2)++
	put_cpu_ptr()

instead of

	preempt_disable()
	*__this_cpu_ptr(per_cpu_ptr1)++
	*__this_cpu_ptr(per_cpu_ptr2)++
	preempt_enable().


And for context in which we know that preemption is off

	*__get_cpu_ptr(per_cpu_ptr1)++
	*__get_cpu_ptr(per_cpu_ptr2)++

vs

	*__this_cpu_ptr(per_cpu_ptr2)++
	*__this_cpu_ptr(per_cpu_ptr2)++

So its the same

How would that look for atomic per cpu ops?

	get_cpu_ptr_inc(per_cpu_ptr1);
	__get_cpu_ptr_inc(per_cpu_ptr2)
	put_cpu_ptr()

vs.

	this_cpu_inc(per_cpu_ptr1)
	this_cpu_inc(per_cpu_ptr2)


and known preemption off context

	__get_cpu_ptr_inc(per_cpu_ptr1)
	__get_cpu_ptr_inc(per_cpu_ptr2)

vs
	__this_cpu_inc(percpu_ptr1)
	__this_cpu_inc(percpu_ptr2)

Same again

Seems to be added complexity to also push a change of preemption into
these operations. I'd rather keep the preempt status unchanged by these
operations in particular because some counters may need interrupt
protection on some platforms and not on others. We will likely need some
additional operations like

	this_cpu_inc_irqsafe( ....)

that will require an irq off / on on some platforms and be a simple
increment on others.

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Rusty Russell]

On Sat, 30 May 2009 01:07:48 am Christoph Lameter wrote:
> On Fri, 29 May 2009, Rusty Russell wrote:
> > > Have not seen it but it would be a bit confusing since
> > > we already have get_cpu* which must be paired with put_cpu*
> > > because of the refcount taking (get_cpu_var and get_cpu).
> > > get_cpu_ptr() would not have to be paired.
> >
> > To clarify, get_cpu_ptr() would be paired with put_cpu_ptr(). 
> > __get_cpu_ptr() would be the "raw" one:
> >
> > #define get_cpu_ptr(xx) per_cpu_ptr(xx, get_cpu())
> > #define __get_cpu_ptr(xx) per_cpu_ptr(xx, smp_processor_id())
>
> Hmmm.. That would be a major change in semantics.

It's exactly like get_cpu_var.  For better or worse, let's not invent YA new 
convention.

> How would that look for atomic per cpu ops?
>
> 	get_cpu_ptr_inc(per_cpu_ptr1);
> 	__get_cpu_ptr_inc(per_cpu_ptr2)
> 	put_cpu_ptr()
>
> vs.
>
> 	this_cpu_inc(per_cpu_ptr1)
> 	this_cpu_inc(per_cpu_ptr2)

Well, get_* doesn't really make sense for any function which doesn't return a 
value.  

So that name question doesn't really have a clear convention answer: we could 
re-use cpu_local_inc() since I think we decided to kill local_t.  I slightly 
prefer it over "this_cpu_*" since we're not actually doing anything to the cpu 
itself, but I don't think anyone will get too confused and think that after 
this executes their CPU will be stepping 11. :)

Thanks,
Rusty.

Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

[Christoph Lameter]

On Sun, 31 May 2009, Rusty Russell wrote:

> > Hmmm.. That would be a major change in semantics.
>
> It's exactly like get_cpu_var.  For better or worse, let's not invent YA new
> convention.

It hink we are doing that if we go the get_cpu_ptr line. this_cpu_ptr
starts from per_cpu_ptr() and its straightforward as a special case of
per_cpu_ptr.

> So that name question doesn't really have a clear convention answer: we could
> re-use cpu_local_inc() since I think we decided to kill local_t.  I slightly
> prefer it over "this_cpu_*" since we're not actually doing anything to the cpu
> itself, but I don't think anyone will get too confused and think that after
> this executes their CPU will be stepping 11. :)

local is loaded with meaning by glibc where it means thread local. The
variables here are cpu specific, not task specific.