On Thu, Jan 17, 2019 at 5:05 PM Verma, Vishal L <[email protected]> wrote: > > Hi Dan, > > These mostly look good, just a few minor comments below - > > On Mon, 2019-01-14 at 20:24 -0800, Dan Williams wrote: > > The kernel is implementing a '/sys/bus/dax' ABI to allow for alternate > > device-DAX drivers to be bound to device instances. While the kernel > > conversion to '/sys/bus/dax' does not effect the primary ndctl use case > > "kernel's conversion" or "kernel converting to" > s/effect/affect/ > > > of putting namespaces into 'devdax' mode since that uses libnvdimm > > namespace device relative paths, it does break current implementations > > of 'ndctl list -X' and 'daxctl list'. It is also known to break fio and > > some pmdk versions that explicitly reference "/sys/class/dax". > > > > In order to avoid userspace regressions the kernel can be configured to > > maintain '/sys/class/dax' as the default ABI. However, once all > > '/sys/class/dax' users have been converted, or removed from the > > installation, an administrator can opt-in to the new '/sys/bus/dax' ABI. > > The 'dax migrate-device-model' command installs a modprobe rule to > > blacklist the dax_pmem_compat module and arrange for the dax_pmem module > > to auto-load in response to the detection of device-DAX instances > > emitted from the libnvdimm subsystem. > > > > Reported-by: Jeff Moyer <[email protected]> > > Signed-off-by: Dan Williams <[email protected]> > > --- > > .gitignore | 1 > > Documentation/daxctl/Makefile.am | 3 + > > .../daxctl/daxctl-migrate-device-model.txt | 47 > > ++++++++++++++++++++ > > configure.ac | 5 ++ > > daxctl/Makefile.am | 10 ++++ > > daxctl/builtin.h | 1 > > daxctl/daxctl.c | 1 > > daxctl/lib/Makefile.am | 2 + > > daxctl/lib/daxctl.conf | 2 + > > daxctl/migrate.c | 41 +++++++++++++++++ > > ndctl.spec.in | 1 > > 11 files changed, 113 insertions(+), 1 deletion(-) > > create mode 100644 Documentation/daxctl/daxctl-migrate-device-model.txt > > create mode 100644 daxctl/lib/daxctl.conf > > create mode 100644 daxctl/migrate.c > > > > [..] > > > --- /dev/null > > +++ b/daxctl/migrate.c > > @@ -0,0 +1,41 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* Copyright(c) 2015-2018 Intel Corporation. All rights reserved. */ > > Copyright notice can be updated to 2019. This applies the one included > by man pages as well, but that is unrelated and I can take care of that > separately. > > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <stdio.h> > > +#include <errno.h> > > +#include <fcntl.h> > > +#include <daxctl/config.h> > > +#include <daxctl/libdaxctl.h> > > +#include <util/parse-options.h> > > +#include <ccan/array_size/array_size.h> > > + > > +int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx) > > +{ > > + int i; > > + static const struct option options[] = { > > + OPT_END(), > > + }; > > + const char * const u[] = { > > + "daxctl migrate-device-model", > > + NULL > > + }; > > + > > + argc = parse_options(argc, argv, options, u, 0); > > + for (i = 0; i < argc; i++) > > + error("unknown parameter \"%s\"\n", argv[i]); > > + > > + if (argc) > > + usage_with_options(u, options); > > + > > + if (symlink(DAXCTL_MODPROBE_DATA, DAXCTL_MODPROBE_INSTALL) == 0) { > > + fprintf(stderr, " Success: installed %s\n", > > + DAXCTL_MODPROBE_INSTALL); > > + return EXIT_SUCCESS; > > + } > > + > > + error("failed to install %s: %s\n", DAXCTL_MODPROBE_INSTALL, > > + strerror(errno)); > > 'Success' is capitalized, but 'failed' is lower case, could make them > consistent. Most other messages seem to be lowercase, including the > 'unknown parameter' one above. >
Good point, I'll respin with the copyright and changelog fixups. Thanks! _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
