On Fri, Apr 29, 2016 at 10:20 AM, David Sterba <dste...@suse.com> wrote:
> In __set_extent_bit we allocate with GFP_ATOMIC with the tree lock
> held, this takes away allocator opportunities to satisfy the allocation.
> In some cases we leave the locked section and we could repeat the
> preallocation with less strict flags. It could lead to unnecessary
> allocation, but we won't fail until we really need it.
>
> Signed-off-by: David Sterba <dste...@suse.com>
> ---
> fs/btrfs/extent_io.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 8707bcc615ff..06ad442f6c03 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1049,6 +1049,13 @@ __set_extent_bit(struct extent_io_tree *tree, u64
> start, u64 end,
> spin_unlock(&tree->lock);
> if (gfpflags_allow_blocking(mask))
> cond_resched();
> + /*
> + * If we used the preallocated state, try again here out of the
> + * locked section so we can avoid GFP_ATOMIC. No error checking
> + * as we might not need it in the end.
> + */
> + if (!prealloc)
> + prealloc = alloc_extent_state(mask);
Under the again label we already try to allocate in non atomic mode
(gfpflags_allow_blocking() returns us true for GFP_NOFS), and some
other patch in the series removes the BUG_ON() there.
So this extra allocation attempt seems unnecessary to me. Or did I
miss something?
At least I'm seeing the following in 4.5's gfp.h:
#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) /*
Caller can reclaim */
#define __GFP_RECLAIM ((__force
gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM))
#define GFP_NOFS (__GFP_RECLAIM | __GFP_IO)
static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
{
return !!(gfp_flags & __GFP_DIRECT_RECLAIM);
}
So:
GFP_NOFS == ___GFP_DIRECT_RECLAIM | ___GFP_KSWAPD_RECLAIM | __GFP_IO
__GFP_DIRECT_RECLAIM == ___GFP_DIRECT_RECLAIM
So, as we always use GFP_NOFS for the gfp flags value...
Some comment goes for the other similar patches.
> goto again;
>
> out:
> --
> 2.7.1
>
> --
> 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
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
--
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
