[patch] powerpc: hash lock use lock bitops

Nick Piggin npiggin at suse.de
Tue Nov 20 17:26:19 EST 2007


On Tue, Nov 20, 2007 at 05:08:24PM +1100, Benjamin Herrenschmidt wrote:
> 
> On Tue, 2007-11-20 at 06:09 +0100, Nick Piggin wrote:
> > This isn't a bugfix, but may help performance slightly...
> > 
> > --
> > powerpc 64-bit hash pte lock bit is an actual lock, so it can take advantage
> > of lock bitops for slightly more optimal memory barriers (can avoid an lwsync
> > in the trylock).
> > 
> > Signed-off-by: Nick Piggin <npiggin at suse.de>
> > Acked-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> > ---
> 
> Looks nice, I'll try it out on a G5 and let you know.

Cool, thanks (I don't have mine handy ATM...).

BTW, here is another thing which you might want to think about. Again
untested for temporary lack of hardware.

--

The radix-tree is now RCU safe, so powerpc can avoid the games it was playing
in order to have a lockless readside. Saves an irq disable/enable, a couple of
__get_cpu_var()s, a cacheline, and a memory barrier, in the fastpath. Should
save a cycle or two...

---
Index: linux-2.6/arch/powerpc/kernel/irq.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/irq.c
+++ linux-2.6/arch/powerpc/kernel/irq.c
@@ -406,8 +406,6 @@ void do_softirq(void)
 
 static LIST_HEAD(irq_hosts);
 static DEFINE_SPINLOCK(irq_big_lock);
-static DEFINE_PER_CPU(unsigned int, irq_radix_reader);
-static unsigned int irq_radix_writer;
 struct irq_map_entry irq_map[NR_IRQS];
 static unsigned int irq_virq_count = NR_IRQS;
 static struct irq_host *irq_default_host;
@@ -550,57 +548,6 @@ void irq_set_virq_count(unsigned int cou
 		irq_virq_count = count;
 }
 
-/* radix tree not lockless safe ! we use a brlock-type mecanism
- * for now, until we can use a lockless radix tree
- */
-static void irq_radix_wrlock(unsigned long *flags)
-{
-	unsigned int cpu, ok;
-
-	spin_lock_irqsave(&irq_big_lock, *flags);
-	irq_radix_writer = 1;
-	smp_mb();
-	do {
-		barrier();
-		ok = 1;
-		for_each_possible_cpu(cpu) {
-			if (per_cpu(irq_radix_reader, cpu)) {
-				ok = 0;
-				break;
-			}
-		}
-		if (!ok)
-			cpu_relax();
-	} while(!ok);
-}
-
-static void irq_radix_wrunlock(unsigned long flags)
-{
-	smp_wmb();
-	irq_radix_writer = 0;
-	spin_unlock_irqrestore(&irq_big_lock, flags);
-}
-
-static void irq_radix_rdlock(unsigned long *flags)
-{
-	local_irq_save(*flags);
-	__get_cpu_var(irq_radix_reader) = 1;
-	smp_mb();
-	if (likely(irq_radix_writer == 0))
-		return;
-	__get_cpu_var(irq_radix_reader) = 0;
-	smp_wmb();
-	spin_lock(&irq_big_lock);
-	__get_cpu_var(irq_radix_reader) = 1;
-	spin_unlock(&irq_big_lock);
-}
-
-static void irq_radix_rdunlock(unsigned long flags)
-{
-	__get_cpu_var(irq_radix_reader) = 0;
-	local_irq_restore(flags);
-}
-
 static int irq_setup_virq(struct irq_host *host, unsigned int virq,
 			    irq_hw_number_t hwirq)
 {
@@ -791,9 +738,9 @@ void irq_dispose_mapping(unsigned int vi
 		/* Check if radix tree allocated yet */
 		if (host->revmap_data.tree.gfp_mask == 0)
 			break;
-		irq_radix_wrlock(&flags);
+		spin_lock_irqsave(&irq_big_lock, flags);
 		radix_tree_delete(&host->revmap_data.tree, hwirq);
-		irq_radix_wrunlock(flags);
+		spin_unlock_irqrestore(&irq_big_lock, flags);
 		break;
 	}
 
@@ -861,9 +808,9 @@ unsigned int irq_radix_revmap(struct irq
 		return irq_find_mapping(host, hwirq);
 
 	/* Now try to resolve */
-	irq_radix_rdlock(&flags);
+	rcu_read_lock();
 	ptr = radix_tree_lookup(tree, hwirq);
-	irq_radix_rdunlock(flags);
+	rcu_read_unlock();
 
 	/* Found it, return */
 	if (ptr) {
@@ -874,9 +821,9 @@ unsigned int irq_radix_revmap(struct irq
 	/* If not there, try to insert it */
 	virq = irq_find_mapping(host, hwirq);
 	if (virq != NO_IRQ) {
-		irq_radix_wrlock(&flags);
+		spin_lock_irqsave(&irq_big_lock, flags);
 		radix_tree_insert(tree, hwirq, &irq_map[virq]);
-		irq_radix_wrunlock(flags);
+		spin_unlock_irqrestore(&irq_big_lock, flags);
 	}
 	return virq;
 }
@@ -989,12 +936,12 @@ static int irq_late_init(void)
 	struct irq_host *h;
 	unsigned long flags;
 
-	irq_radix_wrlock(&flags);
+	spin_lock_irqsave(&irq_big_lock, flags);
 	list_for_each_entry(h, &irq_hosts, link) {
 		if (h->revmap_type == IRQ_HOST_MAP_TREE)
 			INIT_RADIX_TREE(&h->revmap_data.tree, GFP_ATOMIC);
 	}
-	irq_radix_wrunlock(flags);
+	spin_unlock_irqrestore(&irq_big_lock, flags);
 
 	return 0;
 }



More information about the Linuxppc-dev mailing list