[0/2] vfs: move the clone/dedupe/remap helpers to a single file

Message

Darrick J. Wong Oct. 15, 2020, 12:31 a.m. UTC

Hi all,

I would like to move the generic helper functions that support the file
remap range operations (aka clone and dedupe) to a separate file under
fs/.  For the moment, I have a few goals here: one is to declutter
fs/read_write.c and mm/filemap.c.  The second goal is to be able to
deselect all the remap code if no filesystems require it.

The third (and much more long term) goal is to have a place to land the
generic code for the atomic file extent swap functionality, since it
will reuse some of the functionality.  Someday.  Whenever I get around
to submitting that again.

AFAICT, nobody is attempting to land any major changes in any of the vfs
remap functions during the 5.10 window -- for-next showed conflicts only
in the Makefile, so it seems like a quiet enough time to do this.  There
are no functional changes here, it's just moving code blocks around.

So, I have a few questions, particularly for Al, Andrew, and Linus:

(1) Do you find this reorganizing acceptable?

(2) I was planning to rebase this series next Friday and try to throw it
in at the end of the merge window; is that ok?  (The current patches are
based on 5.9, and applying them manually to current master and for-next
didn't show any new conflicts.)

(3) Can I just grab the copyrights from mm/filemap.c?  Or fs/read_write.c?
Or something entirely different?

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=vfs-rearrange-remap-helpers
---
 fs/Makefile        |    3 
 fs/read_write.c    |  473 -------------------------------------------
 fs/remap_range.c   |  577 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |    5 
 mm/filemap.c       |   81 -------
 5 files changed, 582 insertions(+), 557 deletions(-)
 create mode 100644 fs/remap_range.c

Comments

Linus Torvalds Oct. 15, 2020, 2:48 a.m. UTC | #1

On Wed, Oct 14, 2020 at 5:31 PM Darrick J. Wong <[e-mail]> wrote:
>
> So, I have a few questions, particularly for Al, Andrew, and Linus:
>
> (1) Do you find this reorganizing acceptable?

I don't see a problem.

> (3) Can I just grab the copyrights from mm/filemap.c?  Or fs/read_write.c?
> Or something entirely different?

Heh. Those copyright notices look pretty old, and I'm not sure how
much - if anything - I had to do with the remap helpers.

I suspect just a

   // SPDX-License-Identifier: GPL-2.0-only

is fine, with no need to try to drag along any other copyright notice
history from those two files.

               Linus

Al Viro Oct. 15, 2020, 3:18 a.m. UTC | #2

On Wed, Oct 14, 2020 at 05:31:14PM -0700, Darrick J. Wong wrote:

> AFAICT, nobody is attempting to land any major changes in any of the vfs
> remap functions during the 5.10 window -- for-next showed conflicts only
> in the Makefile, so it seems like a quiet enough time to do this.  There
> are no functional changes here, it's just moving code blocks around.
> 
> So, I have a few questions, particularly for Al, Andrew, and Linus:
> 
> (1) Do you find this reorganizing acceptable?

No objections, assuming that it's really a move (it's surprisingly easy to
screw that up - BTDT ;-/)

I have not done function-by-function comparison, but assuming it holds...
no problem.

> (2) I was planning to rebase this series next Friday and try to throw it
> in at the end of the merge window; is that ok?  (The current patches are
> based on 5.9, and applying them manually to current master and for-next
> didn't show any new conflicts.)

Up to Linus.  I don't have anything in vfs.git around that area; the
only remaining stuff touching fs/read_write.c is nowhere near those,
AFAICS.

Darrick J. Wong Oct. 15, 2020, 4:46 p.m. UTC | #3

On Thu, Oct 15, 2020 at 04:18:19AM +0100, Al Viro wrote:
> On Wed, Oct 14, 2020 at 05:31:14PM -0700, Darrick J. Wong wrote:
> 
> > AFAICT, nobody is attempting to land any major changes in any of the vfs
> > remap functions during the 5.10 window -- for-next showed conflicts only
> > in the Makefile, so it seems like a quiet enough time to do this.  There
> > are no functional changes here, it's just moving code blocks around.
> > 
> > So, I have a few questions, particularly for Al, Andrew, and Linus:
> > 
> > (1) Do you find this reorganizing acceptable?
> 
> No objections, assuming that it's really a move (it's surprisingly easy to
> screw that up - BTDT ;-/)
> 
> I have not done function-by-function comparison, but assuming it holds...
> no problem.

<nod> The only changes between before and after are that some of the
functions lose their static status, and some gain it; and a minor
indenting issue that I'll fix for the final patch series.

As far as makefiles go, both read_write.o and filemap.o are both obj-y
targets, so I think it's safe to make remap_range.o also an obj-y
target.  The fun part will be the careful Kconfig surgery to make
remap_range.o an optional build target, but that will come later.

> > (2) I was planning to rebase this series next Friday and try to throw it
> > in at the end of the merge window; is that ok?  (The current patches are
> > based on 5.9, and applying them manually to current master and for-next
> > didn't show any new conflicts.)
> 
> Up to Linus.  I don't have anything in vfs.git around that area; the
> only remaining stuff touching fs/read_write.c is nowhere near those,
> AFAICS.

<nod> Thanks. :)

--D