Prev: cgroups: fix failure path in cgroup_write_event_control()
Next: pm_op(): usb_dev_suspend+0x0/0x10 returns -2 on USB device 8087:0020
From: Jan Kara on 24 Feb 2010 11:30 On Tue 23-02-10 23:11:13, Akinobu Mita wrote: > Use ext2_find_next_bit (generic_find_next_le_bit) to find the set bit > in little endian bitmap region. > > Signed-off-by: Akinobu Mita <akinobu.mita(a)gmail.com> > Cc: Jan Kara <jack(a)suse.cz> I've looked at the code and I think this is wrong. UDF uses 1 for free block in the bitmap and 0 for used one. So you need to use generic_find_next_le_bit... Something like the patch below? Honza -- Jan Kara <jack(a)suse.cz> SUSE Labs, CR --- From bfc8c674188d4be5856e445f9b57e1b7a3dbff6d Mon Sep 17 00:00:00 2001 From: Jan Kara <jack(a)suse.cz> Date: Wed, 24 Feb 2010 17:18:57 +0100 Subject: [PATCH] udf: Use generic function for bit searching Use generic_find_next_le_bit to search for bits in bitmap so that we don't duplicate code unnecessarily. Signed-off-by: Jan Kara <jack(a)suse.cz> --- fs/udf/balloc.c | 51 ++------------------------------------------------- 1 files changed, 2 insertions(+), 49 deletions(-) diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c index 82372e3..f5ff06c 100644 --- a/fs/udf/balloc.c +++ b/fs/udf/balloc.c @@ -31,55 +31,8 @@ #define udf_clear_bit(nr, addr) ext2_clear_bit(nr, addr) #define udf_set_bit(nr, addr) ext2_set_bit(nr, addr) #define udf_test_bit(nr, addr) ext2_test_bit(nr, addr) -#define udf_find_first_one_bit(addr, size) find_first_one_bit(addr, size) #define udf_find_next_one_bit(addr, size, offset) \ - find_next_one_bit(addr, size, offset) - -#define leBPL_to_cpup(x) leNUM_to_cpup(BITS_PER_LONG, x) -#define leNUM_to_cpup(x, y) xleNUM_to_cpup(x, y) -#define xleNUM_to_cpup(x, y) (le ## x ## _to_cpup(y)) -#define uintBPL_t uint(BITS_PER_LONG) -#define uint(x) xuint(x) -#define xuint(x) __le ## x - -static inline int find_next_one_bit(void *addr, int size, int offset) -{ - uintBPL_t *p = ((uintBPL_t *) addr) + (offset / BITS_PER_LONG); - int result = offset & ~(BITS_PER_LONG - 1); - unsigned long tmp; - - if (offset >= size) - return size; - size -= result; - offset &= (BITS_PER_LONG - 1); - if (offset) { - tmp = leBPL_to_cpup(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)) { - tmp = leBPL_to_cpup(p++); - if (tmp) - goto found_middle; - result += BITS_PER_LONG; - size -= BITS_PER_LONG; - } - if (!size) - return result; - tmp = leBPL_to_cpup(p); -found_first: - tmp &= ~0UL >> (BITS_PER_LONG - size); -found_middle: - return result + ffz(~tmp); -} - -#define find_first_one_bit(addr, size)\ - find_next_one_bit((addr), (size), 0) + generic_find_next_le_bit((unsigned long *)addr, size, offset) static int read_block_bitmap(struct super_block *sb, struct udf_bitmap *bitmap, unsigned int block, @@ -356,7 +309,7 @@ repeat: break; } } else { - bit = udf_find_next_one_bit((char *)bh->b_data, + bit = udf_find_next_one_bit(bh->b_data, sb->s_blocksize << 3, group_start << 3); if (bit < sb->s_blocksize << 3) -- 1.6.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Akinobu Mita on 24 Feb 2010 20:40 2010/2/25 Jan Kara <jack(a)suse.cz>: > On Tue 23-02-10 23:11:13, Akinobu Mita wrote: >> Use ext2_find_next_bit (generic_find_next_le_bit) to find the set bit >> in little endian bitmap region. >> >> Signed-off-by: Akinobu Mita <akinobu.mita(a)gmail.com> >> Cc: Jan Kara <jack(a)suse.cz> > �I've looked at the code and I think this is wrong. UDF uses 1 for free > block in the bitmap and 0 for used one. So you need to use > generic_find_next_le_bit... > �Something like the patch below? According to include/asm-generic/bitops/ext2-non-atomic.h, generic_find_next_le_bit() == ext2_find_next_bit(). So, is there any difference between my patch and yours? Or am I missing something? BTW, I realized that udf_find_first_one_bit() macro is not used. So you can remove it in this patch. > � � � � � � � � � � � � � � � � � � � � � � � � � � � � � � � �Honza > > -- > Jan Kara <jack(a)suse.cz> > SUSE Labs, CR > --- > > From bfc8c674188d4be5856e445f9b57e1b7a3dbff6d Mon Sep 17 00:00:00 2001 > From: Jan Kara <jack(a)suse.cz> > Date: Wed, 24 Feb 2010 17:18:57 +0100 > Subject: [PATCH] udf: Use generic function for bit searching > > Use generic_find_next_le_bit to search for bits in bitmap so > that we don't duplicate code unnecessarily. > > Signed-off-by: Jan Kara <jack(a)suse.cz> > --- > �fs/udf/balloc.c | � 51 ++------------------------------------------------- > �1 files changed, 2 insertions(+), 49 deletions(-) > > diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c > index 82372e3..f5ff06c 100644 > --- a/fs/udf/balloc.c > +++ b/fs/udf/balloc.c > @@ -31,55 +31,8 @@ > �#define udf_clear_bit(nr, addr) ext2_clear_bit(nr, addr) > �#define udf_set_bit(nr, addr) ext2_set_bit(nr, addr) > �#define udf_test_bit(nr, addr) ext2_test_bit(nr, addr) > -#define udf_find_first_one_bit(addr, size) find_first_one_bit(addr, size) > �#define udf_find_next_one_bit(addr, size, offset) \ > - � � � � � � � find_next_one_bit(addr, size, offset) > - > -#define leBPL_to_cpup(x) leNUM_to_cpup(BITS_PER_LONG, x) > -#define leNUM_to_cpup(x, y) xleNUM_to_cpup(x, y) > -#define xleNUM_to_cpup(x, y) (le ## x ## _to_cpup(y)) > -#define uintBPL_t uint(BITS_PER_LONG) > -#define uint(x) xuint(x) > -#define xuint(x) __le ## x > - > -static inline int find_next_one_bit(void *addr, int size, int offset) > -{ > - � � � uintBPL_t *p = ((uintBPL_t *) addr) + (offset / BITS_PER_LONG); > - � � � int result = offset & ~(BITS_PER_LONG - 1); > - � � � unsigned long tmp; > - > - � � � if (offset >= size) > - � � � � � � � return size; > - � � � size -= result; > - � � � offset &= (BITS_PER_LONG - 1); > - � � � if (offset) { > - � � � � � � � tmp = leBPL_to_cpup(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)) { > - � � � � � � � tmp = leBPL_to_cpup(p++); > - � � � � � � � if (tmp) > - � � � � � � � � � � � goto found_middle; > - � � � � � � � result += BITS_PER_LONG; > - � � � � � � � size -= BITS_PER_LONG; > - � � � } > - � � � if (!size) > - � � � � � � � return result; > - � � � tmp = leBPL_to_cpup(p); > -found_first: > - � � � tmp &= ~0UL >> (BITS_PER_LONG - size); > -found_middle: > - � � � return result + ffz(~tmp); > -} > - > -#define find_first_one_bit(addr, size)\ > - � � � find_next_one_bit((addr), (size), 0) > + � � � � � � � generic_find_next_le_bit((unsigned long *)addr, size, offset) > > �static int read_block_bitmap(struct super_block *sb, > � � � � � � � � � � � � � � struct udf_bitmap *bitmap, unsigned int block, > @@ -356,7 +309,7 @@ repeat: > � � � � � � � � � � � � � � � �break; > � � � � � � � � � � � �} > � � � � � � � �} else { > - � � � � � � � � � � � bit = udf_find_next_one_bit((char *)bh->b_data, > + � � � � � � � � � � � bit = udf_find_next_one_bit(bh->b_data, > � � � � � � � � � � � � � � � � � � � � � � � � � �sb->s_blocksize << 3, > � � � � � � � � � � � � � � � � � � � � � � � � � �group_start << 3); > � � � � � � � � � � � �if (bit < sb->s_blocksize << 3) > -- > 1.6.4.2 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Geert Uytterhoeven on 25 Feb 2010 04:00 On Thu, Feb 25, 2010 at 02:38, Akinobu Mita <akinobu.mita(a)gmail.com> wrote: > 2010/2/25 Jan Kara <jack(a)suse.cz>: >> On Tue 23-02-10 23:11:13, Akinobu Mita wrote: >>> Use ext2_find_next_bit (generic_find_next_le_bit) to find the set bit >>> in little endian bitmap region. Is any of this in linux-next now? http://kisskb.ellerman.id.au/kisskb/buildresult/2208481/ | fs/udf/balloc.c:274: error: implicit declaration of function 'generic_find_next_le_bit' Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Akinobu Mita on 25 Feb 2010 09:40 2010/2/25 Geert Uytterhoeven <geert(a)linux-m68k.org>: > On Thu, Feb 25, 2010 at 02:38, Akinobu Mita <akinobu.mita(a)gmail.com> wrote: >> 2010/2/25 Jan Kara <jack(a)suse.cz>: >>> On Tue 23-02-10 23:11:13, Akinobu Mita wrote: >>>> Use ext2_find_next_bit (generic_find_next_le_bit) to find the set bit >>>> in little endian bitmap region. > > Is any of this in linux-next now? > > http://kisskb.ellerman.id.au/kisskb/buildresult/2208481/ > | fs/udf/balloc.c:274: error: implicit declaration of function > 'generic_find_next_le_bit' Yep, Jan's patch caused the build breakage. Because generic_find_next_le_bit() is not available for all archtectures. So we should use ext2_find_next_bit() here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Geert Uytterhoeven on 28 Feb 2010 05:10
On Thu, Feb 25, 2010 at 15:29, Akinobu Mita <akinobu.mita(a)gmail.com> wrote: > 2010/2/25 Geert Uytterhoeven <geert(a)linux-m68k.org>: >> On Thu, Feb 25, 2010 at 02:38, Akinobu Mita <akinobu.mita(a)gmail.com> wrote: >>> 2010/2/25 Jan Kara <jack(a)suse.cz>: >>>> On Tue 23-02-10 23:11:13, Akinobu Mita wrote: >>>>> Use ext2_find_next_bit (generic_find_next_le_bit) to find the set bit >>>>> in little endian bitmap region. >> >> Is any of this in linux-next now? >> >> http://kisskb.ellerman.id.au/kisskb/buildresult/2208481/ >> | fs/udf/balloc.c:274: error: implicit declaration of function >> 'generic_find_next_le_bit' > > Yep, Jan's patch caused the build breakage. > > Because generic_find_next_le_bit() is not available for all > archtectures. So we should use ext2_find_next_bit() here. Most architectures use the definitions in asm-generic, so they're OK. M68k doesn't. S390 is also affected, and I think arm as well (but there's no arm all-modconfig build in linux-next, so I'm not 100% sure). I'm cooking a patch... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |