All of lore.kernel.org
 help / color / mirror / Atom feed
* [parisc-linux] ext3 perf patch#2
@ 2005-03-26 10:35 Grant Grundler
       [not found] ` <42454E7C.3050509@tiscali.be>
  2005-03-27  1:18 ` [parisc-linux] ext3 perf patch#4 Grant Grundler
  0 siblings, 2 replies; 6+ messages in thread
From: Grant Grundler @ 2005-03-26 10:35 UTC (permalink / raw)
  To: parisc-linux

James Bottomley in a private conversation suggested our
ext2_test_bit() and ext2_find_next_zero_bit() might not
be counting bits the same way in the bitmap.

ext2_test_bit() is quite straight forward looks right to me.
But my brain wasn't grokking ext2_find_next_zero_bit().
Cloning it from sparc64 was easy to try...
Since the first attempted worked, I also applli

The result was rsync was able to clone from one
ext3 (whole disk) to another ext3 using rsync
at 5-10MB/s (both disks on the same SCSI bus).
This is *alot* better than the previous experience
of < 1MB/s. Since my ext3 test filesystems hadn't aged (been used)
very much, bitmap search is not much overhead.

If someone else has a disposable, mature ext3 file system
they can test the following patch on, that would be great.

BTW, this patch assumes a 64-bit kernel and is certainly
not intended to be committed as is. This is just
an experiment so far.

Patch is against 2.6.11-pa2 (NOT 2.6.12 - sorry)

grant


Index: include/asm-parisc/bitops.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/bitops.h,v
retrieving revision 1.14
diff -u -p -r1.14 bitops.h
--- include/asm-parisc/bitops.h	18 Feb 2005 14:22:09 -0000	1.14
+++ include/asm-parisc/bitops.h	26 Mar 2005 09:58:58 -0000
@@ -466,49 +466,47 @@ static __inline__ int ext2_test_bit(int 
 	return (ADDR[nr >> 3] >> (nr & 7)) & 1;
 }
 
-/*
- * This implementation of ext2_find_{first,next}_zero_bit was stolen from
- * Linus' asm-alpha/bitops.h and modified for a big-endian machine.
- */
 
-#define ext2_find_first_zero_bit(addr, size) \
-        ext2_find_next_zero_bit((addr), (size), 0)
-
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr,
-	unsigned long size, unsigned long offset)
+static __inline__ unsigned long ext2_find_next_zero_bit(unsigned long *addr, unsigned long size, unsigned long offset)
 {
-	unsigned int *p = ((unsigned int *) addr) + (offset >> 5);
-	unsigned int result = offset & ~31UL;
-	unsigned int tmp;
+	unsigned long *p = addr + (offset >> 6);
+	unsigned long result = offset & ~63UL;
+	unsigned long tmp;
 
 	if (offset >= size)
 		return size;
 	size -= result;
-	offset &= 31UL;
-	if (offset) {
-		tmp = cpu_to_le32p(p++);
-		tmp |= ~0UL >> (32-offset);
-		if (size < 32)
+	offset &= 63UL;
+	if(offset) {
+		tmp = __swab64p((u64 *)p++);
+		tmp |= (~0UL >> (64-offset));
+		if(size < 64)
 			goto found_first;
-		if (tmp != ~0U)
+		if(~tmp)
 			goto found_middle;
-		size -= 32;
-		result += 32;
+		size -= 64;
+		result += 64;
 	}
-	while (size >= 32) {
-		if ((tmp = cpu_to_le32p(p++)) != ~0U)
-			goto found_middle;
-		result += 32;
-		size -= 32;
+	while(size & ~63) {
+		if(~(tmp = *(p++)))
+			goto found_middle_swap;
+		result += 64;
+		size -= 64;
 	}
-	if (!size)
+	if(!size)
 		return result;
-	tmp = cpu_to_le32p(p);
+	tmp = __swab64p((u64 *) p);
 found_first:
-	tmp |= ~0U << size;
+	tmp |= (~0UL << size);
+	if (tmp == ~0UL)        /* Are any bits zero? */
+		return result + size; /* Nope. */
 found_middle:
 	return result + ffz(tmp);
+
+found_middle_swap:
+	return result + ffz(__swab64((u64)tmp));
 }
+
 
 /* Bitmap functions for the minix filesystem.  */
 #define minix_test_and_set_bit(nr,addr) ext2_set_bit(nr,addr)
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [parisc-linux] ext3 perf patch#2
       [not found] ` <42454E7C.3050509@tiscali.be>
@ 2005-03-26 21:39   ` Grant Grundler
  0 siblings, 0 replies; 6+ messages in thread
From: Grant Grundler @ 2005-03-26 21:39 UTC (permalink / raw)
  To: Joel Soete; +Cc: parisc-linux

On Sat, Mar 26, 2005 at 11:58:52AM +0000, Joel Soete wrote:
> is the ~63 or 63UL?

63UL is better. But in this case it really doesn't matter.
The bitmaps aren't (yet) 512MB in size.

It should be ~(BIT_PER_LONG-1)....the next step to make the
whole thing usable for 32-bit.

grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [parisc-linux] ext3 perf patch#4
  2005-03-26 10:35 [parisc-linux] ext3 perf patch#2 Grant Grundler
       [not found] ` <42454E7C.3050509@tiscali.be>
@ 2005-03-27  1:18 ` Grant Grundler
  2005-03-27 13:49   ` Joel Soete
  1 sibling, 1 reply; 6+ messages in thread
From: Grant Grundler @ 2005-03-27  1:18 UTC (permalink / raw)
  To: parisc-linux

On Sat, Mar 26, 2005 at 03:35:40AM -0700, Grant Grundler wrote:
> James Bottomley in a private conversation suggested our
> ext2_test_bit() and ext2_find_next_zero_bit() might not
> be counting bits the same way in the bitmap.

James, kudos and thank you.

With appended patch, I'm getting reasonable ext3 performance.
Can someone please test this patch in a 32-bit kernel?

I'm comfortable this patch works for 64-bit since I've been building
kernels and running rsync across 3 disks with no problems.
I *think* the code is 32-bit clean as well, but haven't tested it.


Some minor additional, interesting things folks can do:
o Should this function should really be "static inline"?
  sparc64 puts it into arch/sparc64/lib :
  grundler <502>fgrep find_next_zero_le arch/sparc64/lib/find_bit.c 
  unsigned long find_next_zero_le_bit(unsigned long *addr, unsigned long size, unsigned long offset)

  How often is it called? More than three/four times would make
  it candidate for lib.

o ffz() is expanded inline. Restructure code so we don't
  need find_middle_swap() label. Ie do endian swap inside
  the conditional...but then the loop gets uglier and
  parisc can't use a conditional branch.
  Making this a normal function might make this item irrelevant.

o drop or optimize "if (tmp == ~0UL)" right before found_middle label.
  ffz() isn't that expensive compared to a mis-predicted branch.
  Figure out how many cycles ffz() costs, how many cycles a
  mispredicted branch costs, and then what the right tradeoff
  is in this case (including reduced code size).

Simple test results follow and then the patch.

riot:/mnt3# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/sdk3             16146848   1649084  13677540  11% /
/dev/sdk1               189387     97070     82539  55% /boot
/dev/sdb              17504036  14616368   1998508  88% /mnt3
/dev/sdu              17504036  14616368   1998508  88% /mnt2
/dev/sdv              17504036   5128996  11485880  31% /mnt

grundler@riot:/$ lsscsi
[0:0:0:0]    disk    SEAGATE  ST318203LC       HP02  /dev/sda
[0:0:1:0]    disk    SEAGATE  ST318203LC       HP02  /dev/sdb
...
[5:0:12:0]   disk    SEAGATE  ST318451LC       HP01  /dev/sdu
[5:0:13:0]   disk    SEAGATE  ST318451LC       HP01  /dev/sdv


riot:/mnt# time tar xjf linux-2.6.12-rc1-pa3.tar.bz2 

real    0m51.988s
user    0m44.175s
sys     0m7.912s

riot:/mnt# cd ../mnt2
riot:/mnt2# time tar xjf /mnt/linux-2.6.12-rc1-pa3.tar.bz2 

real    0m52.338s
user    0m44.232s
sys     0m7.912s
riot:/mnt2# cd /mnt3  
riot:/mnt3# time tar xjf /mnt/linux-2.6.12-rc1-pa3.tar.bz2 

real    0m54.755s
user    0m44.181s
sys     0m7.998s


iostat 10 second sample while running tar:

avg-cpu:  %user   %nice    %sys %iowait   %idle
          43.73    0.00   11.14    9.95   35.18

Device:            tps   Blk_read/s   Blk_wrtn/s   Blk_read   Blk_wrtn
sdb               0.00         0.00         0.00          0          0
sdu               0.00         0.00         0.00          0          0
sdv             130.87         2.40     11723.48         24     117352


thanks,
grant


Signed-off-by: Grant Grundler <grundler@parisc-linux.org>


Index: include/asm-parisc/bitops.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/bitops.h,v
retrieving revision 1.15
diff -u -p -r1.15 bitops.h
--- include/asm-parisc/bitops.h	24 Mar 2005 15:26:29 -0000	1.15
+++ include/asm-parisc/bitops.h	27 Mar 2005 01:09:41 -0000
@@ -466,52 +466,67 @@ static __inline__ int ext2_test_bit(int 
 	return (ADDR[nr >> 3] >> (nr & 7)) & 1;
 }
 
-/*
- * This implementation of ext2_find_{first,next}_zero_bit was stolen from
- * Linus' asm-alpha/bitops.h and modified for a big-endian machine.
- */
+/* include/linux/byteorder does not support "unsigned long" type */
+static inline unsigned long ext2_swabp(unsigned long * x)
+{
+#ifdef CONFIG_64BIT
+	return (unsigned long) __swab64p((u64 *) x);
+#else
+	return (unsigned long) __swab32p((u32 *) x);
+#endif
+}
 
-#define ext2_find_first_zero_bit(addr, size) \
-        ext2_find_next_zero_bit((addr), (size), 0)
+/* include/linux/byteorder doesn't support "unsigned long" type */
+static inline unsigned long ext2_swab(unsigned long y)
+{
+#ifdef CONFIG_64BIT
+	return (unsigned long) __swab64((u64) y);
+#else
+	return (unsigned long) __swab32((u32) y);
+#endif
+}
 
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr,
-	unsigned long size, unsigned long offset)
+static __inline__ unsigned long ext2_find_next_zero_bit(unsigned long *addr, unsigned long size, unsigned long offset)
 {
-	unsigned int *p = ((unsigned int *) addr) + (offset >> 5);
-	unsigned int result = offset & ~31UL;
-	unsigned int tmp;
+	unsigned long *p = addr + (offset >> SHIFT_PER_LONG);
+	unsigned long result = offset & ~(BITS_PER_LONG - 1);
+	unsigned long tmp;
 
 	if (offset >= size)
 		return size;
 	size -= result;
-	offset &= 31UL;
-	if (offset) {
-		tmp = cpu_to_le32p(p++);
-		tmp |= ~0UL >> (32-offset);
-		if (size < 32)
+	offset &= (BITS_PER_LONG - 1UL);
+	if(offset) {
+		tmp = ext2_swabp(p++);
+		tmp |= (~0UL >> (BITS_PER_LONG - offset));
+		if(size < BITS_PER_LONG)
 			goto found_first;
-		if (tmp != ~0U)
+		if(~tmp)
 			goto found_middle;
-		size -= 32;
-		result += 32;
+		size -= BITS_PER_LONG;
+		result += BITS_PER_LONG;
 	}
-	while (size >= 32) {
-		if ((tmp = *p++) != ~0U)
+
+	while(size & ~(BITS_PER_LONG - 1)) {
+		if(~(tmp = *(p++)))
 			goto found_middle_swap;
-		result += 32;
-		size -= 32;
+		result += BITS_PER_LONG;
+		size -= BITS_PER_LONG;
 	}
-	if (!size)
+	if(!size)
 		return result;
-	tmp = cpu_to_le32p(p);
+	tmp = ext2_swabp(p);
 found_first:
-	tmp |= ~0U << size;
+	tmp |= ~0UL << size;
+	if (tmp == ~0UL)	/* Are any bits zero? */
+		return result + size; /* Nope. Skip ffz */
 found_middle:
 	return result + ffz(tmp);
+
 found_middle_swap:
-	tmp = cpu_to_le32(tmp);
-	return result + ffz(tmp);
+	return result + ffz(ext2_swab(tmp));
 }
+
 
 /* Bitmap functions for the minix filesystem.  */
 #define minix_test_and_set_bit(nr,addr) ext2_set_bit(nr,addr)
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [parisc-linux] ext3 perf patch#4
  2005-03-27  1:18 ` [parisc-linux] ext3 perf patch#4 Grant Grundler
@ 2005-03-27 13:49   ` Joel Soete
  2005-03-28  3:48     ` Grant Grundler
  0 siblings, 1 reply; 6+ messages in thread
From: Joel Soete @ 2005-03-27 13:49 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux



Grant Grundler wrote:
> On Sat, Mar 26, 2005 at 03:35:40AM -0700, Grant Grundler wrote:
> 
>>James Bottomley in a private conversation suggested our
>>ext2_test_bit() and ext2_find_next_zero_bit() might not
>>be counting bits the same way in the bitmap.
> 
> 
> James, kudos and thank you.
> 
> With appended patch, I'm getting reasonable ext3 performance.
> Can someone please test this patch in a 32-bit kernel?
> 
Obviously ;-)

> I'm comfortable this patch works for 64-bit since I've been building
> kernels and running rsync across 3 disks with no problems.
> I *think* the code is 32-bit clean as well, but haven't tested it.
> 
on my c110 no pb to boot :-)
some rsync test (on non mirored slices on 2 different disk but on the same scsi ctrlr ncr53c720):
# time rsync -a /Debian-apt/SRC/linux-2.6.12-rc1-pa4-050327 /Sources/CVS-tst

real    3m34.512s
user    1m0.730s
sys     1m6.440s

# du -sk /Debian-apt/SRC/linux-2.6.12-rc1-pa4-050327
256616  /Debian-apt/SRC/linux-2.6.12-rc1-pa4-050327

...

> Simple test results follow and then the patch.
> 
# bdf
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/md2               1692128   1194620    411552  75% /
tmpfs                   256300         0    256300   0% /dev/shm
/dev/md0                123699    112645      4667  97% /boot
/dev/md3                247407     82537    152096  36% /var
/dev/md4                123635      4129    113122   4% /tmp
/dev/md5                123635     15749    101502  14% /home
/dev/sdb9              1513656    968940    529340  65% /Debian-apt
/dev/sda9              1513656   1011696    425068  71% /Sources
/dev/sdc5              1692220    913028    693232  57% /chroot
/dev/sdc3               123767      8728    108649   8% /chroot/boot
/dev/sdc6               247511    107636    127096  46% /chroot/var
/dev/sdc7               123735      4130    113216   4% /chroot/tmp
/dev/sdc8               123735      4144    113202   4% /chroot/home
/dev/sdc9              1513656    473444   1024836  32% /chroot/Develop

...

> grundler@riot:/$ lsscsi
Not yet lsscsi

> 
> riot:/mnt# time tar xjf linux-2.6.12-rc1-pa3.tar.bz2 
> 
I prefer simply tar and so:
# cd /Debian-apt/SRC
# time tar -cslpf /chroot/Develop/linux-2.6.12-rc1-pa4.tar2 linux-2.6.12-rc1-pa4-050327

real    0m38.487s
user    0m3.690s
sys     0m18.650s

a sample iostat too :-)
avg-cpu:  %user   %nice    %sys %iowait   %idle
           15.70    0.00   47.00   37.30    0.00

Device:            tps   Blk_read/s   Blk_wrtn/s   Blk_read   Blk_wrtn
sda               2.50         0.00        19.60          0        196
sdb             297.30      5906.40        89.20      59064        892
sdc              15.10         0.00      7712.80          0      77128
md0               0.00         0.00         0.00          0          0
md5               0.00         0.00         0.00          0          0
md4               0.00         0.00         0.00          0          0
md3               0.20         0.00         0.40          0          4
md2               2.00         0.00        16.00          0        160
md1               0.00         0.00         0.00          0          0


# cd /chroot/Develop

# time tar -xslpf linux-2.6.12-rc1-pa4.tar2

real    1m32.901s
user    0m4.880s
sys     0m30.000s

another iostat sample:
avg-cpu:  %user   %nice    %sys %iowait   %idle
           13.19    0.00   39.66   47.15    0.00

Device:            tps   Blk_read/s   Blk_wrtn/s   Blk_read   Blk_wrtn
sda               0.00         0.00         0.00          0          0
sdb               0.00         0.00         0.00          0          0
sdc             173.33      4542.66      6186.61      45472      61928
md0               0.00         0.00         0.00          0          0
md5               0.00         0.00         0.00          0          0
md4               0.00         0.00         0.00          0          0
md3               0.00         0.00         0.00          0          0
md2               0.00         0.00         0.00          0          0
md1               0.00         0.00         0.00          0          0


still have to test on a quicker system as the b2k if that could solve the bug I reproduce on 32bit kernel
(cf patch log message:"... I suspect the real problem is ffz() wants
	an unsigned long and was getting garbage in the top half of the
	unsigned int. Not confirmed but that's what I suspect.)

Thanks,
	Joel
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [parisc-linux] ext3 perf patch#4
  2005-03-27 13:49   ` Joel Soete
@ 2005-03-28  3:48     ` Grant Grundler
  2005-03-28 12:51       ` Joel Soete
  0 siblings, 1 reply; 6+ messages in thread
From: Grant Grundler @ 2005-03-28  3:48 UTC (permalink / raw)
  To: Joel Soete; +Cc: parisc-linux

On Sun, Mar 27, 2005 at 01:49:06PM +0000, Joel Soete wrote:
...
> # time rsync -a /Debian-apt/SRC/linux-2.6.12-rc1-pa4-050327 /Sources/CVS-tst
> 
> real    3m34.512s
> user    1m0.730s
> sys     1m6.440s

Joel,
thanks for the data!

But I don't know what it means.
Was the performance bad for that box before?

> I prefer simply tar and so:
> # cd /Debian-apt/SRC
> # time tar -cslpf /chroot/Develop/linux-2.6.12-rc1-pa4.tar2 
> linux-2.6.12-rc1-pa4-050327
> 
> real    0m38.487s
> user    0m3.690s
> sys     0m18.650s
> 
> a sample iostat too :-)
> avg-cpu:  %user   %nice    %sys %iowait   %idle
>           15.70    0.00   47.00   37.30    0.00
> 
> Device:            tps   Blk_read/s   Blk_wrtn/s   Blk_read   Blk_wrtn
> sda               2.50         0.00        19.60          0        196
> sdb             297.30      5906.40        89.20      59064        892
> sdc              15.10         0.00      7712.80          0      77128
...

That looks fine.

> still have to test on a quicker system as the b2k if that could solve the 
> bug I reproduce on 32bit kernel
> (cf patch log message:"... I suspect the real problem is ffz() wants
> 	an unsigned long and was getting garbage in the top half of the
> 	unsigned int. Not confirmed but that's what I suspect.)

I probably should have posted that speculation to the mailing list
instead of in the commit log...I was quite tired at that point.

That could only be true if ext3 perf issue only shows up in 64-bit kernels.
I don't recall if anyone posted perf results for 32-bit kernels.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [parisc-linux] ext3 perf patch#4
  2005-03-28  3:48     ` Grant Grundler
@ 2005-03-28 12:51       ` Joel Soete
  0 siblings, 0 replies; 6+ messages in thread
From: Joel Soete @ 2005-03-28 12:51 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux



Grant Grundler wrote:
> On Sun, Mar 27, 2005 at 01:49:06PM +0000, Joel Soete wrote:
> ...
> 
>># time rsync -a /Debian-apt/SRC/linux-2.6.12-rc1-pa4-050327 /Sources/CVS-tst
>>
>>real    3m34.512s
>>user    1m0.730s
>>sys     1m6.440s
> 
> 
> Joel,
> thanks for the data!
> 
> But I don't know what it means.
> Was the performance bad for that box before?
> 
:-)
No just looks like as before:
# uname -a
Linux hpalin 2.6.12-rc1-pa1-c110 #2 Sat Mar 19 23:36:05 CET 2005 parisc GNU/Linux
# time rsync -a /Debian-apt/SRC/linux-2.6.12-rc1-pa4-050327 /Sources/CVS-tst

real    3m44.365s
user    1m0.350s
sys     1m11.120s


> 
>>I prefer simply tar and so:
>># cd /Debian-apt/SRC
>># time tar -cslpf /chroot/Develop/linux-2.6.12-rc1-pa4.tar2 
>>linux-2.6.12-rc1-pa4-050327
>>
>>real    0m38.487s
>>user    0m3.690s
>>sys     0m18.650s
>>
>>a sample iostat too :-)
>>avg-cpu:  %user   %nice    %sys %iowait   %idle
>>          15.70    0.00   47.00   37.30    0.00
>>
>>Device:            tps   Blk_read/s   Blk_wrtn/s   Blk_read   Blk_wrtn
>>sda               2.50         0.00        19.60          0        196
>>sdb             297.30      5906.40        89.20      59064        892
>>sdc              15.10         0.00      7712.80          0      77128
> 
> ...
> 
> That looks fine.
> 
Yes, sorry for confusion (I would just show that it works and nothing was broken)

...
> 
> I probably should have posted that speculation to the mailing list
> instead of in the commit log...I was quite tired at that point.
> 
Don't wory I would have to be disturbe or confused by the song of a bird in the garden ?-)
(the 32bit bug has obviously nothing to do with that)

> That could only be true if ext3 perf issue only shows up in 64-bit kernels.
> I don't recall if anyone posted perf results for 32-bit kernels.
> 
TBH, the 2 64bit system I have the oportunity to test (b2k up, n4k smp) are so confortable (well n4k ask many time for self test on 
reboot) from speed point of view that I never noticed ext[23] lack of perf ;-)

Thanks,
	Joel
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2005-03-28 12:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-26 10:35 [parisc-linux] ext3 perf patch#2 Grant Grundler
     [not found] ` <42454E7C.3050509@tiscali.be>
2005-03-26 21:39   ` Grant Grundler
2005-03-27  1:18 ` [parisc-linux] ext3 perf patch#4 Grant Grundler
2005-03-27 13:49   ` Joel Soete
2005-03-28  3:48     ` Grant Grundler
2005-03-28 12:51       ` Joel Soete

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.