collapse concurrent forced allocations (was: Re: clear chunk_alloc flag on retryable failure)

[Alexandre Oliva <oliva@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

On Feb 21, 2013, Alexandre Oliva <oliva@gnu.org> wrote:

> What I saw in that function also happens to explain why in some cases I
> see filesystems allocate a huge number of chunks that remain unused
> (leading to the scenario above, of not having more chunks to allocate).
> It happens for data and metadata, but not necessarily both.  I'm
> guessing some thread sets the force_alloc flag on the corresponding
> space_info, and then several threads trying to get disk space end up
> attempting to allocate a new chunk concurrently.  All of them will see
> the force_alloc flag and bump their local copy of force up to the level
> they see first, and they won't clear it even if another thread succeeds
> in allocating a chunk, thus clearing the force flag.  Then each thread
> that observed the force flag will, on its turn, force the allocation of
> a new chunk.  And any threads that come in while it does that will see
> the force flag still set and pick it up, and so on.  This sounds like a
> problem to me, but...  what should the correct behavior be?  Clear
> force_flag once we copy it to a local force?  Reset force to the
> incoming value on every loop?

I think a slight variant of the following makes the most sense, so I
implemented it in the patch below.

> Set the flag to our incoming force if we have it at first, clear our
> local flag, and move it from the space_info when we determined that we
> are the thread that's going to perform the allocation?



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: force-alloc-only-once.patch --]
[-- Type: text/x-diff, Size: 1415 bytes --]

From: Alexandre Oliva <oliva@gnu.org>

btrfs: consume force_alloc in the first thread to chunk_alloc

Even if multiple threads in do_chunk_alloc look at force_alloc and see
a force flag, it suffices that one of them consumes the flag.  Arrange
for an incoming force argument to make to force_alloc in case of
concurrent calls, so that it is used only by the first thread to get
to allocation after the initial request.

Signed-off-by: Alexandre Oliva <oliva@gnu.org>
---
 fs/btrfs/extent-tree.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 6ee89d5..66283f7 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3574,8 +3574,12 @@ static int do_chunk_alloc(struct btrfs_trans_handle *trans,
 
 again:
 	spin_lock(&space_info->lock);
+
+	/* Bring force_alloc to force and tentatively consume it.  */
 	if (force < space_info->force_alloc)
 		force = space_info->force_alloc;
+	space_info->force_alloc = CHUNK_ALLOC_NO_FORCE;
+
 	if (space_info->full) {
 		spin_unlock(&space_info->lock);
 		return 0;
@@ -3586,6 +3590,10 @@ again:
 		return 0;
 	} else if (space_info->chunk_alloc) {
 		wait_for_alloc = 1;
+		/* Reset force_alloc so that it's consumed by the
+		   first thread that completes the allocation.  */
+		space_info->force_alloc = force;
+		force = CHUNK_ALLOC_NO_FORCE;
 	} else {
 		space_info->chunk_alloc = 1;
 	}

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer