Bitmap alignment issues....

Bitmap alignment issues....

[Tom Evans]

Hi All -

I am new to the list - I have been using Reiser4 on Linux/Alpha
for several months now and like very much what it offers.

(I use a -mm kernel from kernel.org with a Debian distro)

Recently I became aware or a large number or alignment fixups
occurring in the kernel, specifically in the reiser4 code.

(My machine(s) would occasionally slow to a crawl when doing
 intense disk i/o - I thought it was because I had recently
 started using IDE drives in my systems (the IDE controllers
 on these boards are notoriously slow) ).

I determined that 50k+ alignment faults were happening
per second - I traced the majority down to these 3 functions:

1) reiser4_find_next_set_bit  (in fs/reiser4/plugins/space/bitmap.c)
2) find_next_zero_bit (in asm/bitops.h)
3) replace_extent (in fs/reiser4/plugins/item/extent.c)

The first 2 accounted for ~4million faults during boot - essentially,
the base address of the bit array is not aligned properly
for ulong accesses on Alpha (which are 64bit).

I worked around this by using "get_unaligned()" for the array
accesses in #1 and implemented a version of "find_next_zero_bit"
local to reiser4 that handles unaligned base addresses (no need
for all other users of the function to suffer the same performance hit).

Those 2 changes brought the number of alignment fixups to 0 on a boot - 
the #3 issue accounts for a very small number of fixups - I have not
yet found the exact location.

While these changes eliminated the faults, they are not optimal,
get_unaligned() adds many cycles over a standard load.

I wanted to see if anyone here was already familiar with this issue and
what the process would be for getting it addressed.

Thanks for your patience!

...tom

Re: Bitmap alignment issues....

[Alex Zarochentsev]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

hello,

On Wed, Feb 09, 2005 at 12:59:17PM -0500, Tom Evans wrote:
> 
> Hi All -
> 
> I am new to the list - I have been using Reiser4 on Linux/Alpha
> for several months now and like very much what it offers.
> 
> (I use a -mm kernel from kernel.org with a Debian distro)
> 
> Recently I became aware or a large number or alignment fixups
> occurring in the kernel, specifically in the reiser4 code.
> 
> (My machine(s) would occasionally slow to a crawl when doing
>  intense disk i/o - I thought it was because I had recently
>  started using IDE drives in my systems (the IDE controllers
>  on these boards are notoriously slow) ).
> 
> I determined that 50k+ alignment faults were happening
> per second - I traced the majority down to these 3 functions:
> 
> 1) reiser4_find_next_set_bit  (in fs/reiser4/plugins/space/bitmap.c)
> 2) find_next_zero_bit (in asm/bitops.h)
> 3) replace_extent (in fs/reiser4/plugins/item/extent.c)
> 
> The first 2 accounted for ~4million faults during boot - essentially,
> the base address of the bit array is not aligned properly
> for ulong accesses on Alpha (which are 64bit).
> 
> I worked around this by using "get_unaligned()" for the array
> accesses in #1 and implemented a version of "find_next_zero_bit"
> local to reiser4 that handles unaligned base addresses (no need
> for all other users of the function to suffer the same performance hit).
> 
> Those 2 changes brought the number of alignment fixups to 0 on a boot - 
> the #3 issue accounts for a very small number of fixups - I have not
> yet found the exact location.
> 
> While these changes eliminated the faults, they are not optimal,
> get_unaligned() adds many cycles over a standard load.
> 
> I wanted to see if anyone here was already familiar with this issue and
> what the process would be for getting it addressed.

it is known. ca you try the attached patch?

> 
> Thanks for your patience!
> 
> ...tom
> 

-- 
Alex.

[-- Attachment #2: bitmap-word-align-2.diff --]
[-- Type: text/plain, Size: 2528 bytes --]

===== plugin/space/bitmap.c 1.186 vs edited =====
--- 1.186/plugin/space/bitmap.c	Wed Jan 19 18:52:52 2005
+++ edited/plugin/space/bitmap.c	Sun Feb  6 19:23:01 2005
@@ -54,13 +54,15 @@
 
 #define CHECKSUM_SIZE    4
 
+#define BYTES_PER_LONG   (sizeof(long))
+
 #if BITS_PER_LONG == 64
 #  define LONG_INT_SHIFT (6)
 #else
 #  define LONG_INT_SHIFT (5)
 #endif
 
-#define LONG_INT_MASK (BITS_PER_LONG - 1)
+#define LONG_INT_MASK (BITS_PER_LONG - 1UL)
 
 typedef unsigned long ulong_t;
 
@@ -179,17 +181,46 @@
 
 #include <asm/bitops.h>
 
+#if BITS_PER_LONG == 64
+
+#define OFF(addr)  (((ulong_t)(addr) & (BYTES_PER_LONG - 1)) << 3)
+#define BASE(addr) ((ulong_t*) ((ulong_t)(addr) & ~(BYTES_PER_LONG - 1)))
+
+static inline void reiser4_set_bit(int nr, void * addr)
+{
+	ext2_set_bit(nr + OFF(addr), BASE(addr));
+}
+
+static inline void reiser4_clear_bit(int nr, void * addr)
+{
+	ext2_clear_bit(nr + OFF(addr), BASE(addr));
+}
+
+static inline int reiser4_test_bit(int nr, void * addr)
+{
+	return ext2_test_bit(nr + OFF(addr), BASE(addr));
+}
+static inline int reiser4_find_next_zero_bit(void * addr, int maxoffset, int offset) 
+{
+	int off = OFF(addr);
+
+	return ext2_find_next_zero_bit(BASE(addr), maxoffset + off, offset + off) - off;
+}
+
+#else
+
 #define reiser4_set_bit(nr, addr)    ext2_set_bit(nr, addr)
 #define reiser4_clear_bit(nr, addr)  ext2_clear_bit(nr, addr)
 #define reiser4_test_bit(nr, addr)  ext2_test_bit(nr, addr)
 
 #define reiser4_find_next_zero_bit(addr, maxoffset, offset) \
 ext2_find_next_zero_bit(addr, maxoffset, offset)
+#endif
 
 /* Search for a set bit in the bit array [@start_offset, @max_offset[, offsets
  * are counted from @addr, return the offset of the first bit if it is found,
  * @maxoffset otherwise. */
-static bmap_off_t reiser4_find_next_set_bit(
+static bmap_off_t __reiser4_find_next_set_bit(
 	void *addr, bmap_off_t max_offset, bmap_off_t start_offset)
 {
 	ulong_t *base = addr;
@@ -225,6 +256,21 @@
 
 	return max_offset;
 }
+
+#if BITS_PER_LONG == 64 
+
+static bmap_off_t reiser4_find_next_set_bit(
+	void *addr, bmap_off_t max_offset, bmap_off_t start_offset)
+{
+	bmap_off_t off = OFF(addr);
+
+	return __reiser4_find_next_set_bit(BASE(addr), max_offset + off, start_offset + off) - off;
+}
+
+#else
+#define reiser4_find_next_set_bit(addr, max_offset, start_offset) \
+  __reiser4_find_next_set_bit(addr, max_offset, start_offset) 
+#endif
 
 /* search for the first set bit in single word. */
 static int find_last_set_bit_in_word (ulong_t word, int start_bit)

Re: Bitmap alignment issues....

[Thomas Evans]

Thanks for the patch the helped with the vast majority of the fixups.

Now there are just a trickle occurring in various places - would it be helpful 
for me to track them down, or are there folks who are already pursuing this?

...tom

On Wednesday 09 February 2005 01:19 pm, Alex Zarochentsev wrote:
> hello,
>
> On Wed, Feb 09, 2005 at 12:59:17PM -0500, Tom Evans wrote:
> > Hi All -
> >
> > I am new to the list - I have been using Reiser4 on Linux/Alpha
> > for several months now and like very much what it offers.
> >
> > (I use a -mm kernel from kernel.org with a Debian distro)
> >
> > Recently I became aware or a large number or alignment fixups
> > occurring in the kernel, specifically in the reiser4 code.
> >
> > (My machine(s) would occasionally slow to a crawl when doing
> >  intense disk i/o - I thought it was because I had recently
> >  started using IDE drives in my systems (the IDE controllers
> >  on these boards are notoriously slow) ).
> >
> > I determined that 50k+ alignment faults were happening
> > per second - I traced the majority down to these 3 functions:
> >
> > 1) reiser4_find_next_set_bit  (in fs/reiser4/plugins/space/bitmap.c)
> > 2) find_next_zero_bit (in asm/bitops.h)
> > 3) replace_extent (in fs/reiser4/plugins/item/extent.c)
> >
> > The first 2 accounted for ~4million faults during boot - essentially,
> > the base address of the bit array is not aligned properly
> > for ulong accesses on Alpha (which are 64bit).
> >
> > I worked around this by using "get_unaligned()" for the array
> > accesses in #1 and implemented a version of "find_next_zero_bit"
> > local to reiser4 that handles unaligned base addresses (no need
> > for all other users of the function to suffer the same performance hit).
> >
> > Those 2 changes brought the number of alignment fixups to 0 on a boot -
> > the #3 issue accounts for a very small number of fixups - I have not
> > yet found the exact location.
> >
> > While these changes eliminated the faults, they are not optimal,
> > get_unaligned() adds many cycles over a standard load.
> >
> > I wanted to see if anyone here was already familiar with this issue and
> > what the process would be for getting it addressed.
>
> it is known. ca you try the attached patch?
>
> > Thanks for your patience!
> >
> > ...tom

Re: Bitmap alignment issues....

[Alex Zarochentsev]

On Thu, Feb 10, 2005 at 12:10:14AM -0500, Thomas Evans wrote:
> 
> Thanks for the patch the helped with the vast majority of the fixups.
> 
> Now there are just a trickle occurring in various places - would it be helpful 
> for me to track them down, or are there folks who are already pursuing this?

which kernel are you using for alpha axp ?   did it require additional
patches to vanilla kernel?

> ...tom
> 
-- 
Alex.

RE: Bitmap alignment issues....

[Tom Evans]

I am using what was the current 2.6.11 development line with the 
-mm patch series (that's what includes reiser4).
 
I am not certain if any of those patches were "required" - I just
needed it for reiser4 support.

I also may have spoke too soon about the patch - I tried doing a full
kernel build and at some time during the process, it crashed and
then that kernel directory started behaving oddly.

I received a kernel oops - don't have it with me, and then accesses to
that directory would "hang".


I realize my setup may be complicated, I am using a beta kernel, on a
non-x86,
with reiser4 on a LVM partition, so anything in that chain can be
responsible.

I had been running that kernel for several days without issues (except for
the alignment faults).

Is there a recommended kernel to use with reiser4?

Thanks again,

...tom

-----Original Message-----
From: Alex Zarochentsev [mailto:zam@namesys.com] 
Sent: Thursday, February 10, 2005 3:40 AM
To: Thomas Evans
Cc: reiserfs-list@namesys.com
Subject: Re: Bitmap alignment issues....

On Thu, Feb 10, 2005 at 12:10:14AM -0500, Thomas Evans wrote:
> 
> Thanks for the patch the helped with the vast majority of the fixups.
> 
> Now there are just a trickle occurring in various places - would it be 
> helpful for me to track them down, or are there folks who are already
pursuing this?

which kernel are you using for alpha axp ?   did it require additional
patches to vanilla kernel?

> ...tom
> 
--
Alex.

Re: Bitmap alignment issues....

[Alex Zarochentsev]

On Thu, Feb 10, 2005 at 10:29:36AM -0500, Tom Evans wrote:
> 
> 
> I am using what was the current 2.6.11 development line with the 
> -mm patch series (that's what includes reiser4).
>  
> I am not certain if any of those patches were "required" - I just
> needed it for reiser4 support.
> 
> I also may have spoke too soon about the patch - I tried doing a full
> kernel build and at some time during the process, it crashed and
> then that kernel directory started behaving oddly.
> 
> I received a kernel oops - don't have it with me, and then accesses to
> that directory would "hang".
> 
> 
> I realize my setup may be complicated, I am using a beta kernel, on a
> non-x86,
> with reiser4 on a LVM partition, so anything in that chain can be
> responsible.
> 
> I had been running that kernel for several days without issues (except for
> the alignment faults).

do you see aligment faults in the bitmap code after my patch?

I fixed one place in the replace_extent (see the patch below), it  would be
fine to know which other places reiser4 has unaligned access faults in. 

------------------------------------
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2005/02/12 18:55:16+03:00 zam@crimson.namesys.com 
#   replace_extent: unaligned access fix.
# 
# plugin/item/extent.c
#   2005/02/12 18:55:10+03:00 zam@crimson.namesys.com +1 -1
#   replace_extent: unaligned access fix.
# 
diff -Nru a/plugin/item/extent.c b/plugin/item/extent.c
--- a/plugin/item/extent.c	Tue Feb 15 21:25:22 2005
+++ b/plugin/item/extent.c	Tue Feb 15 21:25:22 2005
@@ -135,7 +135,7 @@
 			assert("vs-987", znode_is_loaded(coord_after.node));
 			assert("vs-988", !memcmp(ext, &orig_ext, sizeof (*ext)));
 
-			*ext = *replace;
+			memcpy(ext, replace, sizeof(*ext));
 			znode_make_dirty(coord_after.node);
 
 			if (coord_after.node != orig_znode)

------------------------------------
> 
> Is there a recommended kernel to use with reiser4?

latest -mm kernel.

> Thanks again,
> 
> ...tom
> 

-- 
Alex.

RE: Bitmap alignment issues....

[Tom Evans]

>do you see aligment faults in the bitmap code after my patch?

The patch that was presented a few days ago caused corruption of the volume.
I used a "patch" I did that simply used the unaligned macros.

I will try the new patch this evening.

Thanks,

...tom

Re: Bitmap alignment issues....

[Alex Zarochentsev]

On Tue, Feb 15, 2005 at 04:37:29PM -0500, Tom Evans wrote:
> 
> >do you see aligment faults in the bitmap code after my patch?
> 
> The patch that was presented a few days ago caused corruption of the volume.
> I used a "patch" I did that simply used the unaligned macros.

did you apply the patch (see below) which i sent to amd64 guys?

--------------------------------------------------
diff -Nru a/plugin/space/bitmap.c b/plugin/space/bitmap.c
--- a/plugin/space/bitmap.c     Wed Feb 16 07:53:36 2005
+++ b/plugin/space/bitmap.c     Wed Feb 16 07:53:36 2005
@@ -165,7 +165,7 @@
 static int
 find_next_zero_bit_in_word(ulong_t word, int start_bit)
 {
-       ulong_t mask = 1 << start_bit;
+       ulong_t mask = 1UL << start_bit;
        int i = start_bit;

        while ((word & mask) != 0) {
@@ -235,7 +235,7 @@
        assert ("zam-965", start_bit < BITS_PER_LONG);
        assert ("zam-966", start_bit >= 0);

-       bit_mask = (1 << nr);
+       bit_mask = (1UL << nr);

        while (bit_mask != 0) {
                if (bit_mask & word)

--------------------------------------------------

> 
> I will try the new patch this evening.
> 
> Thanks,
> 
> ...tom
> 

-- 
Alex.