* [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
[not found] <1255076961-21325-1-git-send-email-akinobu.mita@gmail.com>
@ 2009-10-09 8:29 ` Akinobu Mita
2009-10-09 8:29 ` [PATCH 3/8] iommu-helper: Use bitmap library Akinobu Mita
2009-10-09 23:41 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Andrew Morton
0 siblings, 2 replies; 13+ messages in thread
From: Akinobu Mita @ 2009-10-09 8:29 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: Fenghua Yu, Greg Kroah-Hartman, linux-ia64, Tony Luck, x86,
netdev, Akinobu Mita, linux-altix, Yevgeny Petrilin,
FUJITA Tomonori, linuxppc-dev, Ingo Molnar, Paul Mackerras,
H. Peter Anvin, sparclinux, Thomas Gleixner, linux-usb,
David S. Miller, Lothar Wassmann
This introduces new bitmap functions:
bitmap_set: Set specified bit area
bitmap_clear: Clear specified bit area
bitmap_find_next_zero_area: Find free bit area
These are stolen from iommu helper.
I changed the return value of bitmap_find_next_zero_area if there is
no zero area.
find_next_zero_area in iommu helper: returns -1
bitmap_find_next_zero_area: return >= bitmap size
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@ozlabs.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Lothar Wassmann <LW@KARO-electronics.de>
Cc: linux-usb@vger.kernel.org
Cc: Roland Dreier <rolandd@cisco.com>
Cc: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Cc: netdev@vger.kernel.org
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
Cc: linux-altix@sgi.com
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
include/linux/bitmap.h | 11 +++++++++++
lib/bitmap.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 756d78b..daf8c48 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -42,6 +42,9 @@
* bitmap_empty(src, nbits) Are all bits zero in *src?
* bitmap_full(src, nbits) Are all bits set in *src?
* bitmap_weight(src, nbits) Hamming Weight: number set bits
+ * bitmap_set(dst, pos, nbits) Set specified bit area
+ * bitmap_clear(dst, pos, nbits) Clear specified bit area
+ * bitmap_find_next_zero_area(buf, len, pos, n, mask) Find bit free area
* bitmap_shift_right(dst, src, n, nbits) *dst = *src >> n
* bitmap_shift_left(dst, src, n, nbits) *dst = *src << n
* bitmap_remap(dst, src, old, new, nbits) *dst = map(old, new)(src)
@@ -108,6 +111,14 @@ extern int __bitmap_subset(const unsigned long *bitmap1,
const unsigned long *bitmap2, int bits);
extern int __bitmap_weight(const unsigned long *bitmap, int bits);
+extern void bitmap_set(unsigned long *map, int i, int len);
+extern void bitmap_clear(unsigned long *map, int start, int nr);
+extern unsigned long bitmap_find_next_zero_area(unsigned long *map,
+ unsigned long size,
+ unsigned long start,
+ unsigned int nr,
+ unsigned long align_mask);
+
extern int bitmap_scnprintf(char *buf, unsigned int len,
const unsigned long *src, int nbits);
extern int __bitmap_parse(const char *buf, unsigned int buflen, int is_user,
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 7025658..95070fa 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -271,6 +271,53 @@ int __bitmap_weight(const unsigned long *bitmap, int bits)
}
EXPORT_SYMBOL(__bitmap_weight);
+void bitmap_set(unsigned long *map, int i, int len)
+{
+ int end = i + len;
+
+ while (i < end) {
+ __set_bit(i, map);
+ i++;
+ }
+}
+EXPORT_SYMBOL(bitmap_set);
+
+void bitmap_clear(unsigned long *map, int start, int nr)
+{
+ int end = start + nr;
+
+ while (start < end) {
+ __clear_bit(start, map);
+ start++;
+ }
+}
+EXPORT_SYMBOL(bitmap_clear);
+
+unsigned long bitmap_find_next_zero_area(unsigned long *map,
+ unsigned long size,
+ unsigned long start,
+ unsigned int nr,
+ unsigned long align_mask)
+{
+ unsigned long index, end, i;
+again:
+ index = find_next_zero_bit(map, size, start);
+
+ /* Align allocation */
+ index = (index + align_mask) & ~align_mask;
+
+ end = index + nr;
+ if (end >= size)
+ return end;
+ i = find_next_bit(map, end, index);
+ if (i < end) {
+ start = i + 1;
+ goto again;
+ }
+ return index;
+}
+EXPORT_SYMBOL(bitmap_find_next_zero_area);
+
/*
* Bitmap printing & parsing functions: first version by Bill Irwin,
* second version by Paul Jackson, third by Joe Korty.
--
1.5.4.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/8] iommu-helper: Use bitmap library
2009-10-09 8:29 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Akinobu Mita
@ 2009-10-09 8:29 ` Akinobu Mita
2009-10-09 23:41 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Andrew Morton
1 sibling, 0 replies; 13+ messages in thread
From: Akinobu Mita @ 2009-10-09 8:29 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: x86, Akinobu Mita, FUJITA Tomonori, linuxppc-dev, Ingo Molnar,
Paul Mackerras, H. Peter Anvin, sparclinux, Thomas Gleixner,
David S. Miller
Use bitmap library and kill some unused iommu helper functions.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@ozlabs.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
arch/powerpc/kernel/iommu.c | 4 +-
arch/sparc/kernel/iommu.c | 3 +-
arch/x86/kernel/amd_iommu.c | 4 +-
arch/x86/kernel/pci-calgary_64.c | 6 ++--
arch/x86/kernel/pci-gart_64.c | 6 ++--
include/linux/iommu-helper.h | 3 --
lib/iommu-helper.c | 55 ++++---------------------------------
7 files changed, 18 insertions(+), 63 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index fd51578..5547ae6 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -30,7 +30,7 @@
#include <linux/spinlock.h>
#include <linux/string.h>
#include <linux/dma-mapping.h>
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
#include <linux/iommu-helper.h>
#include <linux/crash_dump.h>
#include <asm/io.h>
@@ -251,7 +251,7 @@ static void __iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
}
ppc_md.tce_free(tbl, entry, npages);
- iommu_area_free(tbl->it_map, free_entry, npages);
+ bitmap_clear(tbl->it_map, free_entry, npages);
}
static void iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index 7690cc2..5fad949 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -11,6 +11,7 @@
#include <linux/dma-mapping.h>
#include <linux/errno.h>
#include <linux/iommu-helper.h>
+#include <linux/bitmap.h>
#ifdef CONFIG_PCI
#include <linux/pci.h>
@@ -169,7 +170,7 @@ void iommu_range_free(struct iommu *iommu, dma_addr_t dma_addr, unsigned long np
entry = (dma_addr - iommu->page_table_map_base) >> IO_PAGE_SHIFT;
- iommu_area_free(arena->map, entry, npages);
+ bitmap_clear(arena->map, entry, npages);
}
int iommu_table_init(struct iommu *iommu, int tsbsize,
diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 98f230f..08b1d20 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -19,7 +19,7 @@
#include <linux/pci.h>
#include <linux/gfp.h>
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
#include <linux/debugfs.h>
#include <linux/scatterlist.h>
#include <linux/dma-mapping.h>
@@ -959,7 +959,7 @@ static void dma_ops_free_addresses(struct dma_ops_domain *dom,
address = (address % APERTURE_RANGE_SIZE) >> PAGE_SHIFT;
- iommu_area_free(range->bitmap, address, pages);
+ bitmap_clear(range->bitmap, address, pages);
}
diff --git a/arch/x86/kernel/pci-calgary_64.c b/arch/x86/kernel/pci-calgary_64.c
index 971a3be..c87bb20 100644
--- a/arch/x86/kernel/pci-calgary_64.c
+++ b/arch/x86/kernel/pci-calgary_64.c
@@ -31,7 +31,7 @@
#include <linux/string.h>
#include <linux/crash_dump.h>
#include <linux/dma-mapping.h>
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
#include <linux/pci_ids.h>
#include <linux/pci.h>
#include <linux/delay.h>
@@ -211,7 +211,7 @@ static void iommu_range_reserve(struct iommu_table *tbl,
spin_lock_irqsave(&tbl->it_lock, flags);
- iommu_area_reserve(tbl->it_map, index, npages);
+ bitmap_set(tbl->it_map, index, npages);
spin_unlock_irqrestore(&tbl->it_lock, flags);
}
@@ -305,7 +305,7 @@ static void iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
spin_lock_irqsave(&tbl->it_lock, flags);
- iommu_area_free(tbl->it_map, entry, npages);
+ bitmap_clear(tbl->it_map, entry, npages);
spin_unlock_irqrestore(&tbl->it_lock, flags);
}
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 98a827e..3010cd1 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -22,7 +22,7 @@
#include <linux/module.h>
#include <linux/topology.h>
#include <linux/interrupt.h>
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
#include <linux/kdebug.h>
#include <linux/scatterlist.h>
#include <linux/iommu-helper.h>
@@ -122,7 +122,7 @@ static void free_iommu(unsigned long offset, int size)
unsigned long flags;
spin_lock_irqsave(&iommu_bitmap_lock, flags);
- iommu_area_free(iommu_gart_bitmap, offset, size);
+ bitmap_clear(iommu_gart_bitmap, offset, size);
if (offset >= next_bit)
next_bit = offset + size;
spin_unlock_irqrestore(&iommu_bitmap_lock, flags);
@@ -781,7 +781,7 @@ void __init gart_iommu_init(void)
* Out of IOMMU space handling.
* Reserve some invalid pages at the beginning of the GART.
*/
- iommu_area_reserve(iommu_gart_bitmap, 0, EMERGENCY_PAGES);
+ bitmap_set(iommu_gart_bitmap, 0, EMERGENCY_PAGES);
agp_memory_reserved = iommu_size;
printk(KERN_INFO
diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h
index 3b068e5..64d1b63 100644
--- a/include/linux/iommu-helper.h
+++ b/include/linux/iommu-helper.h
@@ -14,14 +14,11 @@ static inline unsigned long iommu_device_max_index(unsigned long size,
extern int iommu_is_span_boundary(unsigned int index, unsigned int nr,
unsigned long shift,
unsigned long boundary_size);
-extern void iommu_area_reserve(unsigned long *map, unsigned long i, int len);
extern unsigned long iommu_area_alloc(unsigned long *map, unsigned long size,
unsigned long start, unsigned int nr,
unsigned long shift,
unsigned long boundary_size,
unsigned long align_mask);
-extern void iommu_area_free(unsigned long *map, unsigned long start,
- unsigned int nr);
extern unsigned long iommu_num_pages(unsigned long addr, unsigned long len,
unsigned long io_page_size);
diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
index dddbf22..51b53d3 100644
--- a/lib/iommu-helper.c
+++ b/lib/iommu-helper.c
@@ -3,40 +3,7 @@
*/
#include <linux/module.h>
-#include <linux/bitops.h>
-
-static unsigned long find_next_zero_area(unsigned long *map,
- unsigned long size,
- unsigned long start,
- unsigned int nr,
- unsigned long align_mask)
-{
- unsigned long index, end, i;
-again:
- index = find_next_zero_bit(map, size, start);
-
- /* Align allocation */
- index = (index + align_mask) & ~align_mask;
-
- end = index + nr;
- if (end >= size)
- return -1;
- i = find_next_bit(map, end, index);
- if (i < end) {
- start = i + 1;
- goto again;
- }
- return index;
-}
-
-void iommu_area_reserve(unsigned long *map, unsigned long i, int len)
-{
- unsigned long end = i + len;
- while (i < end) {
- __set_bit(i, map);
- i++;
- }
-}
+#include <linux/bitmap.h>
int iommu_is_span_boundary(unsigned int index, unsigned int nr,
unsigned long shift,
@@ -55,30 +22,20 @@ unsigned long iommu_area_alloc(unsigned long *map, unsigned long size,
{
unsigned long index;
again:
- index = find_next_zero_area(map, size, start, nr, align_mask);
- if (index != -1) {
+ index = bitmap_find_next_zero_area(map, size, start, nr, align_mask);
+ if (index < size) {
if (iommu_is_span_boundary(index, nr, shift, boundary_size)) {
/* we could do more effectively */
start = index + 1;
goto again;
}
- iommu_area_reserve(map, index, nr);
+ bitmap_set(map, index, nr);
+ return index;
}
- return index;
+ return -1;
}
EXPORT_SYMBOL(iommu_area_alloc);
-void iommu_area_free(unsigned long *map, unsigned long start, unsigned int nr)
-{
- unsigned long end = start + nr;
-
- while (start < end) {
- __clear_bit(start, map);
- start++;
- }
-}
-EXPORT_SYMBOL(iommu_area_free);
-
unsigned long iommu_num_pages(unsigned long addr, unsigned long len,
unsigned long io_page_size)
{
--
1.5.4.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-09 8:29 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Akinobu Mita
2009-10-09 8:29 ` [PATCH 3/8] iommu-helper: Use bitmap library Akinobu Mita
@ 2009-10-09 23:41 ` Andrew Morton
2009-10-13 2:18 ` Akinobu Mita
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2009-10-09 23:41 UTC (permalink / raw)
To: Akinobu Mita
Cc: Fenghua Yu, Greg Kroah-Hartman, linux-ia64, Tony Luck, x86,
netdev, linux-kernel, linux-altix, Yevgeny Petrilin,
FUJITA Tomonori, linuxppc-dev, Ingo Molnar, Paul Mackerras,
H. Peter Anvin, sparclinux, Thomas Gleixner, linux-usb,
David S. Miller, Lothar Wassmann
On Fri, 9 Oct 2009 17:29:15 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:
> This introduces new bitmap functions:
>
> bitmap_set: Set specified bit area
> bitmap_clear: Clear specified bit area
> bitmap_find_next_zero_area: Find free bit area
>
> These are stolen from iommu helper.
>
> I changed the return value of bitmap_find_next_zero_area if there is
> no zero area.
>
> find_next_zero_area in iommu helper: returns -1
> bitmap_find_next_zero_area: return >= bitmap size
I'll plan to merge this patch into 2.6.32 so we can trickle all the
other patches into subsystems in an orderly fashion.
> +void bitmap_set(unsigned long *map, int i, int len)
> +{
> + int end = i + len;
> +
> + while (i < end) {
> + __set_bit(i, map);
> + i++;
> + }
> +}
This is really inefficient, isn't it? It's a pretty trivial matter to
romp through memory 32 or 64 bits at a time.
> +EXPORT_SYMBOL(bitmap_set);
> +
> +void bitmap_clear(unsigned long *map, int start, int nr)
> +{
> + int end = start + nr;
> +
> + while (start < end) {
> + __clear_bit(start, map);
> + start++;
> + }
> +}
> +EXPORT_SYMBOL(bitmap_clear);
Ditto.
> +unsigned long bitmap_find_next_zero_area(unsigned long *map,
> + unsigned long size,
> + unsigned long start,
> + unsigned int nr,
> + unsigned long align_mask)
> +{
> + unsigned long index, end, i;
> +again:
> + index = find_next_zero_bit(map, size, start);
> +
> + /* Align allocation */
> + index = (index + align_mask) & ~align_mask;
> +
> + end = index + nr;
> + if (end >= size)
> + return end;
> + i = find_next_bit(map, end, index);
> + if (i < end) {
> + start = i + 1;
> + goto again;
> + }
> + return index;
> +}
> +EXPORT_SYMBOL(bitmap_find_next_zero_area);
This needs documentation, please. It appears that `size' is the size
of the bitmap and `nr' is the number of zeroed bits we're looking for,
but an inattentive programmer could get those reversed.
Also the semantics of `align_mask' could benefit from spelling out. Is
the alignment with respect to memory boundaries or with respect to
`map' or with respect to map+start or what?
And why does align_mask exist at all? I was a bit surprised to see it
there. In which scenarios will it be non-zero?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-09 23:41 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Andrew Morton
@ 2009-10-13 2:18 ` Akinobu Mita
2009-10-13 9:10 ` Akinobu Mita
0 siblings, 1 reply; 13+ messages in thread
From: Akinobu Mita @ 2009-10-13 2:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Fenghua Yu, Greg Kroah-Hartman, linux-ia64, Tony Luck, x86,
netdev, linux-kernel, linux-altix, Yevgeny Petrilin,
FUJITA Tomonori, linuxppc-dev, Ingo Molnar, Paul Mackerras,
H. Peter Anvin, sparclinux, Thomas Gleixner, linux-usb,
David S. Miller, Lothar Wassmann
On Fri, Oct 09, 2009 at 04:41:00PM -0700, Andrew Morton wrote:
> On Fri, 9 Oct 2009 17:29:15 +0900
> Akinobu Mita <akinobu.mita@gmail.com> wrote:
>
> > This introduces new bitmap functions:
> >
> > bitmap_set: Set specified bit area
> > bitmap_clear: Clear specified bit area
> > bitmap_find_next_zero_area: Find free bit area
> >
> > These are stolen from iommu helper.
> >
> > I changed the return value of bitmap_find_next_zero_area if there is
> > no zero area.
> >
> > find_next_zero_area in iommu helper: returns -1
> > bitmap_find_next_zero_area: return >= bitmap size
>
> I'll plan to merge this patch into 2.6.32 so we can trickle all the
> other patches into subsystems in an orderly fashion.
Sounds good.
> > +void bitmap_set(unsigned long *map, int i, int len)
> > +{
> > + int end = i + len;
> > +
> > + while (i < end) {
> > + __set_bit(i, map);
> > + i++;
> > + }
> > +}
>
> This is really inefficient, isn't it? It's a pretty trivial matter to
> romp through memory 32 or 64 bits at a time.
OK. I'll do
> > +unsigned long bitmap_find_next_zero_area(unsigned long *map,
> > + unsigned long size,
> > + unsigned long start,
> > + unsigned int nr,
> > + unsigned long align_mask)
> > +{
> > + unsigned long index, end, i;
> > +again:
> > + index = find_next_zero_bit(map, size, start);
> > +
> > + /* Align allocation */
> > + index = (index + align_mask) & ~align_mask;
> > +
> > + end = index + nr;
> > + if (end >= size)
> > + return end;
> > + i = find_next_bit(map, end, index);
> > + if (i < end) {
> > + start = i + 1;
> > + goto again;
> > + }
> > + return index;
> > +}
> > +EXPORT_SYMBOL(bitmap_find_next_zero_area);
>
> This needs documentation, please. It appears that `size' is the size
> of the bitmap and `nr' is the number of zeroed bits we're looking for,
> but an inattentive programmer could get those reversed.
>
> Also the semantics of `align_mask' could benefit from spelling out. Is
> the alignment with respect to memory boundaries or with respect to
> `map' or with respect to map+start or what?
OK. I will document it.
And I plan to change bitmap_find_next_zero_area() to take the alignment
instead of an align_mask as Roland said.
> And why does align_mask exist at all? I was a bit surprised to see it
> there. In which scenarios will it be non-zero?
Because the users of iommu-helper and mlx4 need the alignment requirement
for the zero area.
arch/powerpc/kernel/iommu.c
arch/x86/kernel/amd_iommu.c
arch/x86/kernel/pci-gart_64.c
drivers/net/mlx4/alloc.c
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-13 2:18 ` Akinobu Mita
@ 2009-10-13 9:10 ` Akinobu Mita
2009-10-13 21:54 ` Michael Ellerman
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Akinobu Mita @ 2009-10-13 9:10 UTC (permalink / raw)
To: Andrew Morton
Cc: Fenghua Yu, Greg Kroah-Hartman, linux-ia64, Tony Luck, x86,
netdev, linux-kernel, linux-altix, Yevgeny Petrilin,
FUJITA Tomonori, linuxppc-dev, Ingo Molnar, Paul Mackerras,
H. Peter Anvin, sparclinux, Thomas Gleixner, linux-usb,
David S. Miller, Lothar Wassmann
My user space testing exposed off-by-one error find_next_zero_area
in iommu-helper. Some zero area cannot be found by this bug.
Subject: [PATCH] Fix off-by-one error in find_next_zero_area
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
lib/iommu-helper.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
index 75dbda0..afc58bc 100644
--- a/lib/iommu-helper.c
+++ b/lib/iommu-helper.c
@@ -19,7 +19,7 @@ again:
index = (index + align_mask) & ~align_mask;
end = index + nr;
- if (end >= size)
+ if (end > size)
return -1;
for (i = index; i < end; i++) {
if (test_bit(i, map)) {
--
1.5.4.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-13 9:10 ` Akinobu Mita
@ 2009-10-13 21:54 ` Michael Ellerman
2009-10-14 3:39 ` Akinobu Mita
2009-10-14 3:22 ` [PATCH -mmotm] Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area. patch Akinobu Mita
2009-10-17 13:43 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area FUJITA Tomonori
2 siblings, 1 reply; 13+ messages in thread
From: Michael Ellerman @ 2009-10-13 21:54 UTC (permalink / raw)
To: Akinobu Mita
Cc: Fenghua Yu, x86, linux-ia64, Thomas Gleixner, David S. Miller,
netdev, Greg Kroah-Hartman, linux-kernel, linux-altix,
Yevgeny Petrilin, FUJITA Tomonori, linuxppc-dev, Tony Luck,
Paul Mackerras, H. Peter Anvin, sparclinux, Andrew Morton,
linux-usb, Ingo Molnar, Lothar Wassmann
[-- Attachment #1: Type: text/plain, Size: 241 bytes --]
On Tue, 2009-10-13 at 18:10 +0900, Akinobu Mita wrote:
> My user space testing exposed off-by-one error find_next_zero_area
> in iommu-helper.
Why not merge those tests into the kernel as a configurable boot-time
self-test?
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH -mmotm] Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area. patch
2009-10-13 9:10 ` Akinobu Mita
2009-10-13 21:54 ` Michael Ellerman
@ 2009-10-14 3:22 ` Akinobu Mita
2009-10-15 6:07 ` [PATCH -mmotm -v2] " Akinobu Mita
2009-10-17 13:43 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area FUJITA Tomonori
2 siblings, 1 reply; 13+ messages in thread
From: Akinobu Mita @ 2009-10-14 3:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Fenghua Yu, Greg Kroah-Hartman, linux-ia64, Tony Luck, x86,
netdev, linux-kernel, linux-altix, Yevgeny Petrilin,
FUJITA Tomonori, linuxppc-dev, Ingo Molnar, Paul Mackerras,
H. Peter Anvin, sparclinux, Thomas Gleixner, linux-usb,
David S. Miller, Lothar Wassmann
Update PATCH 2/8 based on review comments by Andrew and bugfix
exposed by user space testing.
I didn't change argument of align_mask at this time because it
turned out that it needs more changes in iommu-helper users.
From: Akinobu Mita <akinobu.mita@gmail.com>
Subject: Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area.patch
- Rewrite bitmap_set and bitmap_clear
Instead of setting or clearing for each bit.
- Fix off-by-one error in bitmap_find_next_zero_area
This bug was derived from find_next_zero_area in iommu-helper.
- Add kerneldoc for bitmap_find_next_zero_area
This patch is supposed to be folded into
bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area.patch
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
lib/bitmap.c | 60 +++++++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 2415da4..84292c9 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -271,28 +271,62 @@ int __bitmap_weight(const unsigned long *bitmap, int bits)
}
EXPORT_SYMBOL(__bitmap_weight);
-void bitmap_set(unsigned long *map, int i, int len)
-{
- int end = i + len;
+#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
- while (i < end) {
- __set_bit(i, map);
- i++;
+void bitmap_set(unsigned long *map, int start, int nr)
+{
+ unsigned long *p = map + BIT_WORD(start);
+ const int size = start + nr;
+ int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
+ unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
+
+ while (nr - bits_to_set >= 0) {
+ *p |= mask_to_set;
+ nr -= bits_to_set;
+ bits_to_set = BITS_PER_LONG;
+ mask_to_set = ~0UL;
+ p++;
+ }
+ if (nr) {
+ mask_to_set &= BITMAP_LAST_WORD_MASK(size);
+ *p |= mask_to_set;
}
}
EXPORT_SYMBOL(bitmap_set);
void bitmap_clear(unsigned long *map, int start, int nr)
{
- int end = start + nr;
-
- while (start < end) {
- __clear_bit(start, map);
- start++;
+ unsigned long *p = map + BIT_WORD(start);
+ const int size = start + nr;
+ int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
+ unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
+
+ while (nr - bits_to_clear >= 0) {
+ *p &= ~mask_to_clear;
+ nr -= bits_to_clear;
+ bits_to_clear = BITS_PER_LONG;
+ mask_to_clear = ~0UL;
+ p++;
+ }
+ if (nr) {
+ mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
+ *p &= ~mask_to_clear;
}
}
EXPORT_SYMBOL(bitmap_clear);
+/*
+ * bitmap_find_next_zero_area - find a contiguous aligned zero area
+ * @map: The address to base the search on
+ * @size: The bitmap size in bits
+ * @start: The bitnumber to start searching at
+ * @nr: The number of zeroed bits we're looking for
+ * @align_mask: Alignment mask for zero area
+ *
+ * The @align_mask should be one less than a power of 2; the effect is that
+ * the bit offset of all zero areas this function finds is multiples of that
+ * power of 2. A @align_mask of 0 means no alignment is required.
+ */
unsigned long bitmap_find_next_zero_area(unsigned long *map,
unsigned long size,
unsigned long start,
@@ -304,10 +338,10 @@ again:
index = find_next_zero_bit(map, size, start);
/* Align allocation */
- index = (index + align_mask) & ~align_mask;
+ index = __ALIGN_MASK(index, align_mask);
end = index + nr;
- if (end >= size)
+ if (end > size)
return end;
i = find_next_bit(map, end, index);
if (i < end) {
--
1.5.4.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-13 21:54 ` Michael Ellerman
@ 2009-10-14 3:39 ` Akinobu Mita
0 siblings, 0 replies; 13+ messages in thread
From: Akinobu Mita @ 2009-10-14 3:39 UTC (permalink / raw)
To: Michael Ellerman
Cc: Fenghua Yu, x86, linux-ia64, Thomas Gleixner, David S. Miller,
netdev, Greg Kroah-Hartman, linux-kernel, linux-altix,
Yevgeny Petrilin, FUJITA Tomonori, linuxppc-dev, Tony Luck,
Paul Mackerras, H. Peter Anvin, sparclinux, Andrew Morton,
linux-usb, Ingo Molnar, Lothar Wassmann
On Wed, Oct 14, 2009 at 08:54:47AM +1100, Michael Ellerman wrote:
> On Tue, 2009-10-13 at 18:10 +0900, Akinobu Mita wrote:
> > My user space testing exposed off-by-one error find_next_zero_area
> > in iommu-helper.
>
> Why not merge those tests into the kernel as a configurable boot-time
> self-test?
I send the test program that I used. Obviously it needs
better diagnostic messages and cleanup to be added into kernel tests.
#include <stdio.h>
#include <time.h>
#include <stdlib.h>
#include <string.h>
#if 1 /* Copy and paste from kernel source */
#define BITS_PER_BYTE 8
#define BITS_PER_LONG (sizeof(long) * BITS_PER_BYTE)
#define BIT_WORD(nr) ((nr) / BITS_PER_LONG)
#define BITOP_WORD(nr) ((nr) / BITS_PER_LONG)
#define BITMAP_LAST_WORD_MASK(nbits) \
( \
((nbits) % BITS_PER_LONG) ? \
(1UL<<((nbits) % BITS_PER_LONG))-1 : ~0UL \
)
#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
void bitmap_set(unsigned long *map, int start, int nr)
{
unsigned long *p = map + BIT_WORD(start);
const int size = start + nr;
int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
while (nr - bits_to_set >= 0) {
*p |= mask_to_set;
nr -= bits_to_set;
bits_to_set = BITS_PER_LONG;
mask_to_set = ~0UL;
p++;
}
if (nr) {
mask_to_set &= BITMAP_LAST_WORD_MASK(size);
*p |= mask_to_set;
}
}
void bitmap_clear(unsigned long *map, int start, int nr)
{
unsigned long *p = map + BIT_WORD(start);
const int size = start + nr;
int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
while (nr - bits_to_clear >= 0) {
*p &= ~mask_to_clear;
nr -= bits_to_clear;
bits_to_clear = BITS_PER_LONG;
mask_to_clear = ~0UL;
p++;
}
if (nr) {
mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
*p &= ~mask_to_clear;
}
}
static unsigned long __ffs(unsigned long word)
{
int num = 0;
if ((word & 0xffff) == 0) {
num += 16;
word >>= 16;
}
if ((word & 0xff) == 0) {
num += 8;
word >>= 8;
}
if ((word & 0xf) == 0) {
num += 4;
word >>= 4;
}
if ((word & 0x3) == 0) {
num += 2;
word >>= 2;
}
if ((word & 0x1) == 0)
num += 1;
return num;
}
unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
unsigned long offset)
{
const unsigned long *p = addr + BITOP_WORD(offset);
unsigned long result = offset & ~(BITS_PER_LONG-1);
unsigned long tmp;
if (offset >= size)
return size;
size -= result;
offset %= BITS_PER_LONG;
if (offset) {
tmp = *(p++);
tmp &= (~0UL << offset);
if (size < BITS_PER_LONG)
goto found_first;
if (tmp)
goto found_middle;
size -= BITS_PER_LONG;
result += BITS_PER_LONG;
}
while (size & ~(BITS_PER_LONG-1)) {
if ((tmp = *(p++)))
goto found_middle;
result += BITS_PER_LONG;
size -= BITS_PER_LONG;
}
if (!size)
return result;
tmp = *p;
found_first:
tmp &= (~0UL >> (BITS_PER_LONG - size));
if (tmp == 0UL) /* Are any bits set? */
return result + size; /* Nope. */
found_middle:
return result + __ffs(tmp);
}
#define ffz(x) __ffs(~(x))
unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
unsigned long offset)
{
const unsigned long *p = addr + BITOP_WORD(offset);
unsigned long result = offset & ~(BITS_PER_LONG-1);
unsigned long tmp;
if (offset >= size)
return size;
size -= result;
offset %= BITS_PER_LONG;
if (offset) {
tmp = *(p++);
tmp |= ~0UL >> (BITS_PER_LONG - offset);
if (size < BITS_PER_LONG)
goto found_first;
if (~tmp)
goto found_middle;
size -= BITS_PER_LONG;
result += BITS_PER_LONG;
}
while (size & ~(BITS_PER_LONG-1)) {
if (~(tmp = *(p++)))
goto found_middle;
result += BITS_PER_LONG;
size -= BITS_PER_LONG;
}
if (!size)
return result;
tmp = *p;
found_first:
tmp |= ~0UL << size;
if (tmp == ~0UL) /* Are any bits zero? */
return result + size; /* Nope. */
found_middle:
return result + ffz(tmp);
}
#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
static inline int test_bit(int nr, const volatile unsigned long *addr)
{
return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
}
unsigned long bitmap_find_next_zero_area(unsigned long *map,
unsigned long size,
unsigned long start,
unsigned int nr,
unsigned long align_mask)
{
unsigned long index, end, i;
again:
index = find_next_zero_bit(map, size, start);
/* Align allocation */
index = __ALIGN_MASK(index, align_mask);
end = index + nr;
#ifdef ORIGINAL
if (end >= size)
#else
if (end > size)
#endif
return end;
#ifdef ORIGINAL
for (i = index; i < end; i++) {
if (test_bit(i, map)) {
start = i+1;
goto again;
}
}
#else
i = find_next_bit(map, end, index);
if (i < end) {
start = i + 1;
goto again;
}
#endif
return index;
}
#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
#define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
#define DECLARE_BITMAP(name,bits) unsigned long name[BITS_TO_LONGS(bits)]
#endif /* Copy and paste from kernel source */
static DECLARE_BITMAP(bitmap, 1000);
static DECLARE_BITMAP(empty, 1000);
static DECLARE_BITMAP(full, 1000);
static void bitmap_dump(unsigned long *bitmap, int size)
{
int i;
for (i = 0; i < size; i++) {
if (test_bit(i, bitmap))
printf("1 ");
else
printf("0 ");
if (i % 10 == 9)
printf("\n");
}
printf("\n");
}
static int test1(int size)
{
int start = random() % size;
int nr = random() % (size - start);
memset(bitmap, 0x00, BITS_TO_LONGS(size) * sizeof(unsigned long));
bitmap_set(bitmap, start, nr);
bitmap_clear(bitmap, start, nr);
if (memcmp(empty, bitmap, BITS_TO_LONGS(size) * sizeof(unsigned long)))
goto error;
return 0;
error:
bitmap_dump(bitmap, size);
return 1;
}
int test2(int size)
{
int start = random() % size;
int nr = random() % (size - start);
memset(bitmap, 0xff, BITS_TO_LONGS(size) * sizeof(unsigned long));
bitmap_clear(bitmap, start, nr);
bitmap_set(bitmap, start, nr);
if (memcmp(full, bitmap, BITS_TO_LONGS(size) * sizeof(unsigned long)))
goto error;
return 0;
error:
bitmap_dump(bitmap, size);
return 1;
}
int test3(int size)
{
int start = random() % size;
int nr = random() % (size - start);
unsigned long offset;
memset(bitmap, 0x00, BITS_TO_LONGS(size) * sizeof(unsigned long));
bitmap_set(bitmap, start, nr);
if (start) {
offset = bitmap_find_next_zero_area(bitmap, size, 0, start, 0);
if (offset != 0) {
printf("start %ld nr %ld\n", start, nr);
printf("offset %ld != 0\n", offset);
goto error;
}
}
offset = bitmap_find_next_zero_area(bitmap, size, start,
size - (start + nr), 0);
if (offset != start + nr) {
printf("start %ld nr %ld\n", start, nr);
printf("offset %ld != size + nr %ld\n", offset, start + nr);
goto error;
}
return 0;
error:
bitmap_dump(bitmap, size);
return 1;
}
int test4(int size)
{
int start = random() % size;
int nr = random() % (size - start);
unsigned long offset;
memset(bitmap, 0xff, BITS_TO_LONGS(size) * sizeof(unsigned long));
bitmap_clear(bitmap, start, nr);
offset = bitmap_find_next_zero_area(bitmap, size, start, nr, 0);
if (nr != 0) {
if (offset != start) {
printf("start %ld nr %ld\n", start, nr);
printf("offset %ld != start %ld\n", offset, start);
goto error;
}
}
return 0;
error:
bitmap_dump(bitmap, size);
return 1;
}
int main(int argc, char *argv[])
{
int err = 0;
srandom(time(NULL));
memset(empty, 0x00, sizeof(empty));
memset(full, 0xff, sizeof(full));
while (!err) {
err |= test1(1000);
err |= test2(1000);
err |= test3(1000);
err |= test4(1000);
}
return 0;
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH -mmotm -v2] Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area. patch
2009-10-14 3:22 ` [PATCH -mmotm] Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area. patch Akinobu Mita
@ 2009-10-15 6:07 ` Akinobu Mita
0 siblings, 0 replies; 13+ messages in thread
From: Akinobu Mita @ 2009-10-15 6:07 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-usb, linux-ia64, linuxppc-dev, Paul Mackerras,
H. Peter Anvin, sparclinux, Lothar Wassmann, x86, linux-altix,
Ingo Molnar, Fenghua Yu, Yevgeny Petrilin, Thomas Gleixner,
Tony Luck, Kyle Hubert, netdev, Greg Kroah-Hartman, linux-kernel,
FUJITA Tomonori, David S. Miller
From: Akinobu Mita <akinobu.mita@gmail.com>
Subject: Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area.patch
- Rewrite bitmap_set and bitmap_clear
Instead of setting or clearing for each bit.
- Fix off-by-one errors in bitmap_find_next_zero_area
This bug was derived from find_next_zero_area in iommu-helper.
- Add kerneldoc for bitmap_find_next_zero_area
This patch is supposed to be folded into
bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area.patch
* -v2
- Remove an extra test at the "index" value by find_next_bit
Index was just returned by find_next_zero_bit, so we know it's zero.
Noticed-by: Kyle Hubert <khubert@gmail.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
lib/bitmap.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 2415da4..962b863 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -271,28 +271,62 @@ int __bitmap_weight(const unsigned long *bitmap, int bits)
}
EXPORT_SYMBOL(__bitmap_weight);
-void bitmap_set(unsigned long *map, int i, int len)
-{
- int end = i + len;
+#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
- while (i < end) {
- __set_bit(i, map);
- i++;
+void bitmap_set(unsigned long *map, int start, int nr)
+{
+ unsigned long *p = map + BIT_WORD(start);
+ const int size = start + nr;
+ int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
+ unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
+
+ while (nr - bits_to_set >= 0) {
+ *p |= mask_to_set;
+ nr -= bits_to_set;
+ bits_to_set = BITS_PER_LONG;
+ mask_to_set = ~0UL;
+ p++;
+ }
+ if (nr) {
+ mask_to_set &= BITMAP_LAST_WORD_MASK(size);
+ *p |= mask_to_set;
}
}
EXPORT_SYMBOL(bitmap_set);
void bitmap_clear(unsigned long *map, int start, int nr)
{
- int end = start + nr;
-
- while (start < end) {
- __clear_bit(start, map);
- start++;
+ unsigned long *p = map + BIT_WORD(start);
+ const int size = start + nr;
+ int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
+ unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
+
+ while (nr - bits_to_clear >= 0) {
+ *p &= ~mask_to_clear;
+ nr -= bits_to_clear;
+ bits_to_clear = BITS_PER_LONG;
+ mask_to_clear = ~0UL;
+ p++;
+ }
+ if (nr) {
+ mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
+ *p &= ~mask_to_clear;
}
}
EXPORT_SYMBOL(bitmap_clear);
+/*
+ * bitmap_find_next_zero_area - find a contiguous aligned zero area
+ * @map: The address to base the search on
+ * @size: The bitmap size in bits
+ * @start: The bitnumber to start searching at
+ * @nr: The number of zeroed bits we're looking for
+ * @align_mask: Alignment mask for zero area
+ *
+ * The @align_mask should be one less than a power of 2; the effect is that
+ * the bit offset of all zero areas this function finds is multiples of that
+ * power of 2. A @align_mask of 0 means no alignment is required.
+ */
unsigned long bitmap_find_next_zero_area(unsigned long *map,
unsigned long size,
unsigned long start,
@@ -304,12 +338,12 @@ again:
index = find_next_zero_bit(map, size, start);
/* Align allocation */
- index = (index + align_mask) & ~align_mask;
+ index = __ALIGN_MASK(index, align_mask);
end = index + nr;
- if (end >= size)
+ if (end > size)
return end;
- i = find_next_bit(map, end, index);
+ i = find_next_bit(map, end, index + 1);
if (i < end) {
start = i + 1;
goto again;
--
1.5.4.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-13 9:10 ` Akinobu Mita
2009-10-13 21:54 ` Michael Ellerman
2009-10-14 3:22 ` [PATCH -mmotm] Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area. patch Akinobu Mita
@ 2009-10-17 13:43 ` FUJITA Tomonori
2009-10-17 14:43 ` Akinobu Mita
2 siblings, 1 reply; 13+ messages in thread
From: FUJITA Tomonori @ 2009-10-17 13:43 UTC (permalink / raw)
To: akinobu.mita
Cc: linux-usb, linux-ia64, linuxppc-dev, paulus, hpa, sparclinux, LW,
x86, linux-altix, mingo, fenghua.yu, yevgenyp, tglx, tony.luck,
netdev, gregkh, linux-kernel, fujita.tomonori, akpm, davem
On Tue, 13 Oct 2009 18:10:17 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:
> My user space testing exposed off-by-one error find_next_zero_area
> in iommu-helper. Some zero area cannot be found by this bug.
>
> Subject: [PATCH] Fix off-by-one error in find_next_zero_area
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> lib/iommu-helper.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
> index 75dbda0..afc58bc 100644
> --- a/lib/iommu-helper.c
> +++ b/lib/iommu-helper.c
> @@ -19,7 +19,7 @@ again:
> index = (index + align_mask) & ~align_mask;
>
> end = index + nr;
> - if (end >= size)
> + if (end > size)
I think that this is intentional; the last byte of the limit doesn't
work.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-17 13:43 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area FUJITA Tomonori
@ 2009-10-17 14:43 ` Akinobu Mita
2009-10-17 14:51 ` FUJITA Tomonori
0 siblings, 1 reply; 13+ messages in thread
From: Akinobu Mita @ 2009-10-17 14:43 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: fenghua.yu, gregkh, tglx, tony.luck, x86, netdev, linux-kernel,
linux-altix, yevgenyp, linuxppc-dev, mingo, paulus, hpa,
sparclinux, linux-ia64, akpm, linux-usb, davem, LW
2009/10/17 FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>:
> On Tue, 13 Oct 2009 18:10:17 +0900
> Akinobu Mita <akinobu.mita@gmail.com> wrote:
>
>> My user space testing exposed off-by-one error find_next_zero_area
>> in iommu-helper. Some zero area cannot be found by this bug.
>>
>> Subject: [PATCH] Fix off-by-one error in find_next_zero_area
>>
>> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> ---
>> =A0lib/iommu-helper.c | =A0 =A02 +-
>> =A01 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
>> index 75dbda0..afc58bc 100644
>> --- a/lib/iommu-helper.c
>> +++ b/lib/iommu-helper.c
>> @@ -19,7 +19,7 @@ again:
>> =A0 =A0 =A0 index =3D (index + align_mask) & ~align_mask;
>>
>> =A0 =A0 =A0 end =3D index + nr;
>> - =A0 =A0 if (end >=3D size)
>> + =A0 =A0 if (end > size)
>
> I think that this is intentional; the last byte of the limit doesn't
> work.
It looks ok to me. Without above change, find_next_zero_area cannot
find a 64 bits zeroed area in next sample code.
unsigned long offset;
DECLARE_BITMAP(map, 64);
bitmap_clear(map, 0, 64);
offset =3D find_next_zero_area(map, 64, 0, 64, 0);
if (offset >=3D 64)
printf("not found\n");
else
printf("found\n");
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-17 14:43 ` Akinobu Mita
@ 2009-10-17 14:51 ` FUJITA Tomonori
2009-10-17 15:42 ` Akinobu Mita
0 siblings, 1 reply; 13+ messages in thread
From: FUJITA Tomonori @ 2009-10-17 14:51 UTC (permalink / raw)
To: akinobu.mita
Cc: linux-usb, linux-ia64, linuxppc-dev, paulus, hpa, sparclinux, LW,
x86, linux-altix, mingo, fenghua.yu, yevgenyp, tglx, tony.luck,
netdev, gregkh, linux-kernel, fujita.tomonori, akpm, davem
On Sat, 17 Oct 2009 23:43:56 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:
> 2009/10/17 FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>:
> > On Tue, 13 Oct 2009 18:10:17 +0900
> > Akinobu Mita <akinobu.mita@gmail.com> wrote:
> >
> >> My user space testing exposed off-by-one error find_next_zero_area
> >> in iommu-helper. Some zero area cannot be found by this bug.
> >>
> >> Subject: [PATCH] Fix off-by-one error in find_next_zero_area
> >>
> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >> ---
> >> =A0lib/iommu-helper.c | =A0 =A02 +-
> >> =A01 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
> >> index 75dbda0..afc58bc 100644
> >> --- a/lib/iommu-helper.c
> >> +++ b/lib/iommu-helper.c
> >> @@ -19,7 +19,7 @@ again:
> >> =A0 =A0 =A0 index =3D (index + align_mask) & ~align_mask;
> >>
> >> =A0 =A0 =A0 end =3D index + nr;
> >> - =A0 =A0 if (end >=3D size)
> >> + =A0 =A0 if (end > size)
> >
> > I think that this is intentional; the last byte of the limit doesn't=
> > work.
> =
> It looks ok to me. Without above change, find_next_zero_area cannot
> find a 64 bits zeroed area in next sample code.
I meant that we don't want to find such area for IOMMUs (IIRC, it code
came from POWER IOMMU).
> unsigned long offset;
> =
> DECLARE_BITMAP(map, 64);
> =
> bitmap_clear(map, 0, 64);
> offset =3D find_next_zero_area(map, 64, 0, 64, 0);
> if (offset >=3D 64)
> printf("not found\n");
> else
> printf("found\n");
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" =
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
2009-10-17 14:51 ` FUJITA Tomonori
@ 2009-10-17 15:42 ` Akinobu Mita
0 siblings, 0 replies; 13+ messages in thread
From: Akinobu Mita @ 2009-10-17 15:42 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: fenghua.yu, gregkh, tglx, tony.luck, x86, netdev, linux-kernel,
linux-altix, yevgenyp, linuxppc-dev, mingo, paulus, hpa,
sparclinux, linux-ia64, akpm, linux-usb, davem, LW
>> >> --- a/lib/iommu-helper.c
>> >> +++ b/lib/iommu-helper.c
>> >> @@ -19,7 +19,7 @@ again:
>> >> =A0 =A0 =A0 index =3D (index + align_mask) & ~align_mask;
>> >>
>> >> =A0 =A0 =A0 end =3D index + nr;
>> >> - =A0 =A0 if (end >=3D size)
>> >> + =A0 =A0 if (end > size)
>> >
>> > I think that this is intentional; the last byte of the limit doesn't
>> > work.
>>
>> It looks ok to me. Without above change, find_next_zero_area cannot
>> find a 64 bits zeroed area in next sample code.
>
> I meant that we don't want to find such area for IOMMUs (IIRC, it code
> came from POWER IOMMU).
OK, I see. I think we need the comment about it.
So we cannot replace find_next_zero_area by bitmap_find_next_zero_area
and current -mmotm has the bug introduced by this patch in iommu-helper
and I also introduced the bug in bitmap_find_next_zero_area if
align_mask !=3D 0 in
bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area-fix.pat=
ch
Andrew, please drop
lib-iommu-helperc-fix-off-by-one-error-in-find_next_zero_area.patch
iommu-helper-simplify-find_next_zero_area.patch
bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area.patch
bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area-fix.pat=
ch
iommu-helper-use-bitmap-library.patch
isp1362-hcd-use-bitmap_find_next_zero_area.patch
mlx4-use-bitmap_find_next_zero_area.patch
sparc-use-bitmap_find_next_zero_area.patch
ia64-use-bitmap_find_next_zero_area.patch
genalloc-use-bitmap_find_next_zero_area.patch
I'll overhaul the patchset and retry again.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2009-10-17 15:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1255076961-21325-1-git-send-email-akinobu.mita@gmail.com>
2009-10-09 8:29 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Akinobu Mita
2009-10-09 8:29 ` [PATCH 3/8] iommu-helper: Use bitmap library Akinobu Mita
2009-10-09 23:41 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Andrew Morton
2009-10-13 2:18 ` Akinobu Mita
2009-10-13 9:10 ` Akinobu Mita
2009-10-13 21:54 ` Michael Ellerman
2009-10-14 3:39 ` Akinobu Mita
2009-10-14 3:22 ` [PATCH -mmotm] Fix bitmap-introduce-bitmap_set-bitmap_clear-bitmap_find_next_zero_area. patch Akinobu Mita
2009-10-15 6:07 ` [PATCH -mmotm -v2] " Akinobu Mita
2009-10-17 13:43 ` [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area FUJITA Tomonori
2009-10-17 14:43 ` Akinobu Mita
2009-10-17 14:51 ` FUJITA Tomonori
2009-10-17 15:42 ` Akinobu Mita
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).