Re: [PATCH] Btrfs: don't commit the transaction if we dont have enough pinned bytes V2
On 05/26/2011 04:57 AM, Adrian Hunter wrote: > On 25/05/11 22:30, Josef Bacik wrote: >> I noticed when running an enospc test that we would get stuck >> committing the >> transaction in check_data_space even though we truly didn't have >> enough space. >> So check to see if bytes_pinned is bigger than num_bytes, if it's not >> don't >> commit the transaction. Thanks, >> >> Signed-off-by: Josef Bacik >> --- >> V1->V2: Make it so it actually compiles ;) >> fs/btrfs/extent-tree.c |7 +++ >> 1 files changed, 7 insertions(+), 0 deletions(-) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index c8c3184..b4f67e8 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -3199,6 +3199,13 @@ alloc: >> } >> goto again; >> } >> + >> +/* >> + * If we have less pinned bytes than we want to allocate then >> + * don't bother committing the transaction, it won't help us. >> + */ >> +if (data_sinfo->bytes_pinned< bytes) >> +committed = 1; >> spin_unlock(&data_sinfo->lock); >> >> /* commit the current transaction and try again */ > > I tried that patch on 2.6.39 with the following: > > sudo modprobe brd rd_size=262144 > sudo mkfs.btrfs /dev/ram0 > sudo mkdir -p /mnt/test > sudo mount -t btrfs /dev/ram0 /mnt/test > sudo mkdir -p /mnt/test/test > sudo chown $USER /mnt/test/test > sudo chgrp $USER /mnt/test/test > sudo umount /mnt/test > i=0 > while true; do > sudo mount -t btrfs /dev/ram0 /mnt/test > fsstress -c -r -d /mnt/test/test -p 3 -n 1000 -l 10 > sudo umount /mnt/test > i=`expr $i \+ 1` > echo $i > done > > > After 3 iterations it got really slow and then after some minutes it > still seems to lock up: > Did you run without my patch? I assume this will still happen even without my patch. The only possible negative side-effect of my patch is we could ENOSPC early. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs: don't commit the transaction if we dont have enough pinned bytes V2
On 25/05/11 22:30, Josef Bacik wrote: I noticed when running an enospc test that we would get stuck committing the transaction in check_data_space even though we truly didn't have enough space. So check to see if bytes_pinned is bigger than num_bytes, if it's not don't commit the transaction. Thanks, Signed-off-by: Josef Bacik --- V1->V2: Make it so it actually compiles ;) fs/btrfs/extent-tree.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index c8c3184..b4f67e8 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -3199,6 +3199,13 @@ alloc: } goto again; } + + /* +* If we have less pinned bytes than we want to allocate then +* don't bother committing the transaction, it won't help us. +*/ + if (data_sinfo->bytes_pinned< bytes) + committed = 1; spin_unlock(&data_sinfo->lock); /* commit the current transaction and try again */ I tried that patch on 2.6.39 with the following: sudo modprobe brd rd_size=262144 sudo mkfs.btrfs /dev/ram0 sudo mkdir -p /mnt/test sudo mount -t btrfs /dev/ram0 /mnt/test sudo mkdir -p /mnt/test/test sudo chown $USER /mnt/test/test sudo chgrp $USER /mnt/test/test sudo umount /mnt/test i=0 while true; do sudo mount -t btrfs /dev/ram0 /mnt/test fsstress -c -r -d /mnt/test/test -p 3 -n 1000 -l 10 sudo umount /mnt/test i=`expr $i \+ 1` echo $i done After 3 iterations it got really slow and then after some minutes it still seems to lock up: [ 2059.881182] SysRq : Show Blocked State [ 2059.881188] taskPC stack pid father [ 2059.881226] btrfs-transacti D 000100046afc 0 2212 2 0x0080 [ 2059.881233] 880181483d60 0046 0003 8801 [ 2059.881239] 00013a80 00013a80 00013a80 880184dac530 [ 2059.881244] 00013a80 880181483fd8 00013a80 00013a80 [ 2059.881249] Call Trace: [ 2059.881273] [] wait_for_commit.clone.14+0x90/0xd5 [btrfs] [ 2059.881280] [] ? wake_up_bit+0x2a/0x2a [ 2059.881297] [] btrfs_commit_transaction+0x102/0x665 [btrfs] [ 2059.881312] [] ? join_transaction.clone.21+0x180/0x18b [btrfs] [ 2059.881316] [] ? wake_up_bit+0x2a/0x2a [ 2059.881331] [] transaction_kthread+0x17a/0x22f [btrfs] [ 2059.881345] [] ? btrfs_congested_fn+0x82/0x82 [btrfs] [ 2059.881349] [] kthread+0x82/0x8a [ 2059.881355] [] kernel_thread_helper+0x4/0x10 [ 2059.881359] [] ? kthread_worker_fn+0x14b/0x14b [ 2059.881364] [] ? gs_change+0x13/0x13 [ 2059.881366] flush-btrfs-5 D 0001000f78a6 0 2219 2 0x0080 [ 2059.881371] 8801828efb10 0046 0754 8801 [ 2059.881376] 00013a80 00013a80 00013a80 8801a53fc530 [ 2059.881382] 00013a80 8801828effd8 00013a80 00013a80 [ 2059.881386] Call Trace: [ 2059.881391] [] schedule_timeout+0x36/0xe3 [ 2059.881396] [] ? arch_local_irq_save+0x18/0x1e [ 2059.881400] [] ? _raw_spin_unlock_irqrestore+0x17/0x19 [ 2059.881404] [] ? prepare_to_wait+0x6c/0x79 [ 2059.881419] [] btrfs_commit_transaction+0x2b2/0x665 [btrfs] [ 2059.881423] [] ? wake_up_bit+0x2a/0x2a [ 2059.881440] [] btrfs_write_inode+0x9b/0xa3 [btrfs] [ 2059.881445] [] writeback_single_inode+0x125/0x1bf [ 2059.881450] [] writeback_sb_inodes+0xce/0x160 [ 2059.881454] [] wb_writeback+0x257/0x3a7 [ 2059.881459] [] wb_do_writeback+0x88/0x1a0 [ 2059.881464] [] ? run_timer_softirq+0x298/0x298 [ 2059.881468] [] bdi_writeback_thread+0x8b/0x20c [ 2059.881473] [] ? wb_do_writeback+0x1a0/0x1a0 [ 2059.881476] [] kthread+0x82/0x8a [ 2059.881481] [] kernel_thread_helper+0x4/0x10 [ 2059.881485] [] ? kthread_worker_fn+0x14b/0x14b [ 2059.881489] [] ? gs_change+0x13/0x13 [ 2059.881493] fsstressD 8801a23f8000 0 2304 1 0x0084 [ 2059.881497] 880189c93d48 0082 0400 [ 2059.881502] 00013a80 00013a80 00013a80 88017bc59710 [ 2059.881507] 00013a80 880189c93fd8 00013a80 00013a80 [ 2059.881512] Call Trace: [ 2059.881517] [] ? resched_task+0x48/0x72 [ 2059.881522] [] schedule_timeout+0x36/0xe3 [ 2059.881526] [] ? _raw_spin_unlock_irqrestore+0x17/0x19 [ 2059.881530] [] ? need_resched+0x23/0x2d [ 2059.881534] [] wait_for_common+0xad/0x102 [ 2059.881539] [] ? try_to_wake_up+0x226/0x226 [ 2059.881543] [] ? local_bh_enable_ip+0xe/0x10 [ 2059.881548] [] ? __sync_filesystem+0x7a/0x7a [ 2059.881552] [] wait_for_completion+0x1d/0x1f [ 2059.881556] [] sync_inodes_sb+0x91/0x187 [ 2059.881561] [] ? __sync_filesystem+0x7a/0x7a [ 2059.881565] [] __sync_filesystem+0x47/0x7a [ 2059.881569] [] sync_one_
[PATCH] Btrfs: don't commit the transaction if we dont have enough pinned bytes V2
I noticed when running an enospc test that we would get stuck committing the transaction in check_data_space even though we truly didn't have enough space. So check to see if bytes_pinned is bigger than num_bytes, if it's not don't commit the transaction. Thanks, Signed-off-by: Josef Bacik --- V1->V2: Make it so it actually compiles ;) fs/btrfs/extent-tree.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index c8c3184..b4f67e8 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -3199,6 +3199,13 @@ alloc: } goto again; } + + /* +* If we have less pinned bytes than we want to allocate then +* don't bother committing the transaction, it won't help us. +*/ + if (data_sinfo->bytes_pinned < bytes) + committed = 1; spin_unlock(&data_sinfo->lock); /* commit the current transaction and try again */ -- 1.7.2.3 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
