2008-04-05 10:24:33

by Adrian Bunk

[permalink] [raw]
Subject: ext4 compile error on m68k

Commit aa02ad67d9b308290fde390682cd039b29f7ab85
"ext4: Add ext4_find_next_bit()" causes the following regression:

<-- snip -->

...
CC [M] fs/ext4/mballoc.o
/home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c: In function 'mb_find_next_bit':
/home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c:696: error: implicit declaration of function 'generic_find_next_le_bit'
make[3]: *** [fs/ext4/mballoc.o] Error 1

<-- snip -->

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed



2008-04-05 11:48:43

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: ext4 compile error on m68k

On Sat, 5 Apr 2008, Adrian Bunk wrote:
> Commit aa02ad67d9b308290fde390682cd039b29f7ab85
> "ext4: Add ext4_find_next_bit()" causes the following regression:
>
> <-- snip -->
>
> ...
> CC [M] fs/ext4/mballoc.o
> /home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c: In function 'mb_find_next_bit':
> /home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c:696: error: implicit declaration of function 'generic_find_next_le_bit'
> make[3]: *** [fs/ext4/mballoc.o] Error 1
>
> <-- snip -->

Known issue. The ext4 developers added a #define (with a different name than in
the patch comment) in the commit below, but forgot to make sure
generic_find_next_le_bit() is actually available.

commit aa02ad67d9b308290fde390682cd039b29f7ab85
Author: Aneesh Kumar K.V <[email protected]>
Date: Mon Jan 28 23:58:27 2008 -0500

ext4: Add ext4_find_next_bit()

This function is used by the ext4 multi block allocator patches.

Also add generic_find_next_le_bit

Signed-off-by: Aneesh Kumar K.V <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>

diff --git a/include/asm-m68k/bitops.h b/include/asm-m68k/bitops.h
index 2976b5d..83d1f28 100644
--- a/include/asm-m68k/bitops.h
+++ b/include/asm-m68k/bitops.h
@@ -410,6 +410,8 @@ static inline int ext2_find_next_zero_bit(const void *vaddr
res = ext2_find_first_zero_bit (p, size - 32 * (p - addr));
return (p - addr) * 32 + res;
}
+#define ext2_find_next_bit(addr, size, off) \
+ generic_find_next_le_bit((unsigned long *)(addr), (size), (off))

#endif /* __KERNEL__ */


Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2008-04-10 10:42:08

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: ext4 compile error on m68k

On Sat, Apr 05, 2008 at 01:48:02PM +0200, Geert Uytterhoeven wrote:
> On Sat, 5 Apr 2008, Adrian Bunk wrote:
> > Commit aa02ad67d9b308290fde390682cd039b29f7ab85
> > "ext4: Add ext4_find_next_bit()" causes the following regression:
> >
> > <-- snip -->
> >
> > ...
> > CC [M] fs/ext4/mballoc.o
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c: In function 'mb_find_next_bit':
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c:696: error: implicit declaration of function 'generic_find_next_le_bit'
> > make[3]: *** [fs/ext4/mballoc.o] Error 1
> >
> > <-- snip -->
>
> Known issue. The ext4 developers added a #define (with a different name than in
> the patch comment) in the commit below, but forgot to make sure
> generic_find_next_le_bit() is actually available.
>

generic_find_next_le_bit is defined in lib/find_next_bit.c.

But m68k doesn't want to use the GENERIC_FIND_NEXT_BIT. Can I request
somebody knowledgeable about m68k to give a try in implementing
generic_find_next_le_bit equivalent on m68k ?

-aneesh

2008-04-10 11:27:24

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: ext4 compile error on m68k

On Thu, Apr 10, 2008 at 04:11:32PM +0530, Aneesh Kumar K.V wrote:
> On Sat, Apr 05, 2008 at 01:48:02PM +0200, Geert Uytterhoeven wrote:
> > On Sat, 5 Apr 2008, Adrian Bunk wrote:
> > > Commit aa02ad67d9b308290fde390682cd039b29f7ab85
> > > "ext4: Add ext4_find_next_bit()" causes the following regression:
> > >
> > > <-- snip -->
> > >
> > > ...
> > > CC [M] fs/ext4/mballoc.o
> > > /home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c: In function 'mb_find_next_bit':
> > > /home/bunk/linux/kernel-2.6/git/linux-2.6/fs/ext4/mballoc.c:696: error: implicit declaration of function 'generic_find_next_le_bit'
> > > make[3]: *** [fs/ext4/mballoc.o] Error 1
> > >
> > > <-- snip -->
> >
> > Known issue. The ext4 developers added a #define (with a different name than in
> > the patch comment) in the commit below, but forgot to make sure
> > generic_find_next_le_bit() is actually available.
> >
>
> generic_find_next_le_bit is defined in lib/find_next_bit.c.
>
> But m68k doesn't want to use the GENERIC_FIND_NEXT_BIT. Can I request
> somebody knowledgeable about m68k to give a try in implementing
> generic_find_next_le_bit equivalent on m68k ?
>

Is this ok ? It is derived out of the ext2_find_next_zero_bit
found in the same file. Compile tested with crosstools

diff --git a/include/asm-m68k/bitops.h b/include/asm-m68k/bitops.h
index 83d1f28..4795bd1 100644
--- a/include/asm-m68k/bitops.h
+++ b/include/asm-m68k/bitops.h
@@ -410,8 +410,51 @@ static inline int ext2_find_next_zero_bit(const void *vaddr, unsigned size,
res = ext2_find_first_zero_bit (p, size - 32 * (p - addr));
return (p - addr) * 32 + res;
}
-#define ext2_find_next_bit(addr, size, off) \
- generic_find_next_le_bit((unsigned long *)(addr), (size), (off))
+
+static inline int ext2_find_first_bit(const void *vaddr, unsigned size)
+{
+ /* vaddr is long aligned */
+ const unsigned long *p = vaddr, *addr = vaddr;
+ int res;
+
+ if (!size)
+ return 0;
+
+ size = (size >> 5) + ((size & 31) > 0);
+ while (*p++ == 0UL)
+ {
+ if (--size == 0)
+ return (p - addr) << 5;
+ }
+
+ --p;
+ for (res = 0; res < 32; res++)
+ if (ext2_test_bit(res, p))
+ break;
+ return (p - addr) * 32 + res;
+}
+
+static inline unsigned long ext2_find_next_bit(const unsigned long *vaddr,
+ unsigned long size, unsigned long offset)
+{
+ const unsigned long *addr = vaddr;
+ const unsigned long *p = addr + (offset >> 5);
+ int bit = offset & 31UL, res;
+
+ if (bit) {
+ /* addr + offse is not long aligned so search till we
+ * have an aligned address
+ */
+ /* Look for one in first longword */
+ for (res = bit; res < 32; res++)
+ if (ext2_test_bit(res, p))
+ return (p - addr) * 32 + res;
+ p++;
+ }
+ /* No set bit yet, search remaining full bytes for a set bit */
+ res = ext2_find_first_bit(p, size - 32 * (p - addr));
+ return (p - addr) * 32 + res;
+}

#endif /* __KERNEL__ */


2008-04-10 11:48:59

by Andreas Schwab

[permalink] [raw]
Subject: Re: ext4 compile error on m68k

"Aneesh Kumar K.V" <[email protected]> writes:

> But m68k doesn't want to use the GENERIC_FIND_NEXT_BIT. Can I request
> somebody knowledgeable about m68k to give a try in implementing
> generic_find_next_le_bit equivalent on m68k ?

What's the exact semantic of generic_find_next_le_bit? Why is it not
documented?

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2008-04-10 13:58:31

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: ext4 compile error on m68k

On Thu, Apr 10, 2008 at 01:48:55PM +0200, Andreas Schwab wrote:
> "Aneesh Kumar K.V" <[email protected]> writes:
>
> > But m68k doesn't want to use the GENERIC_FIND_NEXT_BIT. Can I request
> > somebody knowledgeable about m68k to give a try in implementing
> > generic_find_next_le_bit equivalent on m68k ?
>
> What's the exact semantic of generic_find_next_le_bit? Why is it not
> documented?
>

Should be same ext2_find_next_zero_bit except that it find the next
set bit rather than the zero bit. The API is used to find the set bit
in bitmap. To explain the little endian dependency, what we have is

0 x n
[.......1.......]

Now we read this as (char *) and try to find the first set bit which
in the above example is x. To speed up most of the architectures would
like to consider it as an array of long. That implies that the value
in the array should be considered as a little endian long, hence the
*_le_bit.


-aneesh

2008-04-10 14:02:42

by Christoph Hellwig

[permalink] [raw]
Subject: Re: ext4 compile error on m68k

> Should be same ext2_find_next_zero_bit except that it find the next
> set bit rather than the zero bit. The API is used to find the set bit
> in bitmap. To explain the little endian dependency, what we have is
>
> 0 x n
> [.......1.......]
>
> Now we read this as (char *) and try to find the first set bit which
> in the above example is x. To speed up most of the architectures would
> like to consider it as an array of long. That implies that the value
> in the array should be considered as a little endian long, hence the
> *_le_bit.

This kind of documentation should be in the kernel tree somewhere.
And while we're at it all the routines should be renamed from extN_*
to *_le_bit because that describes what they do.