All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eryu Guan <eguan@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Anna Schumaker <Anna.Schumaker@Netapp.com>,
	fstests@vger.kernel.org, linux-nfs@vger.kernel.org,
	hch@infradead.org
Subject: Re: [PATCH v4 1/5] generic/392: Add copy to new file test
Date: Tue, 20 Dec 2016 11:26:28 +0800	[thread overview]
Message-ID: <20161220032628.GE1859@eguan.usersys.redhat.com> (raw)
In-Reply-To: <20161219223447.GA7300@birch.djwong.org>

On Mon, Dec 19, 2016 at 02:34:47PM -0800, Darrick J. Wong wrote:
> > 
> > And seems copy_range -l option doesn't work well with xfs reflink
> > enabled? I saw many EINVALs when testing reflink enabled xfs, all these
> > 5 tests failed. Kernel bug or xfs_io bug or we just want to _notrun on
> > reflink xfs?
> 
> Ugggggggghhhhhhhhh...
> 
> Prior to hch's vfs patch reorganizing vfs_copy_file_range to try clone first,
> this happened[1]:
> 
> ret = -EOPNOTSUPP;
> if (file_out->f_op->copy_file_range)
> 	ret = file_out->f_op->copy_file_range(file_in, pos_in, file_out,
> 					      pos_out, len, flags);
> if (ret == -EOPNOTSUPP)
> 	ret = do_splice_direct(file_in, &pos_in, file_out, &pos_out,
> 			len > MAX_RW_COUNT ? MAX_RW_COUNT : len, 0);
> 
> Note that copy_file_range pointed to XFS's clone_range implementation,
> so if you pass in an offset/length that aren't block aligned, the clone
> code returns EINVAL and ... we pass that out to userspace instead of
> falling back.
> 
> Now it does this[2]:
> 
> if (file_in->f_op->clone_file_range) {
> 	ret = file_in->f_op->clone_file_range(file_in, pos_in,
> 			file_out, pos_out, len);
> 	if (ret == 0) {
> 		ret = len;
> 		goto done;
> 	}
> }
> 
> if (file_out->f_op->copy_file_range) {
> 	ret = file_out->f_op->copy_file_range(file_in, pos_in, file_out,
> 					      pos_out, len, flags);
> 	if (ret != -EOPNOTSUPP)
> 		goto done;
> }
> 
> ret = do_splice_direct(file_in, &pos_in, file_out, &pos_out,
> 		len > MAX_RW_COUNT ? MAX_RW_COUNT : len, 0);
> 
> XFS removed the copy_file_range pointer, so therefore we'll try to
> reflink the range and if that doesn't succeed we'll fall back to
> whatever do_splice_direct does.  All those EINVALs should go away
> shortly, at least on reflink-XFS.

Good to know, thanks for the explanation!

Eryu

> 
> --D
> 
> [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/read_write.c?id=ffecee4f2442bb8cb6b34c3335fef4eb50c22fdd#n1545
> [2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/read_write.c#n1543
> 
> > 
> > Thanks,
> > Eryu
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-12-20  3:26 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-08 18:49 [PATCH v4 0/5] Add copy_file_range() tests Anna Schumaker
2016-12-08 18:49 ` Anna Schumaker
2016-12-08 18:49 ` [PATCH v4 1/5] generic/392: Add copy to new file test Anna Schumaker
2016-12-08 18:49   ` Anna Schumaker
2016-12-19 10:04   ` Eryu Guan
2016-12-19 22:34     ` Darrick J. Wong
2016-12-20  3:26       ` Eryu Guan [this message]
2016-12-08 18:49 ` [PATCH v4 2/5] generic/393: Add small copies " Anna Schumaker
2016-12-08 18:49   ` Anna Schumaker
2016-12-08 18:49 ` [PATCH v4 3/5] generic/394: Add copy test that overwrites data Anna Schumaker
2016-12-08 18:49   ` Anna Schumaker
2016-12-08 18:49 ` [PATCH v4 4/5] generic/395: Add a copy test for overwriting small amounts of data Anna Schumaker
2016-12-08 18:49   ` Anna Schumaker
2016-12-08 18:49 ` [PATCH v4 5/5] generic/396: Add a copy test for invalid input Anna Schumaker
2016-12-08 18:49   ` Anna Schumaker
2016-12-08 20:20   ` Darrick J. Wong
2016-12-08 20:28     ` Anna Schumaker
2016-12-08 20:28       ` Anna Schumaker
2016-12-08 18:49 ` [PATCH v4 6/5] xfs_io: Improvements to copy_range return code handling Anna Schumaker
2016-12-08 18:49   ` Anna Schumaker
2016-12-11  9:03 ` [PATCH v4 0/5] Add copy_file_range() tests Amir Goldstein
2016-12-11 18:34   ` Darrick J. Wong

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=20161220032628.GE1859@eguan.usersys.redhat.com \
    --to=eguan@redhat.com \
    --cc=Anna.Schumaker@Netapp.com \
    --cc=darrick.wong@oracle.com \
    --cc=fstests@vger.kernel.org \
    --cc=hch@infradead.org \
    --cc=linux-nfs@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.