On Fri, Apr 13, 2018 at 04:52:25PM -0700, Liu Bo wrote:
> On Fri, Apr 13, 2018 at 1:28 PM, Josef Bacik <jo...@toxicpanda.com> wrote:
> > From: Josef Bacik <jba...@fb.com>
> >
> > Since we're allocating under atomic we could every easily enomem, so if
> > that's the case and we can block then loop around and try to allocate
> > the prealloc not under a lock.
> >
> > We also saw this happen during try_to_release_page in production, in
> > which case it's completely valid to return ENOMEM so we can tell
> > try_to_release_page that we can't release this page.
> >
> > Signed-off-by: Josef Bacik <jba...@fb.com>
> > ---
> >  fs/btrfs/extent_io.c | 28 ++++++++++++++++++++++++----
> >  1 file changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> > index fb32394fd830..1054dc0158b5 100644
> > --- a/fs/btrfs/extent_io.c
> > +++ b/fs/btrfs/extent_io.c
> > @@ -593,8 +593,9 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 
> > start, u64 end,
> >         struct extent_state *prealloc = NULL;
> >         struct rb_node *node;
> >         u64 last_end;
> > -       int err;
> > +       int err = 0;
> >         int clear = 0;
> > +       bool need_prealloc = false;
> >
> >         btrfs_debug_check_extent_io_range(tree, start, end);
> >
> > @@ -617,6 +618,9 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 
> > start, u64 end,
> >                  * If we end up needing a new extent state we allocate it 
> > later.
> >                  */
> >                 prealloc = alloc_extent_state(mask);
> > +               if (!prealloc && need_prealloc)
> > +                       return -ENOMEM;
> > +               need_prealloc = false;
> >         }
> >
> >         spin_lock(&tree->lock);
> > @@ -676,7 +680,15 @@ int __clear_extent_bit(struct extent_io_tree *tree, 
> > u64 start, u64 end,
> >
> >         if (state->start < start) {
> >                 prealloc = alloc_extent_state_atomic(prealloc);
> > -               BUG_ON(!prealloc);
> > +               if (!prealloc) {
> > +                       if (gfpflags_allow_blocking(mask)) {
> > +                               need_prealloc = true;
> > +                               spin_unlock(&tree->lock);
> > +                               goto again;
> 
> Could we simply 'goto search_again;' ?
>

We could, but I have another patch that's more involved that reworks this logic
and kills search_again, so I'm leaving it this way for now for this isolated
fix.  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

Reply via email to