[PATCH 1/4] sparc: _sparc_find_resource() should check for exact matches

[PATCH 1/4] sparc: _sparc_find_resource() should check for exact matches

[Geert Uytterhoeven]

The address that's passed to _sparc_find_resource() should always be the
start address of a resource:
  - iounmap() passes a page-aligned virtual address, while the original
    address was created by adding the in-page offset to the resource's
    start address,
  - sbus_free_coherent() and pci32_free_coherent() should be passed an
    address obtained from sbus_alloc_coherent() resp. pci32_alloc_coherent(),
    which is always a resource's start address.

Hence replace the range check by a check for an exact match.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/sparc/kernel/ioport.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
index c6ce9a6..91e8c76 100644
--- a/arch/sparc/kernel/ioport.c
+++ b/arch/sparc/kernel/ioport.c
@@ -733,12 +733,12 @@ static const struct file_operations sparc_io_proc_fops = {
  * This probably warrants some sort of hashing.
  */
 static struct resource *_sparc_find_resource(struct resource *root,
-					     unsigned long hit)
+					     unsigned long start)
 {
 	struct resource *tmp;
 
 	for (tmp = root->child; tmp != 0; tmp = tmp->sibling) {
-		if (tmp->start <= hit && tmp->end >= hit)
+		if (tmp->start == start)
 			return tmp;
 	}
 	return NULL;
-- 
1.7.0.4

[PATCH 2/4] resources: Add lookup_resource()

[Geert Uytterhoeven]

Add a function to find an existing resource by a resource start address.
This allows to implement simple allocators (with a malloc/free-alike API)
on top of the resource system.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 include/linux/ioport.h |    1 +
 kernel/resource.c      |   21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index e9bb22c..63eb429 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -132,6 +132,7 @@ extern int allocate_resource(struct resource *root, struct resource *new,
 						       resource_size_t,
 						       resource_size_t),
 			     void *alignf_data);
+struct resource *lookup_resource(struct resource *root, resource_size_t start);
 int adjust_resource(struct resource *res, resource_size_t start,
 		    resource_size_t size);
 resource_size_t resource_alignment(struct resource *res);
diff --git a/kernel/resource.c b/kernel/resource.c
index 798e2fa..8d0b479 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -473,6 +473,27 @@ int allocate_resource(struct resource *root, struct resource *new,
 
 EXPORT_SYMBOL(allocate_resource);
 
+/**
+ * lookup_resource - find an existing resource by a resource start address
+ * @root: root resource descriptor
+ * @start: resource start address
+ *
+ * Returns a pointer to the resource if found, NULL otherwise
+ */
+struct resource *lookup_resource(struct resource *root, resource_size_t start)
+{
+	struct resource *res;
+
+	read_lock(&resource_lock);
+	for (res = root->child; res; res = res->sibling) {
+		if (res->start == start)
+			break;
+	}
+	read_unlock(&resource_lock);
+
+	return res;
+}
+
 /*
  * Insert a resource into the resource tree. If successful, return NULL,
  * otherwise return the conflicting resource (compare to __request_resource())
-- 
1.7.0.4

[PATCH 3/4] m68k/amiga: Chip RAM - Use lookup_resource()

[Geert Uytterhoeven]

Replace a custom implementation (which doesn't lock the resource tree) by a
call to lookup_resource()

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/m68k/amiga/chipram.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/m68k/amiga/chipram.c b/arch/m68k/amiga/chipram.c
index cfd3b7a..9d53417 100644
--- a/arch/m68k/amiga/chipram.c
+++ b/arch/m68k/amiga/chipram.c
@@ -93,21 +93,21 @@ void *amiga_chip_alloc_res(unsigned long size, struct resource *res)
 void amiga_chip_free(void *ptr)
 {
 	unsigned long start = ZTWO_PADDR(ptr);
-	struct resource **p, *res;
+	struct resource *res;
 	unsigned long size;
 
-	for (p = &chipram_res.child; (res = *p); p = &res->sibling) {
-		if (res->start != start)
-			continue;
-		*p = res->sibling;
-		size = resource_size(res);
-		pr_debug("amiga_chip_free: free %lu bytes at %p\n", size, ptr);
-		atomic_add(size, &chipavail);
-		kfree(res);
+	res = lookup_resource(&chipram_res, start);
+	if (!res) {
+		pr_err("amiga_chip_free: trying to free nonexistent region at "
+		       "%p\n", ptr);
 		return;
 	}
-	pr_err("amiga_chip_free: trying to free nonexistent region at %p\n",
-	       ptr);
+
+	size = resource_size(res);
+	pr_debug("amiga_chip_free: free %lu bytes at %p\n", size, ptr);
+	atomic_add(size, &chipavail);
+	release_resource(res);
+	kfree(res);
 }
 EXPORT_SYMBOL(amiga_chip_free);
 
-- 
1.7.0.4

[PATCH 4/4] sparc: iounmap() and *_free_coherent() - Use lookup_resource()

[Geert Uytterhoeven]

Replace a custom implementation (which doesn't lock the resource tree) by a
call to lookup_resource()

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/sparc/kernel/ioport.c |   32 +++++++-------------------------
 1 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
index 91e8c76..7be5438 100644
--- a/arch/sparc/kernel/ioport.c
+++ b/arch/sparc/kernel/ioport.c
@@ -60,9 +60,6 @@ static inline void mmu_inval_dma_area(void *va, unsigned long len)
 }
 #endif
 
-static struct resource *_sparc_find_resource(struct resource *r,
-					     unsigned long);
-
 static void __iomem *_sparc_ioremap(struct resource *res, u32 bus, u32 pa, int sz);
 static void __iomem *_sparc_alloc_io(unsigned int busno, unsigned long phys,
     unsigned long size, char *name);
@@ -138,7 +135,11 @@ void iounmap(volatile void __iomem *virtual)
 	unsigned long vaddr = (unsigned long) virtual & PAGE_MASK;
 	struct resource *res;
 
-	if ((res = _sparc_find_resource(&sparc_iomap, vaddr)) == NULL) {
+	/*
+	 * XXX Too slow. Can have 8192 DVMA pages on sun4m in the worst case.
+	 * This probably warrants some sort of hashing.
+	*/
+	if ((res = lookup_resource(&sparc_iomap, vaddr)) == NULL) {
 		printk("free_io/iounmap: cannot free %lx\n", vaddr);
 		return;
 	}
@@ -315,7 +316,7 @@ static void sbus_free_coherent(struct device *dev, size_t n, void *p,
 	struct resource *res;
 	struct page *pgv;
 
-	if ((res = _sparc_find_resource(&_sparc_dvma,
+	if ((res = lookup_resource(&_sparc_dvma,
 	    (unsigned long)p)) == NULL) {
 		printk("sbus_free_consistent: cannot free %p\n", p);
 		return;
@@ -491,7 +492,7 @@ static void pci32_free_coherent(struct device *dev, size_t n, void *p,
 	struct resource *res;
 	void *pgp;
 
-	if ((res = _sparc_find_resource(&_sparc_dvma,
+	if ((res = lookup_resource(&_sparc_dvma,
 	    (unsigned long)p)) == NULL) {
 		printk("pci_free_consistent: cannot free %p\n", p);
 		return;
@@ -725,25 +726,6 @@ static const struct file_operations sparc_io_proc_fops = {
 };
 #endif /* CONFIG_PROC_FS */
 
-/*
- * This is a version of find_resource and it belongs to kernel/resource.c.
- * Until we have agreement with Linus and Martin, it lingers here.
- *
- * XXX Too slow. Can have 8192 DVMA pages on sun4m in the worst case.
- * This probably warrants some sort of hashing.
- */
-static struct resource *_sparc_find_resource(struct resource *root,
-					     unsigned long start)
-{
-	struct resource *tmp;
-
-	for (tmp = root->child; tmp != 0; tmp = tmp->sibling) {
-		if (tmp->start == start)
-			return tmp;
-	}
-	return NULL;
-}
-
 static void register_proc_sparc_ioport(void)
 {
 #ifdef CONFIG_PROC_FS
-- 
1.7.0.4

Re: [PATCH 4/4] sparc: iounmap() and *_free_coherent() - Use lookup_resource()

[David Miller]

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Sat, 21 May 2011 21:39:16 +0200

> Replace a custom implementation (which doesn't lock the resource tree) by a
> call to lookup_resource()
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 1/4] sparc: _sparc_find_resource() should check for exact matches

[David Miller]

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Sat, 21 May 2011 21:39:13 +0200

> The address that's passed to _sparc_find_resource() should always be the
> start address of a resource:
>   - iounmap() passes a page-aligned virtual address, while the original
>     address was created by adding the in-page offset to the resource's
>     start address,
>   - sbus_free_coherent() and pci32_free_coherent() should be passed an
>     address obtained from sbus_alloc_coherent() resp. pci32_alloc_coherent(),
>     which is always a resource's start address.
> 
> Hence replace the range check by a check for an exact match.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 0/4] resources: Add lookup_resource()

[Geert Uytterhoeven]

On Sat, May 21, 2011 at 21:39, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> The resource system is a nice existing subsystem to implement simple allocators
> on top of.
> Unfortunately, there's no official method to find an existing resource by a
> resource start address, which is needed for a freeing function that just takes
> the start address of the region to free.
>
> Currently there are 3 users of the resource subsystem that implement this by
> traversing the resource tree theirselves:
>  - The Amiga Chip RAM allocator on m68k,
>  - iomap()/iounmap() on sparc,
>  - DMA allocation API on sparc.
> They all lack locking of the resource tree, as resource_lock is static in
> kernel/resource.c.
>
> Hence this patchset adds lookup_resource() and converts the above users to it.
>
>  [1/4] sparc: _sparc_find_resource() should check for exact matches
>  [2/4] resources: Add lookup_resource()
>  [3/4] m68k/amiga: Chip RAM - Use lookup_resource()
>  [4/4] sparc: iounmap() and *_free_coherent() - Use lookup_resource()

As I got two acks from David for the sparc bits and no objections, I
queued these
on my for-2.6.41 and for-next branches.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds