All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-nfs@vger.kernel.org, Chuck Lever <chuck.lever@oracle.com>
Subject: Re: nfs generic/373 failure after "fs: allow cross-vfsmount reflink/dedupe"
Date: Wed, 2 Mar 2022 18:45:12 -0500	[thread overview]
Message-ID: <YiABiLtH/4nMJE+u@localhost.localdomain> (raw)
In-Reply-To: <20220302224250.GF10757@fieldses.org>

On Wed, Mar 02, 2022 at 05:42:50PM -0500, J. Bruce Fields wrote:
> On Wed, Mar 02, 2022 at 05:26:08PM -0500, Josef Bacik wrote:
> > On Wed, Mar 02, 2022 at 05:04:50PM -0500, J. Bruce Fields wrote:
> > > I started seeing generic/373 fail on recent linux-next in NFS testing.
> > > 
> > > Bisect lands it on aaf40970b1d0 "fs: allow cross-vfsmount
> > > reflink/dedupe".
> > > 
> > > The test fails because a clone between two mounts is expected to fail,
> > > and no longer does.
> > > 
> > > In my setup both mounts are nfs mounts.  They are mounts of different
> > > exports, and the exports are exports of different filesystems.  So it
> > > does make sense that the clone should fail.
> > > 
> > > I see the NFS client send a CLONE rpc to the server, and the server
> > > return success.  That seems wrong.
> > > 
> > > Both exported filesystems are xfs, and from the code it looks like the
> > > server calls vfs_clone_file_range(), which ends up calling
> > > xfs_file_remap_range().
> > > 
> > > Are we missing a check now in that xfs case?
> > > 
> > > I haven't looked any more closely at what's going on, so I could be
> > > missing something.
> > > 
> > 
> > Yeah there's a few fstests that test this functionality that need to be removed,
> > I have patches pending for this in our fstests staging tree (since we run
> > fstests nightly on our tree)
> > 
> > https://github.com/btrfs/fstests/tree/staging
> > 
> > Right now the patches just remove the tests from auto since that's what we run,
> > I'll remove them properly once the patch lands in linus.  Thanks,
> 
> So, out of curiosity, what is xfs doing in this case?  These are two
> filesystems on separate partitions, is it falling back on a read/write
> loop or something?

I don't think so?  I'm actually kind of confused, because nfsd does
vfs_clone_file_range, and the only place I messed with for CLONE was
ioctl_clone_file, so the patch changed literally nothing, unless you aren't
using nfsd for the server?

And if they are in fact two different file systems the i_sb != i_sb of the
files, so there's something pretty strange going on here, my patch shouldn't
affect your setup.  Thanks,

Josef

  reply	other threads:[~2022-03-02 23:46 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-01 18:42 [PATCH v3 0/6] Generic per-sb io stats Amir Goldstein
2022-03-01 18:42 ` [PATCH v3 1/6] lib/percpu_counter: add helpers for arrays of counters Amir Goldstein
2022-03-01 18:42 ` [PATCH v3 2/6] fs: add optional iostats counters to struct super_block Amir Goldstein
2022-03-01 18:42 ` [PATCH v3 3/6] fs: collect per-sb io stats Amir Goldstein
2022-03-01 18:42 ` [PATCH v3 4/6] fs: report " Amir Goldstein
2022-03-01 18:42 ` [PATCH v3 5/6] ovl: opt-in for " Amir Goldstein
2022-03-01 18:42 ` [PATCH v3 6/6] fuse: " Amir Goldstein
2022-03-02  6:59 ` [PATCH v3 0/6] Generic " Dave Chinner
2022-03-02  7:43   ` Amir Goldstein
2022-03-02  8:26     ` Dave Chinner
2022-03-02  8:34       ` Amir Goldstein
2022-03-02 16:59       ` Amir Goldstein
2022-03-02 21:12         ` Dave Chinner
2022-03-02 22:04           ` nfs generic/373 failure after "fs: allow cross-vfsmount reflink/dedupe" J. Bruce Fields
2022-03-02 22:26             ` Josef Bacik
2022-03-02 22:42               ` J. Bruce Fields
2022-03-02 23:45                 ` Josef Bacik [this message]
2022-03-03  0:07                   ` J. Bruce Fields
2022-03-03  0:29                     ` Josef Bacik
2022-03-03  0:50                       ` J. Bruce Fields
2022-03-04 20:03                         ` J. Bruce Fields
2022-03-03  6:50           ` [PATCH v3 0/6] Generic per-sb io stats Amir Goldstein

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=YiABiLtH/4nMJE+u@localhost.localdomain \
    --to=josef@toxicpanda.com \
    --cc=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=linux-fsdevel@vger.kernel.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.