| Message ID | 20171117202524.24696-3-ross.zwisler@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 11/17/17 2:25 PM, Ross Zwisler wrote: > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes > log marks via the external 'dmsetup' executable. It's helpful to allow > users of xfs_io to adds these marks from within xfs_io instead of waiting > until after xfs_io exits because then they are able to replay the > dm-log-writes log up to immediately after another xfs_io operation such as > mwrite. This isolates the log replay from other operations that happen as > part of xfs_io exiting (file handles being closed, mmaps being torn down, > etc.). This also allows users to insert multiple marks between different > xfs_io commands. > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > Suggested-by: Dave Chinner <david@fromorbit.com> Without reviewing in detail, what is the advantage of wrapping dmsetup into xfs_io? My first inclination is that there is none at all, and xfstests can call dmsetup as easily as they can call xfs_io. No? -Eric -- 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 Fri, Nov 17, 2017 at 01:25:24PM -0700, Ross Zwisler wrote: > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes > log marks via the external 'dmsetup' executable. It's helpful to allow > users of xfs_io to adds these marks from within xfs_io instead of waiting > until after xfs_io exits because then they are able to replay the > dm-log-writes log up to immediately after another xfs_io operation such as > mwrite. This isolates the log replay from other operations that happen as > part of xfs_io exiting (file handles being closed, mmaps being torn down, > etc.). This also allows users to insert multiple marks between different > xfs_io commands. > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > Suggested-by: Dave Chinner <david@fromorbit.com> > --- > io/Makefile | 5 ++-- > io/init.c | 1 + > io/io.h | 1 + > io/log_writes.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > man/man8/xfs_io.8 | 19 ++++++++++++++ > 5 files changed, 102 insertions(+), 2 deletions(-) > create mode 100644 io/log_writes.c > > diff --git a/io/Makefile b/io/Makefile > index 050d6bd..51b2eae 100644 > --- a/io/Makefile > +++ b/io/Makefile > @@ -10,8 +10,9 @@ LSRCFILES = xfs_bmap.sh xfs_freeze.sh xfs_mkfile.sh > 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 > + getrusage.c imap.c link.c log_writes.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 > > LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD) > LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE) > diff --git a/io/init.c b/io/init.c > index 20d5f80..34d87b5 100644 > --- a/io/init.c > +++ b/io/init.c > @@ -72,6 +72,7 @@ init_commands(void) > help_init(); > imap_init(); > inject_init(); > + log_writes_init(); > madvise_init(); > mincore_init(); > mmap_init(); > diff --git a/io/io.h b/io/io.h > index 8b2753b..d62034a 100644 > --- a/io/io.h > +++ b/io/io.h > @@ -109,6 +109,7 @@ extern void getrusage_init(void); > extern void help_init(void); > extern void imap_init(void); > extern void inject_init(void); > +extern void log_writes_init(void); > extern void mmap_init(void); > extern void open_init(void); > extern void parent_init(void); > diff --git a/io/log_writes.c b/io/log_writes.c > new file mode 100644 > index 0000000..bc3952c > --- /dev/null > +++ b/io/log_writes.c > @@ -0,0 +1,78 @@ > +/* > + * Copyright (c) 2017 Intel Corporation. > + * 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 "platform_defs.h" > +#include "command.h" > +#include "init.h" > + > +static cmdinfo_t log_writes_cmd; > + > +static int > +mark_log(char *device, char *mark) > +{ > + char command[256]; > + > + snprintf(command, 256, "dmsetup message %s 0 mark %s", > + device, mark); > + > + return system(command); I want to call my mark "bobby'; DROP TABLES;"... ;) Maybe we should check snprintf return value just in case someone feeds us an overlong string? --D > +} > + > +static int > +log_writes_f( > + int argc, > + char **argv) > +{ > + char *device = NULL; > + char *mark = NULL; > + int c; > + > + while ((c = getopt(argc, argv, "d:m:")) != EOF) { > + switch (c) { > + case 'd': > + device = optarg; > + break; > + case 'm': > + mark = optarg; > + break; > + default: > + return command_usage(&log_writes_cmd); > + } > + } > + > + if (device == NULL || mark == NULL) > + return command_usage(&log_writes_cmd); > + > + return mark_log(device, mark); > +} > + > +void > +log_writes_init(void) > +{ > + log_writes_cmd.name = "log_writes"; > + log_writes_cmd.altname = "lw"; > + log_writes_cmd.cfunc = log_writes_f; > + log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK; > + log_writes_cmd.argmin = 0; > + log_writes_cmd.argmax = -1; > + log_writes_cmd.args = _("-d device -m mark"); > + log_writes_cmd.oneline = > + _("uses dmsetup to interact with the dm-log-writes module"); > + > + add_command(&log_writes_cmd); > +} > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 > index 1693f7f..f18af99 100644 > --- a/man/man8/xfs_io.8 > +++ b/man/man8/xfs_io.8 > @@ -1123,7 +1123,25 @@ version of policy structure (numeric) > .BR get_encpolicy > On filesystems that support encryption, display the encryption policy of the > current file. > +.RE > +.PD > +.TP > +.BI "log_writes \-d " device " \-m " mark > +Use > +.B dmsetup > +to interact with the > +.B dm-log-writes > +kernel module. Currently the only operation > +supported is the creation of a mark in the log by executing the shell command: > > +.B dmsetup message <device> 0 mark <mark> > +.PD > +.RE > +.TP > +.B lw > +See the > +.B log_writes > +command. > .SH SEE ALSO > .BR mkfs.xfs (8), > .BR xfsctl (3), > @@ -1142,3 +1160,4 @@ current file. > .BR pread (2), > .BR pwrite (2), > .BR readdir (3). > +.BR dmsetup (8). > -- > 2.9.5 > > -- > 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 Fri, Nov 17, 2017 at 02:39:07PM -0600, Eric Sandeen wrote: > On 11/17/17 2:25 PM, Ross Zwisler wrote: > > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes > > log marks via the external 'dmsetup' executable. It's helpful to allow > > users of xfs_io to adds these marks from within xfs_io instead of waiting > > until after xfs_io exits because then they are able to replay the > > dm-log-writes log up to immediately after another xfs_io operation such as > > mwrite. This isolates the log replay from other operations that happen as > > part of xfs_io exiting (file handles being closed, mmaps being torn down, > > etc.). This also allows users to insert multiple marks between different > > xfs_io commands. > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > Suggested-by: Dave Chinner <david@fromorbit.com> > > Without reviewing in detail, what is the advantage of wrapping dmsetup > into xfs_io? My first inclination is that there is none at all, and > xfstests can call dmsetup as easily as they can call xfs_io. No? > > -Eric I commented on this a bit in the changelog for the 2nd patch: It's helpful to allow users of xfs_io to adds these marks from within xfs_io instead of waiting until after xfs_io exits because then they are able to replay the dm-log-writes log up to immediately after another xfs_io operation such as mwrite. This isolates the log replay from other operations that happen as part of xfs_io exiting (file handles being closed, mmaps being torn down, etc.). This also allows users to insert multiple marks between different xfs_io commands. I agree that the shell-out to dmsetup isn't awesome... For the current test I have written I think we can get away with just assuming that the xfs_io exit stuff won't interact too heavily with the dm-log-writes log, and we could potentially move the dmsetup call back into the fstest. This is how I initially had it, and moved it into the C program via shell-out in response to Amir's feedback: https://lists.01.org/pipermail/linux-nvdimm/2017-October/012976.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 11/17/17 2:48 PM, Ross Zwisler wrote: > On Fri, Nov 17, 2017 at 02:39:07PM -0600, Eric Sandeen wrote: >> On 11/17/17 2:25 PM, Ross Zwisler wrote: >>> Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes >>> log marks via the external 'dmsetup' executable. It's helpful to allow >>> users of xfs_io to adds these marks from within xfs_io instead of waiting >>> until after xfs_io exits because then they are able to replay the >>> dm-log-writes log up to immediately after another xfs_io operation such as >>> mwrite. This isolates the log replay from other operations that happen as >>> part of xfs_io exiting (file handles being closed, mmaps being torn down, >>> etc.). This also allows users to insert multiple marks between different >>> xfs_io commands. >>> >>> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> >>> Suggested-by: Dave Chinner <david@fromorbit.com> >> >> Without reviewing in detail, what is the advantage of wrapping dmsetup >> into xfs_io? My first inclination is that there is none at all, and >> xfstests can call dmsetup as easily as they can call xfs_io. No? >> >> -Eric > > I commented on this a bit in the changelog for the 2nd patch: > > It's helpful to allow users of xfs_io to adds these marks from within xfs_io > instead of waiting until after xfs_io exits because then they are able to > replay the dm-log-writes log up to immediately after another xfs_io operation > such as mwrite. This isolates the log replay from other operations that > happen as part of xfs_io exiting (file handles being closed, mmaps being torn > down, etc.). This also allows users to insert multiple marks between > different xfs_io commands. > > I agree that the shell-out to dmsetup isn't awesome... For the current test I > have written I think we can get away with just assuming that the xfs_io exit > stuff won't interact too heavily with the dm-log-writes log, and we could > potentially move the dmsetup call back into the fstest. This is how I > initially had it, and moved it into the C program via shell-out in response to > Amir's feedback: Sorry, terrible of me to not have read that. :( Ok, so next question - DM_TARGET_MSG seems to be public, can we just invoke the ioctl directly instead of shelling out to dmsetup? I'm checking w/ the dm folks too, to make sure that's expected to work. As long as the use isn't too tricky it seems like that might be better. -Eric -- 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 Fri, Nov 17, 2017 at 03:03:39PM -0600, Eric Sandeen wrote: > On 11/17/17 2:48 PM, Ross Zwisler wrote: > > On Fri, Nov 17, 2017 at 02:39:07PM -0600, Eric Sandeen wrote: > >> On 11/17/17 2:25 PM, Ross Zwisler wrote: > >>> Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes > >>> log marks via the external 'dmsetup' executable. It's helpful to allow > >>> users of xfs_io to adds these marks from within xfs_io instead of waiting > >>> until after xfs_io exits because then they are able to replay the > >>> dm-log-writes log up to immediately after another xfs_io operation such as > >>> mwrite. This isolates the log replay from other operations that happen as > >>> part of xfs_io exiting (file handles being closed, mmaps being torn down, > >>> etc.). This also allows users to insert multiple marks between different > >>> xfs_io commands. > >>> > >>> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > >>> Suggested-by: Dave Chinner <david@fromorbit.com> > >> > >> Without reviewing in detail, what is the advantage of wrapping dmsetup > >> into xfs_io? My first inclination is that there is none at all, and > >> xfstests can call dmsetup as easily as they can call xfs_io. No? > >> > >> -Eric > > > > I commented on this a bit in the changelog for the 2nd patch: > > > > It's helpful to allow users of xfs_io to adds these marks from within xfs_io > > instead of waiting until after xfs_io exits because then they are able to > > replay the dm-log-writes log up to immediately after another xfs_io operation > > such as mwrite. This isolates the log replay from other operations that > > happen as part of xfs_io exiting (file handles being closed, mmaps being torn > > down, etc.). This also allows users to insert multiple marks between > > different xfs_io commands. > > > > I agree that the shell-out to dmsetup isn't awesome... For the current test I > > have written I think we can get away with just assuming that the xfs_io exit > > stuff won't interact too heavily with the dm-log-writes log, and we could > > potentially move the dmsetup call back into the fstest. This is how I > > initially had it, and moved it into the C program via shell-out in response to > > Amir's feedback: > > Sorry, terrible of me to not have read that. :( Ok, so next question - > DM_TARGET_MSG seems to be public, can we just invoke the ioctl directly > instead of shelling out to dmsetup? > > I'm checking w/ the dm folks too, to make sure that's expected to work. As > long as the use isn't too tricky it seems like that might be better. Yea, that seems like a better option - I'll take a look. Thanks for the suggestion. -- 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 11/17/17 3:14 PM, Ross Zwisler wrote: > On Fri, Nov 17, 2017 at 03:03:39PM -0600, Eric Sandeen wrote: >> On 11/17/17 2:48 PM, Ross Zwisler wrote: >>> On Fri, Nov 17, 2017 at 02:39:07PM -0600, Eric Sandeen wrote: >>>> On 11/17/17 2:25 PM, Ross Zwisler wrote: >>>>> Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes >>>>> log marks via the external 'dmsetup' executable. It's helpful to allow >>>>> users of xfs_io to adds these marks from within xfs_io instead of waiting >>>>> until after xfs_io exits because then they are able to replay the >>>>> dm-log-writes log up to immediately after another xfs_io operation such as >>>>> mwrite. This isolates the log replay from other operations that happen as >>>>> part of xfs_io exiting (file handles being closed, mmaps being torn down, >>>>> etc.). This also allows users to insert multiple marks between different >>>>> xfs_io commands. >>>>> >>>>> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> >>>>> Suggested-by: Dave Chinner <david@fromorbit.com> >>>> >>>> Without reviewing in detail, what is the advantage of wrapping dmsetup >>>> into xfs_io? My first inclination is that there is none at all, and >>>> xfstests can call dmsetup as easily as they can call xfs_io. No? >>>> >>>> -Eric >>> >>> I commented on this a bit in the changelog for the 2nd patch: >>> >>> It's helpful to allow users of xfs_io to adds these marks from within xfs_io >>> instead of waiting until after xfs_io exits because then they are able to >>> replay the dm-log-writes log up to immediately after another xfs_io operation >>> such as mwrite. This isolates the log replay from other operations that >>> happen as part of xfs_io exiting (file handles being closed, mmaps being torn >>> down, etc.). This also allows users to insert multiple marks between >>> different xfs_io commands. >>> >>> I agree that the shell-out to dmsetup isn't awesome... For the current test I >>> have written I think we can get away with just assuming that the xfs_io exit >>> stuff won't interact too heavily with the dm-log-writes log, and we could >>> potentially move the dmsetup call back into the fstest. This is how I >>> initially had it, and moved it into the C program via shell-out in response to >>> Amir's feedback: >> >> Sorry, terrible of me to not have read that. :( Ok, so next question - >> DM_TARGET_MSG seems to be public, can we just invoke the ioctl directly >> instead of shelling out to dmsetup? >> >> I'm checking w/ the dm folks too, to make sure that's expected to work. As >> long as the use isn't too tricky it seems like that might be better. > > Yea, that seems like a better option - I'll take a look. Thanks for the > suggestion. FWIW, agk is saying the only right way to do it is to use libdevicemapper ... sounds like they don't expect anyone to use the ioctl directly, though I'm not sure why, or what the complexity is. -Eric -- 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 050d6bd..51b2eae 100644 --- a/io/Makefile +++ b/io/Makefile @@ -10,8 +10,9 @@ LSRCFILES = xfs_bmap.sh xfs_freeze.sh xfs_mkfile.sh 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 + getrusage.c imap.c link.c log_writes.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 LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD) LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE) diff --git a/io/init.c b/io/init.c index 20d5f80..34d87b5 100644 --- a/io/init.c +++ b/io/init.c @@ -72,6 +72,7 @@ init_commands(void) help_init(); imap_init(); inject_init(); + log_writes_init(); madvise_init(); mincore_init(); mmap_init(); diff --git a/io/io.h b/io/io.h index 8b2753b..d62034a 100644 --- a/io/io.h +++ b/io/io.h @@ -109,6 +109,7 @@ extern void getrusage_init(void); extern void help_init(void); extern void imap_init(void); extern void inject_init(void); +extern void log_writes_init(void); extern void mmap_init(void); extern void open_init(void); extern void parent_init(void); diff --git a/io/log_writes.c b/io/log_writes.c new file mode 100644 index 0000000..bc3952c --- /dev/null +++ b/io/log_writes.c @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * 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 "platform_defs.h" +#include "command.h" +#include "init.h" + +static cmdinfo_t log_writes_cmd; + +static int +mark_log(char *device, char *mark) +{ + char command[256]; + + snprintf(command, 256, "dmsetup message %s 0 mark %s", + device, mark); + + return system(command); +} + +static int +log_writes_f( + int argc, + char **argv) +{ + char *device = NULL; + char *mark = NULL; + int c; + + while ((c = getopt(argc, argv, "d:m:")) != EOF) { + switch (c) { + case 'd': + device = optarg; + break; + case 'm': + mark = optarg; + break; + default: + return command_usage(&log_writes_cmd); + } + } + + if (device == NULL || mark == NULL) + return command_usage(&log_writes_cmd); + + return mark_log(device, mark); +} + +void +log_writes_init(void) +{ + log_writes_cmd.name = "log_writes"; + log_writes_cmd.altname = "lw"; + log_writes_cmd.cfunc = log_writes_f; + log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK; + log_writes_cmd.argmin = 0; + log_writes_cmd.argmax = -1; + log_writes_cmd.args = _("-d device -m mark"); + log_writes_cmd.oneline = + _("uses dmsetup to interact with the dm-log-writes module"); + + add_command(&log_writes_cmd); +} diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 index 1693f7f..f18af99 100644 --- a/man/man8/xfs_io.8 +++ b/man/man8/xfs_io.8 @@ -1123,7 +1123,25 @@ version of policy structure (numeric) .BR get_encpolicy On filesystems that support encryption, display the encryption policy of the current file. +.RE +.PD +.TP +.BI "log_writes \-d " device " \-m " mark +Use +.B dmsetup +to interact with the +.B dm-log-writes +kernel module. Currently the only operation +supported is the creation of a mark in the log by executing the shell command: +.B dmsetup message <device> 0 mark <mark> +.PD +.RE +.TP +.B lw +See the +.B log_writes +command. .SH SEE ALSO .BR mkfs.xfs (8), .BR xfsctl (3), @@ -1142,3 +1160,4 @@ current file. .BR pread (2), .BR pwrite (2), .BR readdir (3). +.BR dmsetup (8).
Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes log marks via the external 'dmsetup' executable. It's helpful to allow users of xfs_io to adds these marks from within xfs_io instead of waiting until after xfs_io exits because then they are able to replay the dm-log-writes log up to immediately after another xfs_io operation such as mwrite. This isolates the log replay from other operations that happen as part of xfs_io exiting (file handles being closed, mmaps being torn down, etc.). This also allows users to insert multiple marks between different xfs_io commands. Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> Suggested-by: Dave Chinner <david@fromorbit.com> --- io/Makefile | 5 ++-- io/init.c | 1 + io/io.h | 1 + io/log_writes.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ man/man8/xfs_io.8 | 19 ++++++++++++++ 5 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 io/log_writes.c