linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matt Whitlock <kernel@mattwhitlock.name>
To: David Howells <dhowells@redhat.com>
Cc: Matthew Wilcox <willy@infradead.org>,
	Dave Chinner <david@fromorbit.com>,
	<linux-fsdevel@vger.kernel.org>, Christoph Hellwig <hch@lst.de>,
	Jens Axboe <axboe@kernel.dk>, Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [Reproducer] Corruption, possible race between splice and FALLOC_FL_PUNCH_HOLE
Date: Wed, 28 Jun 2023 13:35:27 -0400	[thread overview]
Message-ID: <a60594ef-ff85-498f-a1c4-0fcb9586621c@mattwhitlock.name> (raw)
In-Reply-To: <3299543.1687933850@warthog.procyon.org.uk>

On Wednesday, 28 June 2023 02:30:50 EDT, David Howells wrote:
> Matthew Wilcox <willy@infradead.org> wrote:
>
>>>> Expected behavior:
>>>> Punching holes in a file after splicing pages out of that 
>>>> file into a pipe
>>>> should not corrupt the spliced-out pages in the pipe buffer.
>
> I think this bit is the key.  Why would this be the expected behaviour?

Why? Because SPLICE_F_MOVE exists. Even though that flag is a no-op as of 
Linux 2.6.21, its existence implies that calling splice() *without* 
specifying SPLICE_F_MOVE performs a *copy*. The kernel is, of course, free 
*not* to copy pages in pursuit of better performance, but it must behave as 
though it did copy unless SPLICE_F_MOVE is specified, in which case 
userspace is explicitly acknowledging that subsequent modification of the 
spliced pages may impact the spliced data. Effectively, SPLICE_F_MOVE is a 
promise by userspace that the moved pages will not be subsequently 
modified, and if they are, then all bets are off.

In other words, the currently implemented behavior is appropriate for 
SPLICE_F_MOVE, but it is not appropriate for ~SPLICE_F_MOVE.

  parent reply	other threads:[~2023-06-28 17:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27  1:12 [Reproducer] Corruption, possible race between splice and FALLOC_FL_PUNCH_HOLE Matt Whitlock
2023-06-27  5:47 ` Dave Chinner
2023-06-27  6:20   ` Dave Chinner
2023-06-27 10:31   ` Matthew Wilcox
2023-06-27 18:38     ` Matt Whitlock
2023-06-27 21:49     ` Dave Chinner
2023-06-27 18:14   ` Matt Whitlock
2023-06-27 21:51     ` Dave Chinner
2023-06-28  6:30   ` David Howells
2023-06-28  8:15     ` Dave Chinner
2023-06-28  9:33     ` David Howells
2023-06-28 12:51       ` Matthew Wilcox
2023-06-28 14:19       ` David Howells
2023-06-28 22:41       ` Dave Chinner
2023-06-28 17:35     ` Matt Whitlock [this message]
2023-06-28 18:27     ` David Howells
2023-06-28 22:17       ` Dave Chinner

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=a60594ef-ff85-498f-a1c4-0fcb9586621c@mattwhitlock.name \
    --to=kernel@mattwhitlock.name \
    --cc=axboe@kernel.dk \
    --cc=david@fromorbit.com \
    --cc=dhowells@redhat.com \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.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;
as well as URLs for NNTP newsgroup(s).