diff mbox

Btrfs: wait on async pages when shrinking delalloc

Message ID 1346964716-12070-1-git-send-email-jbacik@fusionio.com (mailing list archive)
State New, archived
Headers show

Commit Message

Josef Bacik Sept. 6, 2012, 8:51 p.m. UTC
Mitch reported a problem where you could get an ENOSPC error when untarring
a kernel git tree onto a 16gb file system with compress-force=zlib.  This is
because compression is a huge pain, it will return from ->writepages()
without having actually created any ordered extents.  To get around this we
check to see if the async submit counter is up, and if it is wait until it
drops to 0 before doing our normal ordered wait dance.  With this patch I
can now untar a kernel git tree onto a 16gb file system without getting
ENOSPC errors.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
 fs/btrfs/extent-tree.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

Zach Brown Sept. 6, 2012, 9:07 p.m. UTC | #1
> +		if (atomic_read(&root->fs_info->async_delalloc_pages))
> +			wait_event(root->fs_info->async_submit_wait,
> +			  !atomic_read(&root->fs_info->async_delalloc_pages));

The very first thing wait_event_*() family does is test the condition.
You don't need to guard them with an inverse copy of the condition.
Just call them directly and they'll do the right thing and bail cheaply
when the condition is already true.

- z
--
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
Mitch Harder Sept. 6, 2012, 11:57 p.m. UTC | #2
On Thu, Sep 6, 2012 at 3:51 PM, Josef Bacik <jbacik@fusionio.com> wrote:
> Mitch reported a problem where you could get an ENOSPC error when untarring
> a kernel git tree onto a 16gb file system with compress-force=zlib.  This is
> because compression is a huge pain, it will return from ->writepages()
> without having actually created any ordered extents.  To get around this we
> check to see if the async submit counter is up, and if it is wait until it
> drops to 0 before doing our normal ordered wait dance.  With this patch I
> can now untar a kernel git tree onto a 16gb file system without getting
> ENOSPC errors.  Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>

Thanks, this patch fixes the issues I was seeing with ENOSPC on zlib
compression.

I also did some rough testing for any performance regressions on lzo
and with no compression, and my benchmarks were all in the same range.
 I don't have any comparisons available for zlib since my benchmark
tests would always trigger ENOSPC errors.

I also checked Zach Brown's suggestion of dropping:

> +               if (atomic_read(&root->fs_info->async_delalloc_pages))

and just leaving:

> +                       wait_event(root->fs_info->async_submit_wait,
> +                         !atomic_read(&root->fs_info->async_delalloc_pages));

This is because the wait_event macro should perform the same test
(although it will start a 'do' loop before making the same test).

This change also worked in my tests.

For reference, I pulled up the wait_event macro according to the Linux
Cross Reference site:

http://lxr.free-electrons.com/source/include/linux/wait.h#L205

205 /**
206  * wait_event - sleep until a condition gets true
207  * @wq: the waitqueue to wait on
208  * @condition: a C expression for the event to wait for
209  *
210  * The process is put to sleep (TASK_UNINTERRUPTIBLE) until the
211  * @condition evaluates to true. The @condition is checked each time
212  * the waitqueue @wq is woken up.
213  *
214  * wake_up() has to be called after changing any variable that could
215  * change the result of the wait condition.
216  */
217 #define wait_event(wq, condition)                                       \
218 do {                                                                    \
219         if (condition)                                                  \
220                 break;                                                  \
221         __wait_event(wq, condition);                                    \
222 } while (0)

Tested-by: Mitch Harder <mitch.harder@sabayonlinux.org>
--
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
diff mbox

Patch

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index b6b33e4..4bb546da 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3697,6 +3697,14 @@  static void shrink_delalloc(struct btrfs_root *root, u64 to_reclaim, u64 orig,
 		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages,
 					       WB_REASON_FS_FREE_SPACE);
 
+		/*
+		 * We need to wait for the async pages to actually start before
+		 * we do anything.
+		 */
+		if (atomic_read(&root->fs_info->async_delalloc_pages))
+			wait_event(root->fs_info->async_submit_wait,
+			  !atomic_read(&root->fs_info->async_delalloc_pages));
+
 		spin_lock(&space_info->lock);
 		if (space_info->bytes_used + space_info->bytes_reserved +
 		    space_info->bytes_pinned + space_info->bytes_readonly +