Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1647894991.git.dsterba@suse.com> <CAADWXX-uX74SETx8QNnGDyBGMJHY-6wr8jC9Sjpv4ARqUca0Xw@mail.gmail.com>
 <Yjo3tQO+fNNlZ4/i@localhost.localdomain> <20220322211935.GC8182@magnolia>
In-Reply-To: <20220322211935.GC8182@magnolia>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Thu, 24 Mar 2022 12:10:56 +0200
Message-ID: <CAOQ4uxjNqNxF6YQK7Euo9qCg3sTHrWESw+H_G0c8QaXFDQhGRA@mail.gmail.com>
Subject: Re: [GIT PULL] Btrfs updates for 5.18
To:     "Darrick J. Wong" <djwong@kernel.org>
Cc:     Josef Bacik <josef@toxicpanda.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        David Sterba <dsterba@suse.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        linux-btrfs <linux-btrfs@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Mar 24, 2022 at 1:35 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Tue, Mar 22, 2022 at 04:55:17PM -0400, Josef Bacik wrote:
> > On Tue, Mar 22, 2022 at 11:23:21AM -0700, Linus Torvalds wrote:
> > > On Mon, Mar 21, 2022 at 2:37 PM David Sterba <dsterba@suse.com> wrote:
> > > >
> > > > - allow reflinks/deduplication from two different mounts of the same
> > > >   filesystem
> > >
> > > So I've pulled this, and it looks ok, but I'm not getting the warm and fuzzies.
> > >
> > > In particular, I'm not seeing any commentary about different
> > > filesystems for this.
> > >
> > > There are several filesystems that use that ->remap_file_range()
> > > operation, so these relaxed rules don't just affect btrfs.
> > >
> > > Yes, yes, checking for i_sb matching does seem sensible, but I'd
> > > *really* have liked some sign that people checked with other
> > > filesystem maintainers and this is ok for all of them, and they didn't
> > > make assumptions about "always same mount" rather than "always same
> > > filesystem".
> > >
> >
> > > This affects at least cifs, nfs, overlayfs and ocfs2.
> >
> > I had a talk with Darrick Wong about this on IRC, and his Reviewed-by is on the
> > patch.  This did surprise nfsd when xfstests started failing, but talking with
> > Bruce he didn't complain once he understood what was going on.
>
> FWIW, I remember talking about this with Bruce and (probably Anna too)
> during a hallway BOF at the last LSFMMBPFBBQ that I went to, which was
> 2018(?)  At the time, I think we resolved that nfs42_remap_file_range
> was capable of detecting and dealing with unsupported requests, so a
> direct comparison of the ->remap_file_range or ->f_op wasn't necessary
> for them.
>
> > Believe me I
> > have 0 interest in getting the other maintainers upset with me by sneaking
> > something by them, I made sure to run it by people first, tho I probably should
> > have checked with people directly other than Darrick.
>
> I /am/ a little curious what Steve French has to say w.r.t CIFS.
>
> AFAICT overlayfs passes the request down to the appropriate fs
> under-layer, so its correctness mostly depends on the under-layer's
> implementation.  But I'll let Amir or someone chime in on that. ;)
>

The thing is, overlayfs ALREADY does cross-mnt clone_file_range()
on underlying layers. So if there was a bug with allowing cross mnt
clones on xfs it would have been in the wild for a long time already.

OTOH, overlayfs doesn't support nfs/cifs/ocfs2 as upper fs.

If you mount an overlay with lower and upper layer on the same
xfs/btrfs sb the original mnt of lower path and upper patch is irrelevant.

Overlayfs uses different private mnt per layer anyway, so if the source
file is on lower layer then even if originally the overlay mount was done
with different upper/lower mounts of the same sb, clone via overlayfs
would work.

Allowing cross overlayfs mount clone makes very little difference.

Thanks,
Amir.