* [PATCH 0/1] btrfs: Fix NO_SPACE bug caused by delayed-iput @ 2015-02-26 3:20 Zhaolei 2015-02-26 3:20 ` [PATCH 1/1] " Zhaolei 0 siblings, 1 reply; 5+ messages in thread From: Zhaolei @ 2015-02-26 3:20 UTC (permalink / raw) To: linux-btrfs; +Cc: Zhao Lei From: Zhao Lei <zhaolei@cn.fujitsu.com> It is the last patch to fix following write fail case: while true; do write a file to 75% fs size delete above file sync or sleep done Above issue is caused by several reason, and fixed in following patch respectively: Btrfs: fix find_free_dev_extent() malfunction in case device tree has hole from Forrest Liu <forrestl@synology.com> btrfs: Fix out-of-space bug merged into v4.0-rc1 btrfs: fix condition of commit transaction btrfs: Fix tail space processing in find_free_dev_extent() btrfs: Adjust commit-transaction condition to avoid NO_SPACE more btrfs: Fix NO_SPACE bug caused by delayed-iput this patch These patchs reduced fail-rate step by step, from 50 fails in 20 * 200 loops, to 0 fails now. And now we can add a test case to xfstests for above action. Zhao Lei (1): btrfs: Fix NO_SPACE bug caused by delayed-iput fs/btrfs/ctree.h | 1 + fs/btrfs/disk-io.c | 3 ++- fs/btrfs/inode.c | 4 ++++ fs/btrfs/transaction.c | 6 +++++- 4 files changed, 12 insertions(+), 2 deletions(-) -- 1.8.5.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] btrfs: Fix NO_SPACE bug caused by delayed-iput 2015-02-26 3:20 [PATCH 0/1] btrfs: Fix NO_SPACE bug caused by delayed-iput Zhaolei @ 2015-02-26 3:20 ` Zhaolei 2015-02-26 22:29 ` David Sterba 0 siblings, 1 reply; 5+ messages in thread From: Zhaolei @ 2015-02-26 3:20 UTC (permalink / raw) To: linux-btrfs; +Cc: Zhao Lei From: Zhao Lei <zhaolei@cn.fujitsu.com> Steps to reproduce: while true; do dd if=/dev/zero of=/btrfs_dir/file count=[fs_size * 75%] rm /btrfs_dir/file sync done And we'll see dd failed because btrfs return NO_SPACE. Reason: Normally, btrfs_commit_transaction() call btrfs_run_delayed_iputs() in end to free fs space for next write, but sometimes it hadn't done work on time, because btrfs-cleaner thread get delayed-iputs from list before, but do iput() after next write. This is log: [ 2569.050776] comm=btrfs-cleaner func=btrfs_evict_inode() begin [ 2569.084280] comm=sync func=btrfs_commit_transaction() call btrfs_run_delayed_iputs() [ 2569.085418] comm=sync func=btrfs_commit_transaction() done btrfs_run_delayed_iputs() [ 2569.087554] comm=sync func=btrfs_commit_transaction() end [ 2569.191081] comm=dd begin [ 2569.790112] comm=dd func=__btrfs_buffered_write() ret=-28 [ 2569.847479] comm=btrfs-cleaner func=add_pinned_bytes() 0 + 32677888 = 32677888 [ 2569.849530] comm=btrfs-cleaner func=add_pinned_bytes() 32677888 + 23834624 = 56512512 ... [ 2569.903893] comm=btrfs-cleaner func=add_pinned_bytes() 943976448 + 21762048 = 965738496 [ 2569.908270] comm=btrfs-cleaner func=btrfs_evict_inode() end Fix: Make btrfs_commit_transaction() wait current running delayed-iputs() done in end. Test: Use script similar to above(more complex), before patch: 7 failed in 100 * 20 loop. after patch: 0 failed in 100 * 20 loop. Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com> --- fs/btrfs/ctree.h | 1 + fs/btrfs/disk-io.c | 3 ++- fs/btrfs/inode.c | 4 ++++ fs/btrfs/transaction.c | 6 +++++- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 84c3b00..ec2dac0 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1513,6 +1513,7 @@ struct btrfs_fs_info { spinlock_t delayed_iput_lock; struct list_head delayed_iputs; + struct rw_semaphore delayed_iput_sem; /* this protects tree_mod_seq_list */ spinlock_t tree_mod_seq_lock; diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index f79f385..1c0d8ec 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2241,11 +2241,12 @@ int open_ctree(struct super_block *sb, spin_lock_init(&fs_info->qgroup_op_lock); spin_lock_init(&fs_info->buffer_lock); spin_lock_init(&fs_info->unused_bgs_lock); - mutex_init(&fs_info->unused_bg_unpin_mutex); rwlock_init(&fs_info->tree_mod_log_lock); + mutex_init(&fs_info->unused_bg_unpin_mutex); mutex_init(&fs_info->reloc_mutex); mutex_init(&fs_info->delalloc_root_mutex); seqlock_init(&fs_info->profiles_lock); + init_rwsem(&fs_info->delayed_iput_sem); init_completion(&fs_info->kobj_unregister); INIT_LIST_HEAD(&fs_info->dirty_cowonly_roots); diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index a85c23d..a396bb9 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -3087,6 +3087,8 @@ void btrfs_run_delayed_iputs(struct btrfs_root *root) if (empty) return; + down_read(&fs_info->delayed_iput_sem); + spin_lock(&fs_info->delayed_iput_lock); list_splice_init(&fs_info->delayed_iputs, &list); spin_unlock(&fs_info->delayed_iput_lock); @@ -3097,6 +3099,8 @@ void btrfs_run_delayed_iputs(struct btrfs_root *root) iput(delayed->inode); kfree(delayed); } + + up_read(&fs_info->delayed_iput_sem); } /* diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index 7e80f32..175cdef 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -2068,8 +2068,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans, kmem_cache_free(btrfs_trans_handle_cachep, trans); - if (current != root->fs_info->transaction_kthread) + if (current != root->fs_info->transaction_kthread) { btrfs_run_delayed_iputs(root); + /* make sure that all running delayed iput are done */ + down_write(&root->fs_info->delayed_iput_sem); + up_write(&root->fs_info->delayed_iput_sem); + } return ret; -- 1.8.5.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] btrfs: Fix NO_SPACE bug caused by delayed-iput 2015-02-26 3:20 ` [PATCH 1/1] " Zhaolei @ 2015-02-26 22:29 ` David Sterba 2015-02-27 1:21 ` Zhao Lei 0 siblings, 1 reply; 5+ messages in thread From: David Sterba @ 2015-02-26 22:29 UTC (permalink / raw) To: Zhaolei; +Cc: linux-btrfs On Thu, Feb 26, 2015 at 11:20:23AM +0800, Zhaolei wrote: > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -2068,8 +2068,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans, > > kmem_cache_free(btrfs_trans_handle_cachep, trans); > > - if (current != root->fs_info->transaction_kthread) > + if (current != root->fs_info->transaction_kthread) { > btrfs_run_delayed_iputs(root); Using a mutex for this kind of synchronization is not entirely correct though it works. > + /* make sure that all running delayed iput are done */ > + down_write(&root->fs_info->delayed_iput_sem); > + up_write(&root->fs_info->delayed_iput_sem); ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/1] btrfs: Fix NO_SPACE bug caused by delayed-iput 2015-02-26 22:29 ` David Sterba @ 2015-02-27 1:21 ` Zhao Lei 2015-03-18 15:59 ` David Sterba 0 siblings, 1 reply; 5+ messages in thread From: Zhao Lei @ 2015-02-27 1:21 UTC (permalink / raw) To: dsterba; +Cc: linux-btrfs Hi, David Sterba * From: David Sterba [mailto:dsterba@suse.cz] > Sent: Friday, February 27, 2015 6:29 AM > To: Zhaolei > Cc: linux-btrfs@vger.kernel.org > Subject: Re: [PATCH 1/1] btrfs: Fix NO_SPACE bug caused by delayed-iput > > On Thu, Feb 26, 2015 at 11:20:23AM +0800, Zhaolei wrote: > > --- a/fs/btrfs/transaction.c > > +++ b/fs/btrfs/transaction.c > > @@ -2068,8 +2068,12 @@ int btrfs_commit_transaction(struct > > btrfs_trans_handle *trans, > > > > kmem_cache_free(btrfs_trans_handle_cachep, trans); > > > > - if (current != root->fs_info->transaction_kthread) > > + if (current != root->fs_info->transaction_kthread) { > > btrfs_run_delayed_iputs(root); > > Using a mutex for this kind of synchronization is not entirely correct though it > works. > rw sem is best way I thought which was choosed from mutex, rw_sem, and wait_queue, although the function name is not self documented... Do you have some suggestion for this kind of synchronization? Thanks Zhaolei > > + /* make sure that all running delayed iput are done */ > > + down_write(&root->fs_info->delayed_iput_sem); > > + up_write(&root->fs_info->delayed_iput_sem); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] btrfs: Fix NO_SPACE bug caused by delayed-iput 2015-02-27 1:21 ` Zhao Lei @ 2015-03-18 15:59 ` David Sterba 0 siblings, 0 replies; 5+ messages in thread From: David Sterba @ 2015-03-18 15:59 UTC (permalink / raw) To: Zhao Lei; +Cc: linux-btrfs On Fri, Feb 27, 2015 at 09:21:43AM +0800, Zhao Lei wrote: > Hi, David Sterba > > * From: David Sterba [mailto:dsterba@suse.cz] > > Sent: Friday, February 27, 2015 6:29 AM > > To: Zhaolei > > Cc: linux-btrfs@vger.kernel.org > > Subject: Re: [PATCH 1/1] btrfs: Fix NO_SPACE bug caused by delayed-iput > > > > On Thu, Feb 26, 2015 at 11:20:23AM +0800, Zhaolei wrote: > > > --- a/fs/btrfs/transaction.c > > > +++ b/fs/btrfs/transaction.c > > > @@ -2068,8 +2068,12 @@ int btrfs_commit_transaction(struct > > > btrfs_trans_handle *trans, > > > > > > kmem_cache_free(btrfs_trans_handle_cachep, trans); > > > > > > - if (current != root->fs_info->transaction_kthread) > > > + if (current != root->fs_info->transaction_kthread) { > > > btrfs_run_delayed_iputs(root); > > > > Using a mutex for this kind of synchronization is not entirely correct though it > > works. > > > rw sem is best way I thought which was choosed from > mutex, rw_sem, and wait_queue, although the function name is not > self documented... > Do you have some suggestion for this kind of synchronization? No better ideas at the moment, please proceed. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-18 15:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-26 3:20 [PATCH 0/1] btrfs: Fix NO_SPACE bug caused by delayed-iput Zhaolei 2015-02-26 3:20 ` [PATCH 1/1] " Zhaolei 2015-02-26 22:29 ` David Sterba 2015-02-27 1:21 ` Zhao Lei 2015-03-18 15:59 ` David Sterba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).