Message ID | 20180206130247.61882-1-bfoster@redhat.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Tue, Feb 06, 2018 at 08:02:47AM -0500, Brian Foster wrote: > Extent swap is a low level mechanism exported by XFS to facilitate > filesystem defragmentation. It is typically invoked by xfs_fsr under > conditions that will atomically adjust inode extent state without > loss of file data. > > xfs_fsr does not provide the necessary controls for low-level > testing of the extent swap command, however. For example, xfs_fsr > implements policies that dictate when to perform an extent swap and > offers no control over the donor file characteristics. > > Add a basic swapext command to xfs_io that allows userspace > invocation of the command under more controlled conditions. This > command makes no effort to retain data across the operation. As > such, it is primarily for testing purposes (i.e., in support of > xfstests, etc.). > > Signed-off-by: Brian Foster <bfoster@redhat.com> > --- > > This command is in support of an xfstests test to be posted shortly... > > Brian > > io/Makefile | 3 +- > io/init.c | 1 + > io/io.h | 1 + > io/swapext.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 111 insertions(+), 1 deletion(-) > create mode 100644 io/swapext.c > > diff --git a/io/Makefile b/io/Makefile > index 6725936d..2f9249e8 100644 > --- a/io/Makefile > +++ b/io/Makefile > @@ -11,7 +11,8 @@ HFILES = init.h io.h > CFILES = init.c \ > attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \ > getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \ > - pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c > + pwrite.c reflink.c seek.c shutdown.c stat.c swapext.c sync.c \ > + truncate.c utimes.c > > LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD) > LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE) > diff --git a/io/init.c b/io/init.c > index 20d5f80d..2ade03f4 100644 > --- a/io/init.c > +++ b/io/init.c > @@ -88,6 +88,7 @@ init_commands(void) > sendfile_init(); > shutdown_init(); > stat_init(); > + swapext_init(); > sync_init(); > sync_range_init(); > truncate_init(); > diff --git a/io/io.h b/io/io.h > index 3862985f..258311f1 100644 > --- a/io/io.h > +++ b/io/io.h > @@ -118,6 +118,7 @@ extern void quit_init(void); > extern void seek_init(void); > extern void shutdown_init(void); > extern void stat_init(void); > +extern void swapext_init(void); > extern void sync_init(void); > extern void truncate_init(void); > extern void utimes_init(void); > diff --git a/io/swapext.c b/io/swapext.c > new file mode 100644 > index 00000000..5e161d69 > --- /dev/null > +++ b/io/swapext.c > @@ -0,0 +1,107 @@ > +/* > + * Copyright (c) 2018 Red Hat, Inc. > + * All Rights Reserved. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it would be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write the Free Software Foundation, > + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include "command.h" > +#include "input.h" > +#include "init.h" > +#include "io.h" > + > +static cmdinfo_t swapext_cmd; > + > +static void > +swapext_help(void) > +{ > + printf(_( > +"\n" > +" Swaps extents between the open file descriptor and the supplied filename.\n" An update to the xfs_io manpage is needed, but this otherwise looks ok to me. --D > +"\n")); > +} > + > +static int > +xfs_bulkstat_single( > + int fd, > + xfs_ino_t *lastip, > + struct xfs_bstat *ubuffer) > +{ > + struct xfs_fsop_bulkreq bulkreq; > + > + bulkreq.lastip = (__u64 *)lastip; > + bulkreq.icount = 1; > + bulkreq.ubuffer = ubuffer; > + bulkreq.ocount = NULL; > + return ioctl(fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq); > +} > + > +static int > +swapext_f( > + int argc, > + char **argv) > +{ > + int fd; > + int error; > + struct xfs_swapext sx; > + struct stat stat; > + > + /* open the donor file */ > + fd = openfile(argv[1], NULL, 0, 0, NULL); > + if (fd < 0) > + return 0; > + > + /* > + * stat the target file to get the inode number and use the latter to > + * get the bulkstat info for the swapext cmd. > + */ > + error = fstat(file->fd, &stat); > + if (error) { > + perror("fstat"); > + goto out; > + } > + > + error = xfs_bulkstat_single(file->fd, &stat.st_ino, &sx.sx_stat); > + if (error) { > + perror("bulkstat"); > + goto out; > + } > + sx.sx_version = XFS_SX_VERSION; > + sx.sx_fdtarget = file->fd; > + sx.sx_fdtmp = fd; > + sx.sx_offset = 0; > + sx.sx_length = stat.st_size; > + error = ioctl(file->fd, XFS_IOC_SWAPEXT, &sx); > + if (error) > + perror("swapext"); > + > +out: > + close(fd); > + return 0; > +} > + > +void > +swapext_init(void) > +{ > + swapext_cmd.name = "swapext"; > + swapext_cmd.cfunc = swapext_f; > + swapext_cmd.argmin = 1; > + swapext_cmd.argmax = 1; > + swapext_cmd.flags = CMD_NOMAP_OK; > + swapext_cmd.args = _("<donorfile>"); > + swapext_cmd.oneline = _("Swap extents between files."); > + swapext_cmd.help = swapext_help; > + > + add_command(&swapext_cmd); > +} > -- > 2.13.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 06, 2018 at 09:32:13AM -0800, Darrick J. Wong wrote: > On Tue, Feb 06, 2018 at 08:02:47AM -0500, Brian Foster wrote: > > Extent swap is a low level mechanism exported by XFS to facilitate > > filesystem defragmentation. It is typically invoked by xfs_fsr under > > conditions that will atomically adjust inode extent state without > > loss of file data. > > > > xfs_fsr does not provide the necessary controls for low-level > > testing of the extent swap command, however. For example, xfs_fsr > > implements policies that dictate when to perform an extent swap and > > offers no control over the donor file characteristics. > > > > Add a basic swapext command to xfs_io that allows userspace > > invocation of the command under more controlled conditions. This > > command makes no effort to retain data across the operation. As > > such, it is primarily for testing purposes (i.e., in support of > > xfstests, etc.). > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > --- > > > > This command is in support of an xfstests test to be posted shortly... > > > > Brian > > > > io/Makefile | 3 +- > > io/init.c | 1 + > > io/io.h | 1 + > > io/swapext.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 111 insertions(+), 1 deletion(-) > > create mode 100644 io/swapext.c > > > > diff --git a/io/Makefile b/io/Makefile > > index 6725936d..2f9249e8 100644 > > --- a/io/Makefile > > +++ b/io/Makefile > > @@ -11,7 +11,8 @@ HFILES = init.h io.h > > CFILES = init.c \ > > attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \ > > getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \ > > - pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c > > + pwrite.c reflink.c seek.c shutdown.c stat.c swapext.c sync.c \ > > + truncate.c utimes.c > > > > LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD) > > LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE) > > diff --git a/io/init.c b/io/init.c > > index 20d5f80d..2ade03f4 100644 > > --- a/io/init.c > > +++ b/io/init.c > > @@ -88,6 +88,7 @@ init_commands(void) > > sendfile_init(); > > shutdown_init(); > > stat_init(); > > + swapext_init(); > > sync_init(); > > sync_range_init(); > > truncate_init(); > > diff --git a/io/io.h b/io/io.h > > index 3862985f..258311f1 100644 > > --- a/io/io.h > > +++ b/io/io.h > > @@ -118,6 +118,7 @@ extern void quit_init(void); > > extern void seek_init(void); > > extern void shutdown_init(void); > > extern void stat_init(void); > > +extern void swapext_init(void); > > extern void sync_init(void); > > extern void truncate_init(void); > > extern void utimes_init(void); > > diff --git a/io/swapext.c b/io/swapext.c > > new file mode 100644 > > index 00000000..5e161d69 > > --- /dev/null > > +++ b/io/swapext.c > > @@ -0,0 +1,107 @@ > > +/* > > + * Copyright (c) 2018 Red Hat, Inc. > > + * All Rights Reserved. > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it would be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program; if not, write the Free Software Foundation, > > + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > > + */ > > + > > +#include "command.h" > > +#include "input.h" > > +#include "init.h" > > +#include "io.h" > > + > > +static cmdinfo_t swapext_cmd; > > + > > +static void > > +swapext_help(void) > > +{ > > + printf(_( > > +"\n" > > +" Swaps extents between the open file descriptor and the supplied filename.\n" > > An update to the xfs_io manpage is needed, but this otherwise looks ok to me. > Ok, will fix.. Brian > --D > > > +"\n")); > > +} > > + > > +static int > > +xfs_bulkstat_single( > > + int fd, > > + xfs_ino_t *lastip, > > + struct xfs_bstat *ubuffer) > > +{ > > + struct xfs_fsop_bulkreq bulkreq; > > + > > + bulkreq.lastip = (__u64 *)lastip; > > + bulkreq.icount = 1; > > + bulkreq.ubuffer = ubuffer; > > + bulkreq.ocount = NULL; > > + return ioctl(fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq); > > +} > > + > > +static int > > +swapext_f( > > + int argc, > > + char **argv) > > +{ > > + int fd; > > + int error; > > + struct xfs_swapext sx; > > + struct stat stat; > > + > > + /* open the donor file */ > > + fd = openfile(argv[1], NULL, 0, 0, NULL); > > + if (fd < 0) > > + return 0; > > + > > + /* > > + * stat the target file to get the inode number and use the latter to > > + * get the bulkstat info for the swapext cmd. > > + */ > > + error = fstat(file->fd, &stat); > > + if (error) { > > + perror("fstat"); > > + goto out; > > + } > > + > > + error = xfs_bulkstat_single(file->fd, &stat.st_ino, &sx.sx_stat); > > + if (error) { > > + perror("bulkstat"); > > + goto out; > > + } > > + sx.sx_version = XFS_SX_VERSION; > > + sx.sx_fdtarget = file->fd; > > + sx.sx_fdtmp = fd; > > + sx.sx_offset = 0; > > + sx.sx_length = stat.st_size; > > + error = ioctl(file->fd, XFS_IOC_SWAPEXT, &sx); > > + if (error) > > + perror("swapext"); > > + > > +out: > > + close(fd); > > + return 0; > > +} > > + > > +void > > +swapext_init(void) > > +{ > > + swapext_cmd.name = "swapext"; > > + swapext_cmd.cfunc = swapext_f; > > + swapext_cmd.argmin = 1; > > + swapext_cmd.argmax = 1; > > + swapext_cmd.flags = CMD_NOMAP_OK; > > + swapext_cmd.args = _("<donorfile>"); > > + swapext_cmd.oneline = _("Swap extents between files."); > > + swapext_cmd.help = swapext_help; > > + > > + add_command(&swapext_cmd); > > +} > > -- > > 2.13.6 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 06, 2018 at 08:02:47AM -0500, Brian Foster wrote: > Extent swap is a low level mechanism exported by XFS to facilitate > filesystem defragmentation. It is typically invoked by xfs_fsr under > conditions that will atomically adjust inode extent state without > loss of file data. > > xfs_fsr does not provide the necessary controls for low-level > testing of the extent swap command, however. For example, xfs_fsr > implements policies that dictate when to perform an extent swap and > offers no control over the donor file characteristics. Hmmmm. I'm confused - we already use xfs_fsr for low level testing of swapext functionality with carefully constructed donor file characteristics in xfs/227. What am I missing here? I don't see any problem with adding a specific command to run more controlled swapext tests, i just want to make sure I understand what xfs_fsr is not providing you with. Cheers, Dave.
On Wed, Feb 07, 2018 at 12:34:41PM +1100, Dave Chinner wrote: > On Tue, Feb 06, 2018 at 08:02:47AM -0500, Brian Foster wrote: > > Extent swap is a low level mechanism exported by XFS to facilitate > > filesystem defragmentation. It is typically invoked by xfs_fsr under > > conditions that will atomically adjust inode extent state without > > loss of file data. > > > > xfs_fsr does not provide the necessary controls for low-level > > testing of the extent swap command, however. For example, xfs_fsr > > implements policies that dictate when to perform an extent swap and > > offers no control over the donor file characteristics. > > Hmmmm. I'm confused - we already use xfs_fsr for low level testing > of swapext functionality with carefully constructed donor file > characteristics in xfs/227. > > What am I missing here? > AFAICT, xfs/227 does so by formatting the filesystem in a particular way and running fsr over it, so it wasn't immediately clear to me what you're referring to. Staring at it some more, it appears to be using the -C debug option which looks like it forces an extent count..? If that's the case, the test[1] that was posted to use this feature could probably be modified to do a similar thing via xfs_fsr. Giving it a quick test, it doesn't appear to do exactly what I want with larger extent counts, however. It seems to work with a few extents or so, then doesn't ever create more than 4 or 5 if that's what I ask for with the donor file. Hmm, I wonder if prealloc or something is getting in the way of whatever it's attempting to do. If I try a similar test using falloc instead of pwrite, it doesn't do the swap at all. :/ Note that xfs/227 uses counts 5-15, so that also makes me wonder whether that test is actually doing what is expected, but that's a separate question (and I haven't dug into any of this deeply enough). I could just be misunderstanding the option. > I don't see any problem with adding a specific command to run > more controlled swapext tests, i just want to make sure I understand > what xfs_fsr is not providing you with. > I wasn't aware of 'xfs_fsr -C', thanks for pointing it out. I just looked at the fsr help/manpage to see if there was any "input file" capability and missed it. Having given it a quick test, I still prefer the existing test because it is explicit and doesn't have to copy data or anything like that. IOW, the -C thing to me seems like an fsr unit test whereas what I really want is a swapext unit test. FWIW, I do already have a variant of the test that reproduces the problem in question using xfs_fsr (without -C). The donor file extent count thing isn't actually a hard requirement to reproduce, just an advantage I was trying to document in the commit log for this patch. With respect to the test, the predictable extent count thing is more for efficiency purposes because it allows the test to run having only done a one-time set up of two files (the rest is hole punching and extent swaps). So I had already abandoned using xfs_fsr in favor of the current test for similar reasons: it was significantly slower due to the additional work required to set up the fs, unnecessary file data copying involved, etc. So unless there's any objections, I can try to reword the commit log for this patch to make the advantages/differences more clear. Brian [1] https://marc.info/?l=fstests&m=151792263511472&w=2 > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 07, 2018 at 09:47:12AM -0500, Brian Foster wrote: > On Wed, Feb 07, 2018 at 12:34:41PM +1100, Dave Chinner wrote: > > On Tue, Feb 06, 2018 at 08:02:47AM -0500, Brian Foster wrote: > > > Extent swap is a low level mechanism exported by XFS to facilitate > > > filesystem defragmentation. It is typically invoked by xfs_fsr under > > > conditions that will atomically adjust inode extent state without > > > loss of file data. > > > > > > xfs_fsr does not provide the necessary controls for low-level > > > testing of the extent swap command, however. For example, xfs_fsr > > > implements policies that dictate when to perform an extent swap and > > > offers no control over the donor file characteristics. > > > > Hmmmm. I'm confused - we already use xfs_fsr for low level testing > > of swapext functionality with carefully constructed donor file > > characteristics in xfs/227. > > > > What am I missing here? > > > > AFAICT, xfs/227 does so by formatting the filesystem in a particular way > and running fsr over it, so it wasn't immediately clear to me what > you're referring to. Staring at it some more, it appears to be using the > -C debug option which looks like it forces an extent count..? If that's > the case, the test[1] that was posted to use this feature could probably > be modified to do a similar thing via xfs_fsr. > > Giving it a quick test, it doesn't appear to do exactly what I want with > larger extent counts, however. It seems to work with a few extents or > so, then doesn't ever create more than 4 or 5 if that's what I ask for > with the donor file. Hmm, I wonder if prealloc or something is getting > in the way of whatever it's attempting to do. If I try a similar test > using falloc instead of pwrite, it doesn't do the swap at all. :/ Yeah, that's because of the way xfs_fsr preserves unwritten extents across defragmentation. i.e. it assumes unwritten extents are a result of application level preallocation and so it's there intentionally and be preserved. When I wrote this yesterday, I hadn't see the test you'd posted to the fstests list so I didn't have that context. Now that I've seen it, and you've explained the problems, I can see why you wrote the simpler xfs_io test command.... [....] > So unless there's any objections, I can try to reword the commit log for > this patch to make the advantages/differences more clear. Yeah, I think that's all that is necessary - just expand on "xfs_fsr does not provide the necessary controls" a bit so I don't have to rely on my brain to remember this conversation. :P Cheers, Dave.
On Thu, Feb 08, 2018 at 10:02:17AM +1100, Dave Chinner wrote: > On Wed, Feb 07, 2018 at 09:47:12AM -0500, Brian Foster wrote: > > On Wed, Feb 07, 2018 at 12:34:41PM +1100, Dave Chinner wrote: > > > On Tue, Feb 06, 2018 at 08:02:47AM -0500, Brian Foster wrote: > > > > Extent swap is a low level mechanism exported by XFS to facilitate > > > > filesystem defragmentation. It is typically invoked by xfs_fsr under > > > > conditions that will atomically adjust inode extent state without > > > > loss of file data. > > > > > > > > xfs_fsr does not provide the necessary controls for low-level > > > > testing of the extent swap command, however. For example, xfs_fsr > > > > implements policies that dictate when to perform an extent swap and > > > > offers no control over the donor file characteristics. > > > > > > Hmmmm. I'm confused - we already use xfs_fsr for low level testing > > > of swapext functionality with carefully constructed donor file > > > characteristics in xfs/227. > > > > > > What am I missing here? > > > > > > > AFAICT, xfs/227 does so by formatting the filesystem in a particular way > > and running fsr over it, so it wasn't immediately clear to me what > > you're referring to. Staring at it some more, it appears to be using the > > -C debug option which looks like it forces an extent count..? If that's > > the case, the test[1] that was posted to use this feature could probably > > be modified to do a similar thing via xfs_fsr. > > > > Giving it a quick test, it doesn't appear to do exactly what I want with > > larger extent counts, however. It seems to work with a few extents or > > so, then doesn't ever create more than 4 or 5 if that's what I ask for > > with the donor file. Hmm, I wonder if prealloc or something is getting > > in the way of whatever it's attempting to do. If I try a similar test > > using falloc instead of pwrite, it doesn't do the swap at all. :/ > > Yeah, that's because of the way xfs_fsr preserves unwritten extents > across defragmentation. i.e. it assumes unwritten extents are a > result of application level preallocation and so it's there > intentionally and be preserved. > > When I wrote this yesterday, I hadn't see the test you'd posted to > the fstests list so I didn't have that context. Now that I've seen > it, and you've explained the problems, I can see why you wrote the > simpler xfs_io test command.... > > [....] > > > So unless there's any objections, I can try to reword the commit log for > > this patch to make the advantages/differences more clear. > > Yeah, I think that's all that is necessary - just expand on > "xfs_fsr does not provide the necessary controls" a bit so I don't > have to rely on my brain to remember this conversation. :P <shrug> I had figured xfs_fsr == "polished user program not to be cluttered up with weird xfstests warts" and xfs_io == "warty program we use to make raw calls for testing" so except for tests that check the behavior of xfs_fsr itself, there ought to be a raw interface via xfs_io that regression tests use directly. :) --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 07, 2018 at 03:23:19PM -0800, Darrick J. Wong wrote: > On Thu, Feb 08, 2018 at 10:02:17AM +1100, Dave Chinner wrote: > > On Wed, Feb 07, 2018 at 09:47:12AM -0500, Brian Foster wrote: > > > On Wed, Feb 07, 2018 at 12:34:41PM +1100, Dave Chinner wrote: > > > > On Tue, Feb 06, 2018 at 08:02:47AM -0500, Brian Foster wrote: > > > > > Extent swap is a low level mechanism exported by XFS to facilitate > > > > > filesystem defragmentation. It is typically invoked by xfs_fsr under > > > > > conditions that will atomically adjust inode extent state without > > > > > loss of file data. > > > > > > > > > > xfs_fsr does not provide the necessary controls for low-level > > > > > testing of the extent swap command, however. For example, xfs_fsr > > > > > implements policies that dictate when to perform an extent swap and > > > > > offers no control over the donor file characteristics. > > > > > > > > Hmmmm. I'm confused - we already use xfs_fsr for low level testing > > > > of swapext functionality with carefully constructed donor file > > > > characteristics in xfs/227. > > > > > > > > What am I missing here? > > > > > > > > > > AFAICT, xfs/227 does so by formatting the filesystem in a particular way > > > and running fsr over it, so it wasn't immediately clear to me what > > > you're referring to. Staring at it some more, it appears to be using the > > > -C debug option which looks like it forces an extent count..? If that's > > > the case, the test[1] that was posted to use this feature could probably > > > be modified to do a similar thing via xfs_fsr. > > > > > > Giving it a quick test, it doesn't appear to do exactly what I want with > > > larger extent counts, however. It seems to work with a few extents or > > > so, then doesn't ever create more than 4 or 5 if that's what I ask for > > > with the donor file. Hmm, I wonder if prealloc or something is getting > > > in the way of whatever it's attempting to do. If I try a similar test > > > using falloc instead of pwrite, it doesn't do the swap at all. :/ > > > > Yeah, that's because of the way xfs_fsr preserves unwritten extents > > across defragmentation. i.e. it assumes unwritten extents are a > > result of application level preallocation and so it's there > > intentionally and be preserved. > > > > When I wrote this yesterday, I hadn't see the test you'd posted to > > the fstests list so I didn't have that context. Now that I've seen > > it, and you've explained the problems, I can see why you wrote the > > simpler xfs_io test command.... > > > > [....] > > > > > So unless there's any objections, I can try to reword the commit log for > > > this patch to make the advantages/differences more clear. > > > > Yeah, I think that's all that is necessary - just expand on > > "xfs_fsr does not provide the necessary controls" a bit so I don't > > have to rely on my brain to remember this conversation. :P > > <shrug> I had figured xfs_fsr == "polished user program not to be > cluttered up with weird xfstests warts" and xfs_io == "warty program we > use to make raw calls for testing" so except for tests that check the > behavior of xfs_fsr itself, there ought to be a raw interface via xfs_io > that regression tests use directly. :) > Yep. I thought about adding an input file param or some such to xfs_fsr for about 3 seconds or so, then came to the same conclusion. ;P Brian > --D > > > Cheers, > > > > Dave. > > -- > > Dave Chinner > > david@fromorbit.com > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/io/Makefile b/io/Makefile index 6725936d..2f9249e8 100644 --- a/io/Makefile +++ b/io/Makefile @@ -11,7 +11,8 @@ HFILES = init.h io.h CFILES = init.c \ attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \ getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \ - pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c + pwrite.c reflink.c seek.c shutdown.c stat.c swapext.c sync.c \ + truncate.c utimes.c LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD) LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE) diff --git a/io/init.c b/io/init.c index 20d5f80d..2ade03f4 100644 --- a/io/init.c +++ b/io/init.c @@ -88,6 +88,7 @@ init_commands(void) sendfile_init(); shutdown_init(); stat_init(); + swapext_init(); sync_init(); sync_range_init(); truncate_init(); diff --git a/io/io.h b/io/io.h index 3862985f..258311f1 100644 --- a/io/io.h +++ b/io/io.h @@ -118,6 +118,7 @@ extern void quit_init(void); extern void seek_init(void); extern void shutdown_init(void); extern void stat_init(void); +extern void swapext_init(void); extern void sync_init(void); extern void truncate_init(void); extern void utimes_init(void); diff --git a/io/swapext.c b/io/swapext.c new file mode 100644 index 00000000..5e161d69 --- /dev/null +++ b/io/swapext.c @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2018 Red Hat, Inc. + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it would be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "command.h" +#include "input.h" +#include "init.h" +#include "io.h" + +static cmdinfo_t swapext_cmd; + +static void +swapext_help(void) +{ + printf(_( +"\n" +" Swaps extents between the open file descriptor and the supplied filename.\n" +"\n")); +} + +static int +xfs_bulkstat_single( + int fd, + xfs_ino_t *lastip, + struct xfs_bstat *ubuffer) +{ + struct xfs_fsop_bulkreq bulkreq; + + bulkreq.lastip = (__u64 *)lastip; + bulkreq.icount = 1; + bulkreq.ubuffer = ubuffer; + bulkreq.ocount = NULL; + return ioctl(fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq); +} + +static int +swapext_f( + int argc, + char **argv) +{ + int fd; + int error; + struct xfs_swapext sx; + struct stat stat; + + /* open the donor file */ + fd = openfile(argv[1], NULL, 0, 0, NULL); + if (fd < 0) + return 0; + + /* + * stat the target file to get the inode number and use the latter to + * get the bulkstat info for the swapext cmd. + */ + error = fstat(file->fd, &stat); + if (error) { + perror("fstat"); + goto out; + } + + error = xfs_bulkstat_single(file->fd, &stat.st_ino, &sx.sx_stat); + if (error) { + perror("bulkstat"); + goto out; + } + sx.sx_version = XFS_SX_VERSION; + sx.sx_fdtarget = file->fd; + sx.sx_fdtmp = fd; + sx.sx_offset = 0; + sx.sx_length = stat.st_size; + error = ioctl(file->fd, XFS_IOC_SWAPEXT, &sx); + if (error) + perror("swapext"); + +out: + close(fd); + return 0; +} + +void +swapext_init(void) +{ + swapext_cmd.name = "swapext"; + swapext_cmd.cfunc = swapext_f; + swapext_cmd.argmin = 1; + swapext_cmd.argmax = 1; + swapext_cmd.flags = CMD_NOMAP_OK; + swapext_cmd.args = _("<donorfile>"); + swapext_cmd.oneline = _("Swap extents between files."); + swapext_cmd.help = swapext_help; + + add_command(&swapext_cmd); +}
Extent swap is a low level mechanism exported by XFS to facilitate filesystem defragmentation. It is typically invoked by xfs_fsr under conditions that will atomically adjust inode extent state without loss of file data. xfs_fsr does not provide the necessary controls for low-level testing of the extent swap command, however. For example, xfs_fsr implements policies that dictate when to perform an extent swap and offers no control over the donor file characteristics. Add a basic swapext command to xfs_io that allows userspace invocation of the command under more controlled conditions. This command makes no effort to retain data across the operation. As such, it is primarily for testing purposes (i.e., in support of xfstests, etc.). Signed-off-by: Brian Foster <bfoster@redhat.com> --- This command is in support of an xfstests test to be posted shortly... Brian io/Makefile | 3 +- io/init.c | 1 + io/io.h | 1 + io/swapext.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 io/swapext.c