[PATCH] Use do_div() instead of native 64-bit division in btrfs_ordered_sum_size()

[PATCH] Use do_div() instead of native 64-bit division in btrfs_ordered_sum_size()

[David Woodhouse]

Prevents the compiler emitting calls to __udivdi3() from libgcc on some
platforms:
WARNING: "__udivdi3" [/shiny/git/btrfs-kernel-unstable/btrfs.ko] undefi=
ned!

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
Untested but ObviouslyCorrect=E2=84=A2. Now that I can load the module,=
 I hit a
BUG() immediately -- but I don't think it's caused by this patch. qv.

diff --git a/ordered-data.h b/ordered-data.h
index 1794efd..1abe5f5 100644
--- a/ordered-data.h
+++ b/ordered-data.h
@@ -97,9 +97,12 @@ struct btrfs_ordered_extent {
  */
 static inline int btrfs_ordered_sum_size(struct btrfs_root *root, u64 =
bytes)
 {
-	unsigned long num_sectors =3D (bytes + root->sectorsize - 1) /
-		root->sectorsize;
-	num_sectors++;
+	unsigned long num_sectors;
+
+	bytes +=3D root->sectorsize - 1;
+	do_div(bytes, root->sectorsize);
+	num_sectors =3D bytes + 1;
+
 	return sizeof(struct btrfs_ordered_sum) +
 		num_sectors * sizeof(struct btrfs_sector_sum);
 }

--=20
David Woodhouse                            Open Source Technology Centr=
e
David.Woodhouse@intel.com                              Intel Corporatio=
n


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

kernel BUG at extent_map.c:275!

[David Woodhouse]

On Mon, 2008-07-21 at 23:32 -0400, David Woodhouse wrote:
> Untested but ObviouslyCorrect=E2=84=A2. Now that I can load the modul=
e, I hit a
> BUG() immediately -- but I don't think it's caused by this patch. qv.

Current progs-unstable and kernel-unstable. I created a 512MiB file and
ran mkfs.btrfs on it, on the current Fedora kernel:

shinybook /shiny/git/btrfs-progs-unstable $ dd if=3D/dev/zero of=3D/hom=
e/dwmw2/btrfs.test bs=3D$((1024*1024)) count=3D512
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 37.6313 s, 14.3 MB/s
shinybook /shiny/git/btrfs-progs-unstable $ ./mkfs.btrfs /home/dwmw2/bt=
rfs.test fs created label (null) on /home/dwmw2/btrfs.test
	nodesize 4096 leafsize 4096 sectorsize 4096 size 512.00MB
shinybook /shiny/git/btrfs-progs-unstable $ sudo mount -tbtrfs -oloop /=
home/dwmw2/btrfs.test /mnt/spare
shinybook /shiny/git/btrfs-progs-unstable $ dmesg | tail -61=20
loop: module loaded
device fsid 1119bbc059164661-acafbb6be44e5929 devid 1 transid 12 /dev/l=
oop0
------------[ cut here ]------------
kernel BUG at /shiny/git/btrfs-kernel-unstable/extent_map.c:275!
Oops: Exception in kernel mode, sig: 5 [#1]
PowerMac
Modules linked in: loop btrfs libcrc32c b43legacy mac80211 rndis_wlan r=
ndis_host cdc_ether usbnet mii cdc_acm airport bridge bnep orinoco_cs o=
rinoco hermes hidp udf radeon drm rfkill_input hci_usb rfcomm l2cap blu=
etooth autofs4 sunrpc ipv6 dm_mirror dm_mod therm_adt746x arc4 ecb cryp=
to_blkcipher snd_aoa_i2sbus rfkill cfg80211 input_polldev snd_powermac =
snd_seq_dummy pmac_zilog snd_seq_oss snd_seq_midi_event snd_seq ide_cd_=
mod cdrom snd_seq_device sungem snd_pcm_oss snd_mixer_oss sungem_phy sn=
d_pcm firewire_ohci firewire_core crc_itu_t snd_timer snd_page_alloc sn=
d soundcore snd_aoa_soundbus ssb ext3 jbd mbcache uhci_hcd ohci_hcd ehc=
i_hcd [last unloaded: b43legacy]
NIP: f29b6b2c LR: f29cc9f8 CTR: c0090778
REGS: c671dbf0 TRAP: 0700   Tainted: P          (2.6.25.10-86.fc9.ppc)
MSR: 00029032 <EE,ME,IR,DR>  CR: 24044448  XER: 00000000
TASK =3D c65736e0[25115] 'mount' THREAD: c671c000
GPR00: 00000001 c671dca0 c65736e0 e7b9e108 00400000 00000000 00000000 0=
0000000=20
GPR08: 00000001 00000000 00004000 00004000 00000000 20029a70 00000000 b=
fcac591=20
GPR16: 20024880 20024870 bfcac56d e7b9e108 ec94b3c0 ec908400 e7b9f588 e=
d473200=20
GPR24: 00000000 e7b9e108 e1109ab8 00000000 00000000 00000001 00000000 0=
0000000=20
NIP [f29b6b2c] lookup_extent_mapping+0x48/0x350 [btrfs]
LR [f29cc9f8] read_one_chunk+0xa0/0x2f0 [btrfs]
Call Trace:
[c671dca0] [00001000] 0x1000 (unreliable)
[c671dcd0] [f29cc9f8] read_one_chunk+0xa0/0x2f0 [btrfs]
[c671dd20] [f29cd180] btrfs_read_sys_array+0x180/0x200 [btrfs]
[c671dd70] [f29a7b40] open_ctree+0x620/0x928 [btrfs]
[c671ddb0] [f2982334] btrfs_get_sb+0x224/0x3dc [btrfs]
[c671de20] [c00b035c] vfs_kern_mount+0xa0/0x138
[c671de50] [c00b0444] do_kern_mount+0x40/0xf4
[c671de70] [c00c7f38] do_new_mount+0x6c/0xb8
[c671de90] [c00c8f1c] do_mount+0x1c8/0x1f4
[c671df10] [c00c8fd8] sys_mount+0x90/0xd8
[c671df40] [c00139dc] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0x1fea3c54
    LR =3D 0x20004fa8
Instruction dump:
7f87f114 90010034 7c791b78 7f85e040 419d0014 7f85e000 40be0014 7f86e840=
=20
409d000c 3b80ffff 3ba0ffff 38000001 <0f000000> 80790004 2f830000 419e00=
6c=20
---[ end trace 83a08bddd47794a5 ]---
BUG: sleeping function called from invalid context at kernel/rwsem.c:21
in_atomic():0, irqs_disabled():1
Call Trace:
[c671da10] [c0008ddc] show_stack+0x4c/0x180 (unreliable)
[c671da60] [c00346d4] __might_sleep+0xc8/0xd8
[c671da70] [c02ecc48] down_read+0x24/0x5c
[c671da80] [c0069934] acct_collect+0x38/0x15c
[c671daa0] [c003c9a4] do_exit+0x1c4/0x5a4
[c671dad0] [c0011e88] kernel_bad_stack+0x0/0x4c
[c671daf0] [c0012040] _exception+0x58/0x154
[c671dbe0] [c001403c] ret_from_except_full+0x0/0x4c
--- Exception: 700 at lookup_extent_mapping+0x48/0x350 [btrfs]
    LR =3D read_one_chunk+0xa0/0x2f0 [btrfs]
[c671dca0] [00001000] 0x1000 (unreliable)
[c671dcd0] [f29cc9f8] read_one_chunk+0xa0/0x2f0 [btrfs]
[c671dd20] [f29cd180] btrfs_read_sys_array+0x180/0x200 [btrfs]
[c671dd70] [f29a7b40] open_ctree+0x620/0x928 [btrfs]
[c671ddb0] [f2982334] btrfs_get_sb+0x224/0x3dc [btrfs]
[c671de20] [c00b035c] vfs_kern_mount+0xa0/0x138
[c671de50] [c00b0444] do_kern_mount+0x40/0xf4
[c671de70] [c00c7f38] do_new_mount+0x6c/0xb8
[c671de90] [c00c8f1c] do_mount+0x1c8/0x1f4
[c671df10] [c00c8fd8] sys_mount+0x90/0xd8
[c671df40] [c00139dc] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0x1fea3c54
    LR =3D 0x20004fa8
shinybook /shiny/git/btrfs-kernel-unstable #=20


--=20
David Woodhouse                            Open Source Technology Centr=
e
David.Woodhouse@intel.com                              Intel Corporatio=
n


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel BUG at extent_map.c:275!

[Chris Mason]

On Mon, 2008-07-21 at 23:36 -0400, David Woodhouse wrote:
> On Mon, 2008-07-21 at 23:32 -0400, David Woodhouse wrote:
> > Untested but ObviouslyCorrect=E2=84=A2. Now that I can load the mod=
ule, I hit a
> > BUG() immediately -- but I don't think it's caused by this patch. q=
v.
>=20
> Current progs-unstable and kernel-unstable. I created a 512MiB file a=
nd
> ran mkfs.btrfs on it, on the current Fedora kernel:
>=20

What kind of box is this?  The current code should be fine on big
endian, but that hasn't been tested recently.

-chris

> shinybook /shiny/git/btrfs-progs-unstable $ dd if=3D/dev/zero of=3D/h=
ome/dwmw2/btrfs.test bs=3D$((1024*1024)) count=3D512
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 37.6313 s, 14.3 MB/s
> shinybook /shiny/git/btrfs-progs-unstable $ ./mkfs.btrfs /home/dwmw2/=
btrfs.test fs created label (null) on /home/dwmw2/btrfs.test
> 	nodesize 4096 leafsize 4096 sectorsize 4096 size 512.00MB
> shinybook /shiny/git/btrfs-progs-unstable $ sudo mount -tbtrfs -oloop=
 /home/dwmw2/btrfs.test /mnt/spare
> shinybook /shiny/git/btrfs-progs-unstable $ dmesg | tail -61=20
> loop: module loaded
> device fsid 1119bbc059164661-acafbb6be44e5929 devid 1 transid 12 /dev=
/loop0
> ------------[ cut here ]------------
> kernel BUG at /shiny/git/btrfs-kernel-unstable/extent_map.c:275!
> Oops: Exception in kernel mode, sig: 5 [#1]
> PowerMac
> Modules linked in: loop btrfs libcrc32c b43legacy mac80211 rndis_wlan=
 rndis_host cdc_ether usbnet mii cdc_acm airport bridge bnep orinoco_cs=
 orinoco hermes hidp udf radeon drm rfkill_input hci_usb rfcomm l2cap b=
luetooth autofs4 sunrpc ipv6 dm_mirror dm_mod therm_adt746x arc4 ecb cr=
ypto_blkcipher snd_aoa_i2sbus rfkill cfg80211 input_polldev snd_powerma=
c snd_seq_dummy pmac_zilog snd_seq_oss snd_seq_midi_event snd_seq ide_c=
d_mod cdrom snd_seq_device sungem snd_pcm_oss snd_mixer_oss sungem_phy =
snd_pcm firewire_ohci firewire_core crc_itu_t snd_timer snd_page_alloc =
snd soundcore snd_aoa_soundbus ssb ext3 jbd mbcache uhci_hcd ohci_hcd e=
hci_hcd [last unloaded: b43legacy]
> NIP: f29b6b2c LR: f29cc9f8 CTR: c0090778
> REGS: c671dbf0 TRAP: 0700   Tainted: P          (2.6.25.10-86.fc9.ppc=
)
> MSR: 00029032 <EE,ME,IR,DR>  CR: 24044448  XER: 00000000
> TASK =3D c65736e0[25115] 'mount' THREAD: c671c000
> GPR00: 00000001 c671dca0 c65736e0 e7b9e108 00400000 00000000 00000000=
 00000000=20
> GPR08: 00000001 00000000 00004000 00004000 00000000 20029a70 00000000=
 bfcac591=20
> GPR16: 20024880 20024870 bfcac56d e7b9e108 ec94b3c0 ec908400 e7b9f588=
 ed473200=20
> GPR24: 00000000 e7b9e108 e1109ab8 00000000 00000000 00000001 00000000=
 00000000=20
> NIP [f29b6b2c] lookup_extent_mapping+0x48/0x350 [btrfs]
> LR [f29cc9f8] read_one_chunk+0xa0/0x2f0 [btrfs]
> Call Trace:
> [c671dca0] [00001000] 0x1000 (unreliable)
> [c671dcd0] [f29cc9f8] read_one_chunk+0xa0/0x2f0 [btrfs]
> [c671dd20] [f29cd180] btrfs_read_sys_array+0x180/0x200 [btrfs]
> [c671dd70] [f29a7b40] open_ctree+0x620/0x928 [btrfs]
> [c671ddb0] [f2982334] btrfs_get_sb+0x224/0x3dc [btrfs]
> [c671de20] [c00b035c] vfs_kern_mount+0xa0/0x138
> [c671de50] [c00b0444] do_kern_mount+0x40/0xf4
> [c671de70] [c00c7f38] do_new_mount+0x6c/0xb8
> [c671de90] [c00c8f1c] do_mount+0x1c8/0x1f4
> [c671df10] [c00c8fd8] sys_mount+0x90/0xd8
> [c671df40] [c00139dc] ret_from_syscall+0x0/0x38
> --- Exception: c01 at 0x1fea3c54
>     LR =3D 0x20004fa8
> Instruction dump:
> 7f87f114 90010034 7c791b78 7f85e040 419d0014 7f85e000 40be0014 7f86e8=
40=20
> 409d000c 3b80ffff 3ba0ffff 38000001 <0f000000> 80790004 2f830000 419e=
006c=20
> ---[ end trace 83a08bddd47794a5 ]---
> BUG: sleeping function called from invalid context at kernel/rwsem.c:=
21
> in_atomic():0, irqs_disabled():1
> Call Trace:
> [c671da10] [c0008ddc] show_stack+0x4c/0x180 (unreliable)
> [c671da60] [c00346d4] __might_sleep+0xc8/0xd8
> [c671da70] [c02ecc48] down_read+0x24/0x5c
> [c671da80] [c0069934] acct_collect+0x38/0x15c
> [c671daa0] [c003c9a4] do_exit+0x1c4/0x5a4
> [c671dad0] [c0011e88] kernel_bad_stack+0x0/0x4c
> [c671daf0] [c0012040] _exception+0x58/0x154
> [c671dbe0] [c001403c] ret_from_except_full+0x0/0x4c
> --- Exception: 700 at lookup_extent_mapping+0x48/0x350 [btrfs]
>     LR =3D read_one_chunk+0xa0/0x2f0 [btrfs]
> [c671dca0] [00001000] 0x1000 (unreliable)
> [c671dcd0] [f29cc9f8] read_one_chunk+0xa0/0x2f0 [btrfs]
> [c671dd20] [f29cd180] btrfs_read_sys_array+0x180/0x200 [btrfs]
> [c671dd70] [f29a7b40] open_ctree+0x620/0x928 [btrfs]
> [c671ddb0] [f2982334] btrfs_get_sb+0x224/0x3dc [btrfs]
> [c671de20] [c00b035c] vfs_kern_mount+0xa0/0x138
> [c671de50] [c00b0444] do_kern_mount+0x40/0xf4
> [c671de70] [c00c7f38] do_new_mount+0x6c/0xb8
> [c671de90] [c00c8f1c] do_mount+0x1c8/0x1f4
> [c671df10] [c00c8fd8] sys_mount+0x90/0xd8
> [c671df40] [c00139dc] ret_from_syscall+0x0/0x38
> --- Exception: c01 at 0x1fea3c54
>     LR =3D 0x20004fa8
> shinybook /shiny/git/btrfs-kernel-unstable #=20
>=20
>=20

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel BUG at extent_map.c:275!

[David Woodhouse]

On Tue, 2008-07-22 at 06:21 -0400, Chris Mason wrote:
> What kind of box is this?  The current code should be fine on big
> endian, but that hasn't been tested recently.

It's a PowerBook (ppc32).

The bug is a BUG_ON(spin_trylock(&tree->lock)) in
lookup_extent_mapping() -- I didn't think endianness was a likely factor
in that.

Being uniprocessor might be though -- don't we hard-code spin_trylock()
to _always_ succeed on UP, unless spinlock debugging is enabled?

I think that test needs to happen only if spinlocks aren't no-ops. Or
have you moved past the point where you need it now, so can we just
remove it?

T'would be nice if lockdep or sparse could handle this for us -- we
already have annotations which indicate that a function 'acquires' or
'releases' locks. I wonder if we could also do 'requires'?

-- 
dwmw2

Re: kernel BUG at extent_map.c:275!

[Chris Mason]

On Tue, 2008-07-22 at 10:35 -0400, David Woodhouse wrote:
> On Tue, 2008-07-22 at 06:21 -0400, Chris Mason wrote:
> > What kind of box is this?  The current code should be fine on big
> > endian, but that hasn't been tested recently.
> 
> It's a PowerBook (ppc32).
> 
> The bug is a BUG_ON(spin_trylock(&tree->lock)) in
> lookup_extent_mapping() -- I didn't think endianness was a likely factor
> in that.

I think the caller is doing this:

        spin_lock(&map_tree->map_tree.lock);
        em = lookup_extent_mapping(&map_tree->map_tree, logical, 1);
        spin_unlock(&map_tree->map_tree.lock);

> Being uniprocessor might be though -- don't we hard-code spin_trylock()
> to _always_ succeed on UP, unless spinlock debugging is enabled?
> 
> I think that test needs to happen only if spinlocks aren't no-ops. Or
> have you moved past the point where you need it now, so can we just
> remove it?
> 

Well, the test is there to make sure the caller is doing the right
thing.  Before we remove it, I'd like to understand why it is failing.

-chris

Re: kernel BUG at extent_map.c:275!

[David Woodhouse]

On Tue, 2008-07-22 at 13:03 -0400, Chris Mason wrote:
> Well, the test is there to make sure the caller is doing the right
> thing.  Before we remove it, I'd like to understand why it is failing.

Because this is a uniprocessor kernel. So spin_lock() and spin_unlock()
both do absolutely nothing, and spin_trylock() _always_ returns 1.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: kernel BUG at extent_map.c:275!

[Zach Brown]

David Woodhouse wrote:
> On Tue, 2008-07-22 at 13:03 -0400, Chris Mason wrote:
>> Well, the test is there to make sure the caller is doing the right
>> thing.  Before we remove it, I'd like to understand why it is failing.
> 
> Because this is a uniprocessor kernel. So spin_lock() and spin_unlock()
> both do absolutely nothing, and spin_trylock() _always_ returns 1.

How about using assert_spin_locked()?

- z

Re: kernel BUG at extent_map.c:275!

[David Woodhouse]

On Tue, 2008-07-22 at 10:42 -0700, Zach Brown wrote:
> David Woodhouse wrote:
> > On Tue, 2008-07-22 at 13:03 -0400, Chris Mason wrote:
> >> Well, the test is there to make sure the caller is doing the right
> >> thing.  Before we remove it, I'd like to understand why it is failing.
> > 
> > Because this is a uniprocessor kernel. So spin_lock() and spin_unlock()
> > both do absolutely nothing, and spin_trylock() _always_ returns 1.
> 
> How about using assert_spin_locked()?

Yeah, that should work if we still need these checks.

diff --git a/extent_map.c b/extent_map.c
index 71b1ac1..c68abd8 100644
--- a/extent_map.c
+++ b/extent_map.c
@@ -209,7 +209,7 @@ int add_extent_mapping(struct extent_map_tree *tree,
 	struct extent_map *merge = NULL;
 	struct rb_node *rb;
 
-	BUG_ON(spin_trylock(&tree->lock));
+	assert_spin_locked(&tree->lock);
 	rb = tree_insert(&tree->map, em->start, &em->rb_node);
 	if (rb) {
 		ret = -EEXIST;
@@ -272,7 +272,7 @@ struct extent_map *lookup_extent_mapping(struct extent_map_tree *tree,
 	struct rb_node *next = NULL;
 	u64 end = range_end(start, len);
 
-	BUG_ON(spin_trylock(&tree->lock));
+	assert_spin_locked(&tree->lock);
 	em = tree->last;
 	if (em && end > em->start && start < extent_map_end(em))
 		goto found;
@@ -324,7 +324,7 @@ int remove_extent_mapping(struct extent_map_tree *tree, struct extent_map *em)
 	int ret = 0;
 
 	WARN_ON(test_bit(EXTENT_FLAG_PINNED, &em->flags));
-	BUG_ON(spin_trylock(&tree->lock));
+	assert_spin_locked(&tree->lock);
 	rb_erase(&em->rb_node, &tree->map);
 	em->in_tree = 0;
 	if (tree->last == em)

-- 
dwmw2

Re: kernel BUG at extent_map.c:275!

[Chris Mason]

On Tue, 2008-07-22 at 14:04 -0400, David Woodhouse wrote:
> On Tue, 2008-07-22 at 10:42 -0700, Zach Brown wrote:
> > David Woodhouse wrote:
> > > On Tue, 2008-07-22 at 13:03 -0400, Chris Mason wrote:
> > >> Well, the test is there to make sure the caller is doing the right
> > >> thing.  Before we remove it, I'd like to understand why it is failing.
> > > 
> > > Because this is a uniprocessor kernel. So spin_lock() and spin_unlock()
> > > both do absolutely nothing, and spin_trylock() _always_ returns 1.
> > 
> > How about using assert_spin_locked()?
> 
> Yeah, that should work if we still need these checks.
> 

This one is applied now and pushed out.

-chris

Re: kernel BUG at extent_map.c:275!

[Chris Mason]

On Tue, 2008-07-22 at 13:41 -0400, David Woodhouse wrote:
> On Tue, 2008-07-22 at 13:03 -0400, Chris Mason wrote:
> > Well, the test is there to make sure the caller is doing the right
> > thing.  Before we remove it, I'd like to understand why it is failing.
> 
> Because this is a uniprocessor kernel. So spin_lock() and spin_unlock()
> both do absolutely nothing, and spin_trylock() _always_ returns 1.
> 

Duh, sorry that's what you were saying all along.  For some reason I saw
UP machine on SMP kernel in your email.

Thanks for the patch, I'll pull it in.

-chris

[PATCH] Remove BUG_ON(spin_trylock()) checks which have false positives on UP

[David Woodhouse]

On uniprocessor kernels without spinlock debugging, spinlock operations
are all no-ops and spin_trylock() will always succeed.

These BUG_ON() sanity checks are effectively an unconditional BUG() in
that case.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

diff --git a/extent_map.c b/extent_map.c
index 71b1ac1..6a72961 100644
--- a/extent_map.c
+++ b/extent_map.c
@@ -209,7 +209,6 @@ int add_extent_mapping(struct extent_map_tree *tree,
 	struct extent_map *merge = NULL;
 	struct rb_node *rb;
 
-	BUG_ON(spin_trylock(&tree->lock));
 	rb = tree_insert(&tree->map, em->start, &em->rb_node);
 	if (rb) {
 		ret = -EEXIST;
@@ -272,7 +271,6 @@ struct extent_map *lookup_extent_mapping(struct extent_map_tree *tree,
 	struct rb_node *next = NULL;
 	u64 end = range_end(start, len);
 
-	BUG_ON(spin_trylock(&tree->lock));
 	em = tree->last;
 	if (em && end > em->start && start < extent_map_end(em))
 		goto found;
@@ -324,7 +322,6 @@ int remove_extent_mapping(struct extent_map_tree *tree, struct extent_map *em)
 	int ret = 0;
 
 	WARN_ON(test_bit(EXTENT_FLAG_PINNED, &em->flags));
-	BUG_ON(spin_trylock(&tree->lock));
 	rb_erase(&em->rb_node, &tree->map);
 	em->in_tree = 0;
 	if (tree->last == em)


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH] Use do_div() instead of native 64-bit division in btrfs_ordered_sum_size()

[Christoph Hellwig]

Chris, can you please put this patch in?  Without it btrfs can't be
loaded on 32bit platforms.

On Mon, Jul 21, 2008 at 11:32:08PM -0400, David Woodhouse wrote:
> Prevents the compiler emitting calls to __udivdi3() from libgcc on some
> platforms:
> WARNING: "__udivdi3" [/shiny/git/btrfs-kernel-unstable/btrfs.ko] undefined!
> 
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
> Untested but ObviouslyCorrect???. Now that I can load the module, I hit a
> BUG() immediately -- but I don't think it's caused by this patch. qv.
> 
> diff --git a/ordered-data.h b/ordered-data.h
> index 1794efd..1abe5f5 100644
> --- a/ordered-data.h
> +++ b/ordered-data.h
> @@ -97,9 +97,12 @@ struct btrfs_ordered_extent {
>   */
>  static inline int btrfs_ordered_sum_size(struct btrfs_root *root, u64 bytes)
>  {
> -	unsigned long num_sectors = (bytes + root->sectorsize - 1) /
> -		root->sectorsize;
> -	num_sectors++;
> +	unsigned long num_sectors;
> +
> +	bytes += root->sectorsize - 1;
> +	do_div(bytes, root->sectorsize);
> +	num_sectors = bytes + 1;
> +
>  	return sizeof(struct btrfs_ordered_sum) +
>  		num_sectors * sizeof(struct btrfs_sector_sum);
>  }
> 
> -- 
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

Re: [PATCH] Use do_div() instead of native 64-bit division in btrfs_ordered_sum_size()

[Chris Mason]

On Wed, 2008-07-23 at 07:13 -0400, Christoph Hellwig wrote:
> Chris, can you please put this patch in?  Without it btrfs can't be
> loaded on 32bit platforms.
> 

I've pushed out a slightly different fix.  The ordered extents are based
on ram writeback, and so an unsigned long is enough.

-chris

Re: [PATCH] Use do_div() instead of native 64-bit division in btrfs_ordered_sum_size()

[David Woodhouse]

On Wed, 2008-07-23 at 09:28 -0400, Chris Mason wrote:
> On Wed, 2008-07-23 at 07:13 -0400, Christoph Hellwig wrote:
> > Chris, can you please put this patch in?  Without it btrfs can't be
> > loaded on 32bit platforms.
> > 
> 
> I've pushed out a slightly different fix.  The ordered extents are
> based on ram writeback, and so an unsigned long is enough.

Does the job for me, although we still need the
s/BUG_ON(spin_trylock(&tree->lock))/assert_spin_locked(&tree->lock)/
patch I sent a few days ago.

Now I can actually build the module, load it and avoid it hitting a
BUG_ON() when it first tries to mount, I can perhaps move on to doing
something more useful with it... :)

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH] Use do_div() instead of native 64-bit division in btrfs_ordered_sum_size()

[Chris Mason]

On Wed, 2008-07-23 at 17:07 -0400, David Woodhouse wrote:
> On Wed, 2008-07-23 at 09:28 -0400, Chris Mason wrote:
> > On Wed, 2008-07-23 at 07:13 -0400, Christoph Hellwig wrote:
> > > Chris, can you please put this patch in?  Without it btrfs can't be
> > > loaded on 32bit platforms.
> > > 
> > 
> > I've pushed out a slightly different fix.  The ordered extents are
> > based on ram writeback, and so an unsigned long is enough.
> 
> Does the job for me, although we still need the
> s/BUG_ON(spin_trylock(&tree->lock))/assert_spin_locked(&tree->lock)/
> patch I sent a few days ago.
> 
> Now I can actually build the module, load it and avoid it hitting a
> BUG_ON() when it first tries to mount, I can perhaps move on to doing
> something more useful with it... :)

;) My goal tonight is to figure out why I'm leaving locked data pages
everywhere.  Once that is done I'll be able to finally integrate the
pending patches.

-chris