From: Jeff Moyer <jmoyer@redhat.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: dave.hansen@linux.intel.com, linux-nvdimm@lists.01.org
Subject: Re: [daxctl PATCH] daxctl: Opt-in to /sys/bus/dax ABI
Date: Thu, 08 Nov 2018 15:56:57 -0500 [thread overview]
Message-ID: <x49lg63cmp2.fsf@segfault.boston.devel.redhat.com> (raw)
In-Reply-To: <154161803120.1679566.5287706874058480752.stgit@dwillia2-desk3.amr.corp.intel.com> (Dan Williams's message of "Wed, 07 Nov 2018 11:13:51 -0800")
Dan Williams <dan.j.williams@intel.com> writes:
> In support of the kernel's conversion of the dax-subsystem from a
> 'class' to a 'bus', teach the libdaxctl subsystem-layout-specific code
> to parse the new layout. The kernel changes do not effect the primary
> ndctl use case of putting namespaces into 'dax' mode since that uses
> libnvdimm namespace device relative paths, but it does break 'ndctl list
> -X' and 'daxctl list'. For that reason the kernel provides a
> dax_pmem_compat driver to support the old layout and give time for
> userspace components to switch.
>
> Installation of the latest libdaxctl package arranges for a daxctl
> configuration file to be dropped in /etc/modprobe.d. The modprobe
> configuration blacklists dax_pmem_compat modules and sets the proper
> alias for the dax_pmem module. The modprobe configuration upgrades the
> default kernel handling to the /sys/bus/dax scheme.
OK, one other snafu: pmdk also uses the /sys/class/dax path (see
src/common/file.c). Since ndctl isn't the only consumer, we'd have to
coordinate the class->bus switch among all parties.
Is there a way to just keep the class-based paths working alongside the
bus interface?
-Jeff
>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> daxctl/lib/Makefile.am | 3 ++
> daxctl/lib/daxctl.conf | 2 +
> daxctl/lib/libdaxctl-private.h | 11 ++++++
> daxctl/lib/libdaxctl.c | 70 +++++++++++++++++++++++++++++-----------
> ndctl.spec.in | 1 +
> util/sysfs.c | 2 +
> 6 files changed, 68 insertions(+), 21 deletions(-)
> create mode 100644 daxctl/lib/daxctl.conf
>
> diff --git a/daxctl/lib/Makefile.am b/daxctl/lib/Makefile.am
> index 0167e3995b00..749d54a2e397 100644
> --- a/daxctl/lib/Makefile.am
> +++ b/daxctl/lib/Makefile.am
> @@ -18,6 +18,9 @@ libdaxctl_la_SOURCES =\
> libdaxctl_la_LIBADD =\
> $(UUID_LIBS)
>
> +daxctlconfdir=$(sysconfdir)/modprobe.d
> +daxctlconf_DATA = daxctl.conf
> +
> EXTRA_DIST += libdaxctl.sym
>
> libdaxctl_la_LDFLAGS = $(AM_LDFLAGS) \
> diff --git a/daxctl/lib/daxctl.conf b/daxctl/lib/daxctl.conf
> new file mode 100644
> index 000000000000..c64a088cbc0b
> --- /dev/null
> +++ b/daxctl/lib/daxctl.conf
> @@ -0,0 +1,2 @@
> +blacklist dax_pmem_compat
> +alias nd:t7* dax_pmem
> diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
> index f7667324026f..4a462e7245d2 100644
> --- a/daxctl/lib/libdaxctl-private.h
> +++ b/daxctl/lib/libdaxctl-private.h
> @@ -15,6 +15,17 @@
>
> #define DAXCTL_EXPORT __attribute__ ((visibility("default")))
>
> +enum dax_subsystem {
> + DAX_UNKNOWN,
> + DAX_CLASS,
> + DAX_BUS,
> +};
> +
> +static const char *dax_subsystems[] = {
> + [DAX_CLASS] = "/sys/class/dax",
> + [DAX_BUS] = "/sys/bus/dax/devices",
> +};
> +
> /**
> * struct daxctl_region - container for dax_devices
> */
> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
> index 22f4210a7ea0..c2e3a52d6c7c 100644
> --- a/daxctl/lib/libdaxctl.c
> +++ b/daxctl/lib/libdaxctl.c
> @@ -444,26 +444,38 @@ static void dax_devices_init(struct daxctl_region *region)
> {
> struct daxctl_ctx *ctx = daxctl_region_get_ctx(region);
> char daxdev_fmt[50];
> - char *region_path;
> + size_t i;
>
> if (region->devices_init)
> return;
>
> region->devices_init = 1;
> sprintf(daxdev_fmt, "dax%d.", region->id);
> - if (asprintf(®ion_path, "%s/dax", region->region_path) < 0) {
> - dbg(ctx, "region path alloc fail\n");
> - return;
> + for (i = 0; i < ARRAY_SIZE(dax_subsystems); i++) {
> + char *region_path;
> +
> + if (i == DAX_BUS)
> + region_path = region->region_path;
> + else if (i == DAX_CLASS) {
> + if (asprintf(®ion_path, "%s/dax",
> + region->region_path) < 0) {
> + dbg(ctx, "region path alloc fail\n");
> + continue;
> + }
> + } else
> + continue;
> + sysfs_device_parse(ctx, region_path, daxdev_fmt, region,
> + add_dax_dev);
> + if (i == DAX_CLASS)
> + free(region_path);
> }
> - sysfs_device_parse(ctx, region_path, daxdev_fmt, region, add_dax_dev);
> - free(region_path);
> }
>
> -static char *dax_region_path(const char *base, const char *device)
> +static char *dax_region_path(const char *device, enum dax_subsystem subsys)
> {
> char *path, *region_path, *c;
>
> - if (asprintf(&path, "%s/%s", base, device) < 0)
> + if (asprintf(&path, "%s/%s", dax_subsystems[subsys], device) < 0)
> return NULL;
>
> /* dax_region must be the instance's direct parent */
> @@ -472,7 +484,11 @@ static char *dax_region_path(const char *base, const char *device)
> if (!region_path)
> return NULL;
>
> - /* 'region_path' is now regionX/dax/daxX.Y', trim back to regionX */
> + /*
> + * 'region_path' is now regionX/dax/daxX.Y' (DAX_CLASS), or
> + * regionX/daxX.Y (DAX_BUS), trim it back to the regionX
> + * component
> + */
> c = strrchr(region_path, '/');
> if (!c) {
> free(region_path);
> @@ -480,6 +496,9 @@ static char *dax_region_path(const char *base, const char *device)
> }
> *c = '\0';
>
> + if (subsys == DAX_BUS)
> + return region_path;
> +
> c = strrchr(region_path, '/');
> if (!c) {
> free(region_path);
> @@ -490,20 +509,15 @@ static char *dax_region_path(const char *base, const char *device)
> return region_path;
> }
>
> -static void dax_regions_init(struct daxctl_ctx *ctx)
> +static void __dax_regions_init(struct daxctl_ctx *ctx, enum dax_subsystem subsys)
> {
> - const char *base = "/sys/class/dax";
> struct dirent *de;
> - DIR *dir;
> + DIR *dir = NULL;
>
> - if (ctx->regions_init)
> - return;
> -
> - ctx->regions_init = 1;
> -
> - dir = opendir(base);
> + dir = opendir(dax_subsystems[subsys]);
> if (!dir) {
> - dbg(ctx, "no dax regions found\n");
> + dbg(ctx, "no dax regions found via: %s\n",
> + dax_subsystems[subsys]);
> return;
> }
>
> @@ -516,7 +530,7 @@ static void dax_regions_init(struct daxctl_ctx *ctx)
> continue;
> if (sscanf(de->d_name, "dax%d.%d", ®ion_id, &id) != 2)
> continue;
> - dev_path = dax_region_path(base, de->d_name);
> + dev_path = dax_region_path(de->d_name, subsys);
> if (!dev_path) {
> err(ctx, "dax region path allocation failure\n");
> continue;
> @@ -529,6 +543,22 @@ static void dax_regions_init(struct daxctl_ctx *ctx)
> closedir(dir);
> }
>
> +static void dax_regions_init(struct daxctl_ctx *ctx)
> +{
> + size_t i;
> +
> + if (ctx->regions_init)
> + return;
> +
> + ctx->regions_init = 1;
> +
> + for (i = 0; i < ARRAY_SIZE(dax_subsystems); i++) {
> + if (i == DAX_UNKNOWN)
> + continue;
> + __dax_regions_init(ctx, i);
> + }
> +}
> +
> DAXCTL_EXPORT struct daxctl_dev *daxctl_dev_get_first(struct daxctl_region *region)
> {
> dax_devices_init(region);
> diff --git a/ndctl.spec.in b/ndctl.spec.in
> index 26396d4abad7..60d9e6fedf71 100644
> --- a/ndctl.spec.in
> +++ b/ndctl.spec.in
> @@ -136,6 +136,7 @@ make check
> %defattr(-,root,root)
> %doc README.md
> %license COPYING licenses/BSD-MIT licenses/CC0
> +%config(noreplace) %{_sysconfdir}/modprobe.d/daxctl.conf
> %{_libdir}/libdaxctl.so.*
>
> %files -n DNAME
> diff --git a/util/sysfs.c b/util/sysfs.c
> index 0440fd0f49a3..9f7bc1f4930f 100644
> --- a/util/sysfs.c
> +++ b/util/sysfs.c
> @@ -91,7 +91,7 @@ int __sysfs_device_parse(struct log_ctx *ctx, const char *base_path,
> struct dirent *de;
> DIR *dir;
>
> - log_dbg(ctx, "base: %s dev: %s\n", base_path, dev_name);
> + log_dbg(ctx, "base: '%s' dev: '%s'\n", base_path, dev_name);
> dir = opendir(base_path);
> if (!dir) {
> log_dbg(ctx, "no \"%s\" devices found\n", dev_name);
>
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
next prev parent reply other threads:[~2018-11-08 20:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-07 19:13 [daxctl PATCH] daxctl: Opt-in to /sys/bus/dax ABI Dan Williams
2018-11-07 19:44 ` Jeff Moyer
2018-11-07 19:54 ` Dan Williams
2018-11-08 19:34 ` Jeff Moyer
2018-11-08 20:56 ` Jeff Moyer [this message]
2018-11-08 21:14 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=x49lg63cmp2.fsf@segfault.boston.devel.redhat.com \
--to=jmoyer@redhat.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=linux-nvdimm@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).