public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: xfs <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH] xfs: fix reflink source file racing with directio writes
Date: Fri, 16 Aug 2019 07:37:17 +1000	[thread overview]
Message-ID: <20190815213716.GS6129@dread.disaster.area> (raw)
In-Reply-To: <20190815165043.GB15186@magnolia>

On Thu, Aug 15, 2019 at 09:50:43AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> While trawling through the dedupe file comparison code trying to fix
> page deadlocking problems, Dave Chinner noticed that the reflink code
> only takes shared IOLOCK/MMAPLOCKs on the source file.  Because
> page_mkwrite and directio writes do not take the EXCL versions of those
> locks, this means that reflink can race with writer processes.
> 
> For pure remapping this can lead to undefined behavior and file
> corruption; for dedupe this means that we cannot be sure that the
> contents are identical when we decide to go ahead with the remapping.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I don't think this is quite right yet. The source inode locking
change is good, but it doesn't break the layout on the source inode
and so there is still they possibility that something has physical
access and is directly modifying the source file.

And with that, I suspect the locking algorithm changes
substantially:

	order inodes
restart:
	lock first inode
	break layout on first inode
	lock second inode
	break layout on second inode
	fail then unlock both inodes, goto restart

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2019-08-15 21:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-15 16:50 [PATCH] xfs: fix reflink source file racing with directio writes Darrick J. Wong
2019-08-15 21:37 ` Dave Chinner [this message]
2019-08-16  6:42   ` Christoph Hellwig
2019-08-16 16:11 ` [PATCH v2] " Darrick J. Wong
2019-08-18  8:38   ` Christoph Hellwig

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=20190815213716.GS6129@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=darrick.wong@oracle.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox