ext4 compile error on m68k

ext4 compile error on m68k

[Adrian Bunk]

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

Re: ext4 compile error on m68k

[Geert Uytterhoeven]

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 <aneesh.kumar@linux.vnet.ibm.com>
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 <aneesh.kumar@linux.vnet.ibm.com>
    Cc: <linux-ext4@vger.kernel.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

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 -- geert@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

Re: ext4 compile error on m68k

[Aneesh Kumar K.V]

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

Re: ext4 compile error on m68k

[Aneesh Kumar K.V]

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__ */
 

Re: ext4 compile error on m68k

[Andreas Schwab]

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> 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, schwab@suse.de
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."
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ext4 compile error on m68k

[Aneesh Kumar K.V]

On Thu, Apr 10, 2008 at 01:48:55PM +0200, Andreas Schwab wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> 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

Re: ext4 compile error on m68k

[Christoph Hellwig]

> 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.