Re: [PATCH] dma_declare_coherent_memory wrong allocation

[Guennadi Liakhovetski <g.liakhovetski@gmx.de>]

On Sun, 22 Apr 2007, James Bottomley wrote:

> On Fri, 2007-04-13 at 20:08 +0200, Guennadi Liakhovetski wrote:
> > -	int bitmap_size = (pages + 31)/32;
> > +	int bitmap_size = DIV_ROUND_UP(pages, 8);
> 
> This isn't quite right.  Bitmaps are arrays of longs, not arrays of
> bytes.  The bug is forgetting that kmalloc() takes bytes ...
> 
> How about
> 
> int bitmap_size = DIV_ROUNDUP(pages, 32) * 4;

Right, thinko. How about using his:

+	int pages = DIV_ROUND_UP(size, PAGE_SIZE);
+	int bitmap_size = BITS_TO_LONGS(pages) * BYTES_PER_LONG;

to also allow for size not an integer number of pages as Andrew noticed? 
This could be done in 2 patches:

---
Introduce BYTES_PER_LONG and remove local definitions. fbsys 
compile-tested, amifb untested (framebuffer maintainer cc'ed)...

Signed-off-by: G. Liakhovetski <g.liakhovetski@gmx.de>

--- a/include/linux/types.h	2007-04-15 22:07:52.000000000 +0200
+++ b/include/linux/types.h	2007-04-22 22:51:17.000000000 +0200
@@ -9,6 +9,7 @@
 	unsigned long name[BITS_TO_LONGS(bits)]
 
 #define BITS_PER_BYTE 8
+#define BYTES_PER_LONG (BITS_PER_LONG / BITS_PER_BYTE)
 #endif
 
 #include <linux/posix_types.h>
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index 40c80c8..6bb68b8 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -19,6 +19,7 @@
 #include <linux/fb.h>
 #include <linux/console.h>
 #include <linux/module.h>
+#include <linux/types.h>
 
 #define FB_SYSFS_FLAG_ATTR 1
 
@@ -37,7 +38,6 @@
  */
 struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
 {
-#define BYTES_PER_LONG (BITS_PER_LONG/8)
 #define PADDING (BYTES_PER_LONG - (sizeof(struct fb_info) % BYTES_PER_LONG))
 	int fb_info_size = sizeof(struct fb_info);
 	struct fb_info *info;
diff --git a/drivers/video/amifb.c b/drivers/video/amifb.c
index 1a849b8..a53d5a2 100644
--- a/drivers/video/amifb.c
+++ b/drivers/video/amifb.c
@@ -51,6 +51,8 @@
 #include <linux/fb.h>
 #include <linux/init.h>
 #include <linux/ioport.h>
+#include <linux/types.h>
+#include <linux/log2.h>
 
 #include <asm/uaccess.h>
 #include <asm/system.h>
@@ -1350,15 +1352,7 @@ static int amifb_pan_display(struct fb_var_screeninfo *var,
 }
 
 
-#if BITS_PER_LONG == 32
-#define BYTES_PER_LONG	4
-#define SHIFT_PER_LONG	5
-#elif BITS_PER_LONG == 64
-#define BYTES_PER_LONG	8
-#define SHIFT_PER_LONG	6
-#else
-#define Please update me
-#endif
+#define SHIFT_PER_LONG	ilog2(BITS_PER_LONG)
 
 
     /*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/