All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock
Date: Fri, 8 Jun 2018 10:48:29 +1000	[thread overview]
Message-ID: <20180608004829.GX10363@dastard> (raw)
In-Reply-To: <20180607114140.GB7798@bfoster>

On Thu, Jun 07, 2018 at 07:41:40AM -0400, Brian Foster wrote:
> On Thu, Jun 07, 2018 at 03:21:32PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > xfs_reflink_convert_cow() manipulates the incore extent list
> > in GFP_KERNEL context in the IO submission path whilst holding
> > locked pages under writeback. This is a memory reclaim deadlock
> > vector. This code is not in a transaction, so any memory allocations
> > it makes aren't protected via the memalloc_nofs_save() context that
> > transactions carry.
> > 
> > Hence we need to run this call under memalloc_nofs_save() context to
> > prevent potential memory allocations from being run as GFP_KERNEL
> > and deadlocking.
> > 
> > Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> > ---
> 
> Looks fine modulo the header thing Allison already pointed out:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> BTW, shouldn't we also be using XFS_TRANS_NOFS in
> xfs_iomap_write_allocate()?

Most likely, yes.

However, I think the whole ->writepages path should be moved under
memalloc_nofs_save() context. I'm kinda waiting for the bufferhead
removal to land before auditing it fully and determining the scope
we should be covering by nofs alloc contexts. This was just a
drive-by patch to fix a problem that was reported...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

      parent reply	other threads:[~2018-06-08  0:48 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-07  5:21 [PATCH 0/2] xfs: fix a couple of potential deadlocks Dave Chinner
2018-06-07  5:21 ` [PATCH 1/2] xfs: setup VFS i_rwsem lockdep state correctly Dave Chinner
2018-06-07  5:32   ` Dave Chinner
2018-06-07 11:41     ` Brian Foster
2018-06-07  5:50   ` Allison Henderson
2018-06-07 14:53   ` Darrick J. Wong
2018-06-07  5:21 ` [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock Dave Chinner
2018-06-07  5:56   ` Allison Henderson
2018-06-07 14:46     ` Darrick J. Wong
2018-06-07 22:08       ` Dave Chinner
2018-06-07 11:41   ` Brian Foster
2018-06-07 14:48     ` Darrick J. Wong
2018-06-08  0:48     ` Dave Chinner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180608004829.GX10363@dastard \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.