From: "J. Bruce Fields" <bfields@fieldses.org>
To: Olga Kornievskaia <aglo@umich.edu>
Cc: Trond Myklebust <trondmy@gmail.com>,
"J. Bruce Fields" <bfields@redhat.com>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: Fix error return values for nfsd4_clone_file_range()
Date: Fri, 25 Jan 2019 15:15:51 -0500 [thread overview]
Message-ID: <20190125201551.GB5173@fieldses.org> (raw)
In-Reply-To: <20190125201037.GA5173@fieldses.org>
On Fri, Jan 25, 2019 at 03:10:37PM -0500, J. Bruce Fields wrote:
> Yeah. I was assuming it could happen in the case you ask to clone
> beyond the end of the source file. But looking at the code, there's a
> check for that case in generic_remap_checks() before doing the clone,
> and while holding a write lock on i_rwsem (I assume that's enough to
> hold the file size constant). At least that's true in the cases (btrfs
> & xfs) that I checked.
>
> So, I don't know, maybe that check is just dead code.
In the xfs case it looks like the main work of the clone is done in
xfs_reflink_remap_blocks(), where there's a loop like:
while (len) {
... mysterious code that clones range_len worth of
extents?
if (fatal_signal_pending(current)) {
error =-EINTR;
break;
}
...
len -= range_len;
remapped_len += range_len;
}
And then it ends up returning remapped_len if it's positive.
So it looks to me like if you do a big clone on xfs and kill the
process, it can clone part of the range, return the amount cloned, and
then the ioctl code will throw away that amount and just return EINVAL,
with the result that the application thinks the operation failed
completely actually it cloned a bunch of data.
--b.
next prev parent reply other threads:[~2019-01-25 20:15 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-21 20:58 [PATCH] nfsd: Fix error return values for nfsd4_clone_file_range() Trond Myklebust
2019-01-25 0:46 ` J. Bruce Fields
2019-01-25 5:50 ` Trond Myklebust
2019-01-25 16:32 ` J. Bruce Fields
2019-01-25 16:42 ` Olga Kornievskaia
2019-01-25 20:10 ` J. Bruce Fields
2019-01-25 20:15 ` J. Bruce Fields [this message]
2019-01-25 20:57 ` Darrick J. Wong
2019-01-26 22:36 ` J. Bruce Fields
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=20190125201551.GB5173@fieldses.org \
--to=bfields@fieldses.org \
--cc=aglo@umich.edu \
--cc=bfields@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trondmy@gmail.com \
/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).