[PATCH] powerpc:Fix rheap alignment problem

Honour alignment parameter in the rheap allocator.
Remove compile warning.

Signed-off-by: Pantelis Antoniou <redacted>
Signed-off-by: Li Yang <redacted>

---
 arch/powerpc/lib/Makefile |    1 +
 arch/powerpc/lib/rheap.c  |   24 ++++++++++++++++++++----
 include/asm-ppc/rheap.h   |    4 ++++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 34f5c2e..136a892 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile

@@ -11,6 +11,7 @@ obj-y			+= bitops.o
 obj-$(CONFIG_PPC64)	+= checksum_64.o copypage_64.o copyuser_64.o \
 			   memcpy_64.o usercopy_64.o mem_64.o string.o \
 			   strcase.o
+obj-$(CONFIG_QUICC_ENGINE) += rheap.o
 obj-$(CONFIG_PPC_ISERIES) += e2a.o
 obj-$(CONFIG_XMON)	+= sstep.o
 

diff --git a/arch/powerpc/lib/rheap.c b/arch/powerpc/lib/rheap.c
index 31e5118..57bf991 100644
--- a/arch/powerpc/lib/rheap.c
+++ b/arch/powerpc/lib/rheap.c

@@ -423,17 +423,21 @@ void *rh_detach_region(rh_info_t * info,
 	return (void *)s;
 }
 
-void *rh_alloc(rh_info_t * info, int size, const char *owner)
+void *rh_alloc_align(rh_info_t * info, int size, int alignment, const char *owner)
 {
 	struct list_head *l;
 	rh_block_t *blk;
 	rh_block_t *newblk;
 	void *start;
 
-	/* Validate size */
-	if (size <= 0)
+	/* Validate size, (must be power of two) */
+	if (size <= 0 || (alignment & (alignment - 1)) != 0)
 		return ERR_PTR(-EINVAL);
 
+	/* given alignment larger that default rheap alignment */
+	if (alignment > info->alignment)
+		size += alignment - 1;
+
 	/* Align to configured alignment */
 	size = (size + (info->alignment - 1)) & ~(info->alignment - 1);
 

@@ -476,15 +480,27 @@ void *rh_alloc(rh_info_t * info, int siz
 
 	attach_taken_block(info, newblk);
 
+	/* for larger alignment return fixed up pointer  */
+	/* this is no problem with the deallocator since */
+	/* we scan for pointers that lie in the blocks   */
+	if (alignment > info->alignment)
+		start = (void *)(((unsigned long)start + alignment - 1) &
+				~(alignment - 1));
+
 	return start;
 }
 
+void *rh_alloc(rh_info_t * info, int size, const char *owner)
+{
+	return rh_alloc_align(info, size, info->alignment, owner);
+}
+
 /* allocate at precisely the given address */
 void *rh_alloc_fixed(rh_info_t * info, void *start, int size, const char *owner)
 {
 	struct list_head *l;
 	rh_block_t *blk, *newblk1, *newblk2;
-	unsigned long s, e, m, bs, be;
+	unsigned long s, e, m, bs = 0, be = 0;
 
 	/* Validate size */
 	if (size <= 0)

diff --git a/include/asm-ppc/rheap.h b/include/asm-ppc/rheap.h
index e6ca1f6..65b9322 100644
--- a/include/asm-ppc/rheap.h
+++ b/include/asm-ppc/rheap.h

@@ -62,6 +62,10 @@ extern int rh_attach_region(rh_info_t * 
 /* Detach a free region */
 extern void *rh_detach_region(rh_info_t * info, void *start, int size);
 
+/* Allocate the given size from the remote heap (with alignment) */
+extern void *rh_alloc_align(rh_info_t * info, int size, int alignment,
+		const char *owner);
+
 /* Allocate the given size from the remote heap */
 extern void *rh_alloc(rh_info_t * info, int size, const char *owner);

--

Leo Li
Freescale Semiconductor

LeoLi@freescale.com 

On Fri, 2006-06-30 at 21:02 +0800, Li Yang-r58472 wrote:

Honour alignment parameter in the rheap allocator.
Remove compile warning.

What is this used for ? This rheap allocator ? I see no user in
arch/powerpc at least and only two users apparently in arch/ppc... are
we sure we need something that complex for these ? Can't we just use a
running bitmap allocator or an idr ?

Cheers,
Ben.

From: Benjamin Herrenschmidt

What is this used for ? This rheap allocator ?

It is used for allocating dual port RAM on 8xx and 82xx. (and probably others that have a CPM)
Most/all usage is still in ppc.

Yes, it was used for allocating dual port RAM for CPM.  And now we are
adding QE support to powerpc arch which need to use rheap(QE is next
generation for CPM).  Please see the patches I [off-list ref] just
posted for 8360epb support.  Moreover, previous CPM support is adding to
powerpc arch too.

--
Leo

Freescale Semiconductor


On 7/1/06, Rune Torgersen [off-list ref] wrote:

 From: Benjamin Herrenschmidt

quoted

What is this used for ? This rheap allocator ?

It is used for allocating dual port RAM on 8xx and 82xx. (and probably
others that have a CPM)
Most/all usage is still in ppc.


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

On Sat, 2006-07-01 at 14:35 +0800, Linux powerpc wrote:

Yes, it was used for allocating dual port RAM for CPM.  And now we are
adding QE support to powerpc arch which need to use rheap(QE is next
generation for CPM).  Please see the patches I [off-list ref]
just posted for 8360epb support.  Moreover, previous CPM support is
adding to powerpc arch too. 

Ok, well, I don't have anything specifically against that code, I was
just wondering if it may not duplicate something we already have (yet
another space allocator basically)... 

Ben.

On Sat, Jul 01, 2006 at 05:21:06PM +1000, Benjamin Herrenschmidt wrote:

On Sat, 2006-07-01 at 14:35 +0800, Linux powerpc wrote:

quoted

Yes, it was used for allocating dual port RAM for CPM.  And now we are
adding QE support to powerpc arch which need to use rheap(QE is next
generation for CPM).  Please see the patches I [off-list ref]
just posted for 8360epb support.  Moreover, previous CPM support is
adding to powerpc arch too. 

Ok, well, I don't have anything specifically against that code, I was
just wondering if it may not duplicate something we already have (yet
another space allocator basically)... 

Yepp.  Without looking at the rheap allocator in deatail, any reason
it can't use lib/genalloc.c?

On Jul 1, 2006, at 5:25 AM, Christoph Hellwig wrote:

On Sat, Jul 01, 2006 at 05:21:06PM +1000, Benjamin Herrenschmidt  
wrote:

quoted

On Sat, 2006-07-01 at 14:35 +0800, Linux powerpc wrote:

quoted

Yes, it was used for allocating dual port RAM for CPM.  And now  
we are
adding QE support to powerpc arch which need to use rheap(QE is next
generation for CPM).  Please see the patches I [off-list ref]
just posted for 8360epb support.  Moreover, previous CPM support is
adding to powerpc arch too.

Ok, well, I don't have anything specifically against that code, I was
just wondering if it may not duplicate something we already have (yet
another space allocator basically)...

Yepp.  Without looking at the rheap allocator in deatail, any reason
it can't use lib/genalloc.c?

Doing a quick glance at lib/genalloc.c I dont see any reason we  
couldn't use it.  However, Panto will know best, since he wrote rheap.

- k

On Saturday 01 July 2006 17:34, Kumar Gala wrote:

On Jul 1, 2006, at 5:25 AM, Christoph Hellwig wrote:

quoted

On Sat, Jul 01, 2006 at 05:21:06PM +1000, Benjamin Herrenschmidt  
wrote:

quoted

On Sat, 2006-07-01 at 14:35 +0800, Linux powerpc wrote:

quoted

Yes, it was used for allocating dual port RAM for CPM.  And now  
we are
adding QE support to powerpc arch which need to use rheap(QE is next
generation for CPM).  Please see the patches I [off-list ref]
just posted for 8360epb support.  Moreover, previous CPM support is
adding to powerpc arch too.

Ok, well, I don't have anything specifically against that code, I was
just wondering if it may not duplicate something we already have (yet
another space allocator basically)...

Yepp.  Without looking at the rheap allocator in deatail, any reason
it can't use lib/genalloc.c?

Doing a quick glance at lib/genalloc.c I dont see any reason we  
couldn't use it.  However, Panto will know best, since he wrote rheap.

- k

Hi there,

RHEAP started life long before on 2.4 before genalloc was included in the kernel.
The difference is only in the implementation, rheap uses double linked lists
while genalloc uses per pool bitmaps. RHEAP is faster & conserves a bit more space 
since it doesn't use a bitmap to track the chunks.

Since genalloc is the blessed linux thing it might be best to use that & remove
rheap completely. Oh well...

Regards

Pantelis

From: Pantelis Antoniou
Sent: Sat 7/1/2006 9:50 AM

Since genalloc is the blessed linux thing it might be best to use that & remove
rheap completely. Oh well...

Two problems with genalloc that I can see (for CPM programming):
1) (minor) Does not have a way to specify alignment (genalloc does it for you)
2) (major problerm, at least for me) Does not have a way to allocate a specified address in the pool.

2 is needed esp when programming MCC drivers, since a lot of the datastructures must be in DP RAM _and_ be in a specific spot. And if you cannot tell the allocator that I am using a specific address, then the allocator might very well give somebody else that portion of RAM. The only solution without a fixed allocator is to allocate ALL memory in the DP RAM and use your own allocator. 

On Sunday 02 July 2006 06:54, Rune Torgersen wrote:

From: Pantelis Antoniou
Sent: Sat 7/1/2006 9:50 AM

quoted

Since genalloc is the blessed linux thing it might be best to use that & remove
rheap completely. Oh well...

Two problems with genalloc that I can see (for CPM programming):
1) (minor) Does not have a way to specify alignment (genalloc does it for you)
2) (major problerm, at least for me) Does not have a way to allocate a specified address in the pool.

2 is needed esp when programming MCC drivers, since a lot of the datastructures must be in DP RAM _and_ be in a specific spot. And if you cannot tell the allocator that I am using a specific address, then the allocator might very well give somebody else that portion of RAM. The only solution without a fixed allocator is to allocate ALL memory in the DP RAM and use your own allocator. 

Yeah, that too.

Too bad there are no main tree drivers like that, but they do exist.

One could conceivably hack genalloc to do that, but will end up with
something complex too.

BTW, there are other uEngine based architectures with similar alignment
requirements.

So in conclusion, for the in-tree drivers genalloc is sufficient as an cpm memory allocator.
For some out of tree drivers, it is not.

Pantelis

On Sun, 2006-07-02 at 08:18 +0300, Pantelis Antoniou wrote:

On Sunday 02 July 2006 06:54, Rune Torgersen wrote:

quoted

From: Pantelis Antoniou
Sent: Sat 7/1/2006 9:50 AM

quoted

Since genalloc is the blessed linux thing it might be best to use that & remove
rheap completely. Oh well...

Two problems with genalloc that I can see (for CPM programming):
1) (minor) Does not have a way to specify alignment (genalloc does it for you)
2) (major problerm, at least for me) Does not have a way to allocate a specified address in the pool.

2 is needed esp when programming MCC drivers, since a lot of the datastructures must be in DP RAM _and_ be in a specific spot. And if you cannot tell the allocator that I am using a specific address, then the allocator might very well give somebody else that portion of RAM. The only solution without a fixed allocator is to allocate ALL memory in the DP RAM and use your own allocator. 

Yeah, that too.

Too bad there are no main tree drivers like that, but they do exist.

One could conceivably hack genalloc to do that, but will end up with
something complex too.

BTW, there are other uEngine based architectures with similar alignment
requirements.

So in conclusion, for the in-tree drivers genalloc is sufficient as an cpm memory allocator.
For some out of tree drivers, it is not.

Sounds like a good enough justification to keep rheap for now then.

Ben.