| Message ID | 20180322184007.25667-2-ross.zwisler@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 22, 2018 at 11:40 AM, Ross Zwisler <ross.zwisler@linux.intel.com> wrote: > Instead of just failing to find namespaces when trying to filter by NUMA > node when CONFIG_NUMA wasn't enabled in the kernel, instead fail loudly as > numactl does: > > # numactl --cpunodebind=0 ls > numactl: This system does not support NUMA policy > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > --- > > This enabling requires numactl to create a pkg-config file, which it > currently does not. This support is added by the following patch which > I just sent out: > > https://patchwork.kernel.org/patch/10302135/ Hmm, it's going to be a while for that to be picked up and filter out into all the distributions so we can either wait for the next numactl release to become commonly available, or rework this ndctl change to be independent of pkgconfig dependency. The other concern is that this affects the numa-node filtering, but it doesn't effect the numa-node json field reported by 'ndctl list'. I think it would be confusing to see numa-nodes in the listing, but filtering fails just because of the way ndctl was configured at build time. [..] > diff --git a/util/filter.c b/util/filter.c > index 291d7ed..512f927 100644 > --- a/util/filter.c > +++ b/util/filter.c > @@ -10,6 +10,7 @@ > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > * General Public License for more details. > */ > +#include <numa.h> > #include <stdio.h> > #include <string.h> > #include <stdlib.h> > @@ -328,6 +329,16 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx, > } > > if (param->numa_node && strcmp(param->numa_node, "all") != 0) { > +#ifdef WITH_NUMA > + if (numa_available() < 0) { > + error("This system does not support NUMA"); > +#else > + { > + error("ndctl was not configured with NUMA support"); > +#endif > + return -EINVAL; > + } > + Please push this #ifdef'ery into a header file.
On Thu, Mar 22, 2018 at 03:47:01PM -0700, Dan Williams wrote: > On Thu, Mar 22, 2018 at 11:40 AM, Ross Zwisler > <ross.zwisler@linux.intel.com> wrote: > > Instead of just failing to find namespaces when trying to filter by NUMA > > node when CONFIG_NUMA wasn't enabled in the kernel, instead fail loudly as > > numactl does: > > > > # numactl --cpunodebind=0 ls > > numactl: This system does not support NUMA policy > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > --- > > > > This enabling requires numactl to create a pkg-config file, which it > > currently does not. This support is added by the following patch which > > I just sent out: > > > > https://patchwork.kernel.org/patch/10302135/ > > Hmm, it's going to be a while for that to be picked up and filter out > into all the distributions so we can either wait for the next numactl > release to become commonly available, or rework this ndctl change to > be independent of pkgconfig dependency. Sure - I'm not yet sure how to do that. :) Any tips on how to check for library dependencies without pgk-config support?
On Thu, Mar 22, 2018 at 4:27 PM, Ross Zwisler <ross.zwisler@linux.intel.com> wrote: > On Thu, Mar 22, 2018 at 03:47:01PM -0700, Dan Williams wrote: >> On Thu, Mar 22, 2018 at 11:40 AM, Ross Zwisler >> <ross.zwisler@linux.intel.com> wrote: >> > Instead of just failing to find namespaces when trying to filter by NUMA >> > node when CONFIG_NUMA wasn't enabled in the kernel, instead fail loudly as >> > numactl does: >> > >> > # numactl --cpunodebind=0 ls >> > numactl: This system does not support NUMA policy >> > >> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> >> > --- >> > >> > This enabling requires numactl to create a pkg-config file, which it >> > currently does not. This support is added by the following patch which >> > I just sent out: >> > >> > https://patchwork.kernel.org/patch/10302135/ >> >> Hmm, it's going to be a while for that to be picked up and filter out >> into all the distributions so we can either wait for the next numactl >> release to become commonly available, or rework this ndctl change to >> be independent of pkgconfig dependency. > > Sure - I'm not yet sure how to do that. :) Any tips on how to check for > library dependencies without pgk-config support? The AC_CHECK_LIB() autoconf macro is the place to start.
diff --git a/Makefile.am.in b/Makefile.am.in index 6c5229d..b927e17 100644 --- a/Makefile.am.in +++ b/Makefile.am.in @@ -15,7 +15,8 @@ AM_CPPFLAGS = \ $(KMOD_CFLAGS) \ $(UDEV_CFLAGS) \ $(UUID_CFLAGS) \ - $(JSON_CFLAGS) + $(JSON_CFLAGS) \ + $(NUMA_CFLAGS) AM_CFLAGS = ${my_CFLAGS} \ -fvisibility=hidden \ diff --git a/autogen.sh b/autogen.sh index a23cf53..46515b2 100755 --- a/autogen.sh +++ b/autogen.sh @@ -17,7 +17,8 @@ libdir() { args="--prefix=/usr \ --sysconfdir=/etc \ ---libdir=$(libdir /usr/lib)" +--libdir=$(libdir /usr/lib) \ +--with-numa" echo echo "----------------------------------------------------------------" diff --git a/configure.ac b/configure.ac index 3eaac32..af21cb6 100644 --- a/configure.ac +++ b/configure.ac @@ -94,6 +94,18 @@ PKG_CHECK_MODULES([UDEV], [libudev]) PKG_CHECK_MODULES([UUID], [uuid]) PKG_CHECK_MODULES([JSON], [json-c]) +AC_ARG_WITH([numa], + AS_HELP_STRING([--with-numa], + [Install with NUMA support. @<:@default=no@:>@]), + [], + [with_numa=no]) + +if test "x$with_numa" = "xyes"; then + PKG_CHECK_MODULES([NUMA], [numa]) + AC_DEFINE([WITH_NUMA], [1], [with NUMA support]) +fi +AM_CONDITIONAL([WITH_NUMA], [test "x$with_numa" = "xyes"]) + AC_ARG_WITH([bash-completion-dir], AS_HELP_STRING([--with-bash-completion-dir[=PATH]], [Install the bash auto-completion script in this directory. @<:@default=yes@:>@]), diff --git a/ndctl.spec.in b/ndctl.spec.in index e2c879c..3fcb4d8 100644 --- a/ndctl.spec.in +++ b/ndctl.spec.in @@ -20,6 +20,7 @@ BuildRequires: pkgconfig(libudev) BuildRequires: pkgconfig(uuid) BuildRequires: pkgconfig(json-c) BuildRequires: pkgconfig(bash-completion) +BuildRequires: pkgconfig(numa) %description Utility library for managing the "libnvdimm" subsystem. The "libnvdimm" @@ -90,7 +91,7 @@ control API for these devices. %build echo %{version} > version ./autogen.sh -%configure --disable-static --disable-silent-rules +%configure --disable-static --disable-silent-rules --with-numa make %{?_smp_mflags} %install diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am index 213cabd..6b891f8 100644 --- a/ndctl/Makefile.am +++ b/ndctl/Makefile.am @@ -29,7 +29,8 @@ ndctl_LDADD =\ ../libutil.a \ $(UUID_LIBS) \ $(KMOD_LIBS) \ - $(JSON_LIBS) + $(JSON_LIBS) \ + $(NUMA_LIBS) if ENABLE_TEST ndctl_SOURCES += ../test/libndctl.c \ diff --git a/util/filter.c b/util/filter.c index 291d7ed..512f927 100644 --- a/util/filter.c +++ b/util/filter.c @@ -10,6 +10,7 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. */ +#include <numa.h> #include <stdio.h> #include <string.h> #include <stdlib.h> @@ -328,6 +329,16 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx, } if (param->numa_node && strcmp(param->numa_node, "all") != 0) { +#ifdef WITH_NUMA + if (numa_available() < 0) { + error("This system does not support NUMA"); +#else + { + error("ndctl was not configured with NUMA support"); +#endif + return -EINVAL; + } + numa_node = strtol(param->numa_node, &end, 0); if (end == param->numa_node || end[0]) { error("invalid numa_node: '%s'\n", param->numa_node);
Instead of just failing to find namespaces when trying to filter by NUMA node when CONFIG_NUMA wasn't enabled in the kernel, instead fail loudly as numactl does: # numactl --cpunodebind=0 ls numactl: This system does not support NUMA policy Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> --- This enabling requires numactl to create a pkg-config file, which it currently does not. This support is added by the following patch which I just sent out: https://patchwork.kernel.org/patch/10302135/ --- Makefile.am.in | 3 ++- autogen.sh | 3 ++- configure.ac | 12 ++++++++++++ ndctl.spec.in | 3 ++- ndctl/Makefile.am | 3 ++- util/filter.c | 11 +++++++++++ 6 files changed, 31 insertions(+), 4 deletions(-)