Patchwork [2/3] sparc/iommu: use BITS_TO_LONGS() to calculate bitmap size

login
register
mail settings
Submitter Akinobu Mita
Date March 24, 2013, 8:55 a.m.
Message ID <1364115306-17716-2-git-send-email-akinobu.mita@gmail.com>
Download mbox | patch
Permalink /patch/230404/
State Rejected
Delegated to: David Miller
Headers show

Comments

Akinobu Mita - March 24, 2013, 8:55 a.m.
The size of bitmaps which is accessed by bitops must be a multiple of
the sizeof(long).  So it would be better to use BITS_TO_LONGS() to
calculate the bitmap size although IOMMU_NPTES is a multiple of
BITS_PER_LONG.

This also fixes typo in the comment for IOMMU_NPTES.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
---
 arch/sparc/mm/iommu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
David Miller - March 25, 2013, 5:50 p.m.
From: Akinobu Mita <akinobu.mita@gmail.com>
Date: Sun, 24 Mar 2013 17:55:05 +0900

> The size of bitmaps which is accessed by bitops must be a multiple of
> the sizeof(long).  So it would be better to use BITS_TO_LONGS() to
> calculate the bitmap size although IOMMU_NPTES is a multiple of
> BITS_PER_LONG.
> 
> This also fixes typo in the comment for IOMMU_NPTES.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

Again, I'd rather not fix non-bugs.

You've also corrupted the indentation of the prom_printf() calls,
the arguments to a function should line up with the first column
after the openning parenthesis of the function call.

It was correct before your patch, it will be wrong afterwards.

But I don't want to apply this patch anyways, so don't bother
resubmitting the fixed version.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Julian Calaby - March 25, 2013, 10:11 p.m.
Hi David,

I understand that you're not going to fix a non-bug like this, but:

On Sun, Mar 24, 2013 at 7:55 PM, Akinobu Mita <akinobu.mita@gmail.com> wrote:
> The size of bitmaps which is accessed by bitops must be a multiple of
> the sizeof(long).  So it would be better to use BITS_TO_LONGS() to
> calculate the bitmap size although IOMMU_NPTES is a multiple of
> BITS_PER_LONG.
>
> This also fixes typo in the comment for IOMMU_NPTES.
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> ---
>  arch/sparc/mm/iommu.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/sparc/mm/iommu.c b/arch/sparc/mm/iommu.c
> index 0f4f719..f9027fd 100644
> --- a/arch/sparc/mm/iommu.c
> +++ b/arch/sparc/mm/iommu.c
> @@ -34,7 +34,7 @@
>  #define IOMMU_RNGE     IOMMU_RNGE_256MB
>  #define IOMMU_START    0xF0000000
>  #define IOMMU_WINSIZE  (256*1024*1024U)
> -#define IOMMU_NPTES    (IOMMU_WINSIZE/PAGE_SIZE)       /* 64K PTEs, 265KB */
> +#define IOMMU_NPTES    (IOMMU_WINSIZE/PAGE_SIZE)       /* 64K PTEs, 256KB */

Is this change worthwhile?

Thanks,
David Miller - March 25, 2013, 10:14 p.m.
From: Julian Calaby <julian.calaby@gmail.com>
Date: Tue, 26 Mar 2013 09:11:15 +1100

> On Sun, Mar 24, 2013 at 7:55 PM, Akinobu Mita <akinobu.mita@gmail.com> wrote:
>> @@ -34,7 +34,7 @@
>>  #define IOMMU_RNGE     IOMMU_RNGE_256MB
>>  #define IOMMU_START    0xF0000000
>>  #define IOMMU_WINSIZE  (256*1024*1024U)
>> -#define IOMMU_NPTES    (IOMMU_WINSIZE/PAGE_SIZE)       /* 64K PTEs, 265KB */
>> +#define IOMMU_NPTES    (IOMMU_WINSIZE/PAGE_SIZE)       /* 64K PTEs, 256KB */
> 
> Is this change worthwhile?

It's a comment typo fix, so sure why not.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sparc/mm/iommu.c b/arch/sparc/mm/iommu.c
index 0f4f719..f9027fd 100644
--- a/arch/sparc/mm/iommu.c
+++ b/arch/sparc/mm/iommu.c
@@ -34,7 +34,7 @@ 
 #define IOMMU_RNGE	IOMMU_RNGE_256MB
 #define IOMMU_START	0xF0000000
 #define IOMMU_WINSIZE	(256*1024*1024U)
-#define IOMMU_NPTES	(IOMMU_WINSIZE/PAGE_SIZE)	/* 64K PTEs, 265KB */
+#define IOMMU_NPTES	(IOMMU_WINSIZE/PAGE_SIZE)	/* 64K PTEs, 256KB */
 #define IOMMU_ORDER	6				/* 4096 * (1<<6) */
 
 /* srmmu.c */
@@ -103,10 +103,11 @@  static void __init sbus_iommu_init(struct platform_device *op)
 	iommu->regs->base = __pa((unsigned long) iommu->page_table) >> 4;
 	iommu_invalidate(iommu->regs);
 
-	bitmap = kmalloc(IOMMU_NPTES>>3, GFP_KERNEL);
+	bitmap = kmalloc(BITS_TO_LONGS(IOMMU_NPTES) * sizeof(unsigned long),
+			GFP_KERNEL);
 	if (!bitmap) {
-		prom_printf("Unable to allocate iommu bitmap [%d]\n",
-			    (int)(IOMMU_NPTES>>3));
+		prom_printf("Unable to allocate iommu bitmap [%ld]\n",
+			BITS_TO_LONGS(IOMMU_NPTES) * sizeof(unsigned long));
 		prom_halt();
 	}
 	bit_map_init(&iommu->usemap, bitmap, IOMMU_NPTES);