[PATCH v6 2/4] ndctl, monitor: add ndctl monitor daemon

[QI Fuli]

This patch adds the body file of ndctl monitor daemon.

Signed-off-by: QI Fuli 
---
 builtin.h |   1 +
 ndctl/Makefile.am |   3 +-
 ndctl/monitor.c   | 460 ++
 ndctl/ndctl.c |   1 +
 4 files changed, 464 insertions(+), 1 deletion(-)
 create mode 100644 ndctl/monitor.c

diff --git a/builtin.h b/builtin.h
index d3cc723..675a6ce 100644
--- a/builtin.h
+++ b/builtin.h
@@ -39,6 +39,7 @@ int cmd_inject_error(int argc, const char **argv, void *ctx);
 int cmd_wait_scrub(int argc, const char **argv, void *ctx);
 int cmd_start_scrub(int argc, const char **argv, void *ctx);
 int cmd_list(int argc, const char **argv, void *ctx);
+int cmd_monitor(int argc, const char **argv, void *ctx);
 #ifdef ENABLE_TEST
 int cmd_test(int argc, const char **argv, void *ctx);
 #endif
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index d22a379..7dbf223 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -16,7 +16,8 @@ ndctl_SOURCES = ndctl.c \
util/json-smart.c \
util/json-firmware.c \
inject-error.c \
-   inject-smart.c
+   inject-smart.c \
+   monitor.c
 
 if ENABLE_DESTRUCTIVE
 ndctl_SOURCES += ../test/blk_namespaces.c \
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
new file mode 100644
index 000..ab6e701
--- /dev/null
+++ b/ndctl/monitor.c
@@ -0,0 +1,460 @@
+/*
+ * Copyright(c) 2018, FUJITSU LIMITED. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#define BUF_SIZE 2048
+
+static enum log_destination {
+   LOG_DESTINATION_STDERR = 1,
+   LOG_DESTINATION_SYSLOG = 2,
+   LOG_DESTINATION_FILE = 3,
+} log_destination = LOG_DESTINATION_SYSLOG;
+
+static struct {
+   const char *logfile;
+   const char *config_file;
+   bool daemon;
+} monitor;
+
+struct monitor_dimm_node {
+   struct ndctl_dimm *dimm;
+   int health_eventfd;
+   struct list_node list;
+};
+
+struct monitor_filter_arg {
+   struct list_head mdimm;
+   int maxfd_dimm;
+   int num_dimm;
+   unsigned long flags;
+};
+
+struct util_filter_params param;
+
+static int did_fail;
+
+#define fail(fmt, ...) \
+do { \
+   did_fail = 1; \
+   err(ctx, "ndctl-%s:%s:%d: " fmt, \
+   VERSION, __func__, __LINE__, ##__VA_ARGS__); \
+} while (0)
+
+static bool is_dir(char *filepath)
+{
+   DIR *dir = opendir(filepath);
+   if (dir) {
+   closedir(dir);
+   return true;
+   }
+   return false;
+}
+
+static int recur_mkdir(char *filepath, mode_t mode)
+{
+   char *p;
+   char *buf = (char *)malloc(strlen(filepath) + 4);
+
+   strcpy(buf, filepath);
+   for (p = strchr(buf + 1, '/'); p; p = strchr(p + 1, '/')) {
+   *p = '\0';
+   if (!is_dir(buf)) {
+   if (mkdir(buf, mode) < 0) {
+   free(buf);
+   return -1;
+   }
+   }
+   *p = '/';
+   }
+
+   free(buf);
+   return 0;
+}
+
+static void set_value(const char **arg, char *val)
+{
+   struct strbuf value = STRBUF_INIT;
+   size_t arg_len = *arg ? strlen(*arg) : 0;
+
+   if (arg_len) {
+   strbuf_add(, *arg, arg_len);
+   strbuf_addstr(, " ");
+   }
+   strbuf_addstr(, val);
+   *arg = strbuf_detach(, NULL);
+}
+
+static void logreport(struct ndctl_ctx *ctx, int priority, const char *file,
+   int line, const char *fn, const char *format, va_list args)
+{
+   char *log_dir;
+   char *buf = (char *)malloc(BUF_SIZE);
+   vsnprintf(buf, BUF_SIZE, format, args);
+
+   switch (log_destination) {
+   case LOG_DESTINATION_STDERR:
+   fprintf(stderr, "%s\n", buf);
+   goto end;
+
+   case LOG_DESTINATION_SYSLOG:
+   syslog(priority, "%s\n", buf);
+   goto end;
+
+   case LOG_DESTINATION_FILE:
+   log_dir = dirname(strdup(monitor.logfile));
+   if (!is_dir(log_dir) && recur_mkdir(log_dir, 0744) != 0)
+   goto log_file_err;
+   FILE *f = fopen(monitor.logfile, "a+");
+   if (!f)
+   goto log_file_err;
+   fprintf(f, "%s\n", buf);
+   fclose(f);
+ 

[PATCH v6 3/4] ndctl, monitor: add default configuration file

[QI Fuli]

This patch adds the default configuration file of ndctl monitor.
Users can change the configuration by editing this file or by
using [--config-file=] option to override this one.
The monitor started by systemd follows the configuration in this file.

Signed-off-by: QI Fuli 
---
 ndctl/Makefile.am  |  5 +
 ndctl/monitor.conf | 37 +
 2 files changed, 42 insertions(+)
 create mode 100644 ndctl/monitor.conf

diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 7dbf223..ae3d894 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -42,3 +42,8 @@ ndctl_SOURCES += ../test/libndctl.c \
 ../test/multi-pmem.c \
 ../test/core.c
 endif
+
+monitor_config_file = monitor.conf
+monitor_configdir = /etc/ndctl/
+monitor_config_DATA = $(monitor_config_file)
+EXTRA_DIST += $(monitor_config_file)
diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf
new file mode 100644
index 000..13ad7b0
--- /dev/null
+++ b/ndctl/monitor.conf
@@ -0,0 +1,37 @@
+# This is the main ndctl monitor configuration file. It contains the
+# configuration directives that give ndctl monitor instructions.
+# You can change the configuration of ndctl monitor by editing this
+# file or by using [--config-file=] option to override this one.
+# The changed value will work after restart ndctl monitor service.
+
+# In this file, lines starting with a hash (#) are comments.
+# The configurations shold follow  =  style.
+# Multiple space-seperated values are allowed, but except the following
+# charecters: : # ? / \ % " '
+
+# The dimms to monitor are filtered via dimm's name by setting key "dimm".
+# If this value is different from the value of [--dimm=] option,
+# both of the values will work.
+# dimm = all
+
+# The dimms to monitor are filtered via its parent bus by setting key "bus".
+# If this value is different from the value of [--bus=] option,
+# both of the values will work.
+# bus = all
+
+# The dimms to monitor are filtered via region by setting key "region".
+# If this value is different from the value of [--namespace=] option,
+# both of the values will work.
+# region = all
+
+# The dimms to monitor are filtered via namespace by setting key "namespace".
+# If this value is different from the value of [--namespace=] option,
+# both of the values will work.
+# namespace = all
+
+# Users can choose to output the notifications to syslog (logfile=syslog),
+# stderr (logfile=stderr) or to write into a special file (logfile=)
+# by setting key "logfile".
+# If this value is in conflict with the value of [--logfile=] option,
+# this value will be ignored.
+# logfile = /var/log/ndctl/monitor.log
-- 
2.17.0.140.g0b0cc9f86


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v6 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service

[QI Fuli]

This patch adds the systemd unit file for ndctl-monitor service.
The systemd unit directory can be configured by setting environment
variable "--with-systemd-unit-dir[=DIR]".

Signed-off-by: QI Fuli 
---
 autogen.sh  |  3 ++-
 configure.ac| 22 ++
 ndctl/Makefile.am   |  4 
 ndctl/ndctl-monitor.service |  7 +++
 4 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 ndctl/ndctl-monitor.service

diff --git a/autogen.sh b/autogen.sh
index a23cf53..b226c7a 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-systemd-unit-dir"
 
 echo
 echo ""
diff --git a/configure.ac b/configure.ac
index cddad16..fd227eb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -135,6 +135,27 @@ AC_CHECK_FUNCS([ \
secure_getenv\
 ])
 
+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemd-unit-dir],
+   AS_HELP_STRING([--with-systemd-unit-dir[=DIR]],
+   [Directory for systemd service files]),
+   [],
+   [with_systemd_unit_dir=yes])
+
+if test "x$with_systemd_unit_dir" = "xyes"; then
+   def_systemd_unit_dir=$($PKG_CONFIG --variable=systemdsystemunitdir 
systemd)
+   if test "x$def_systemd_unit_dir" = "x"; then
+   AC_MSG_ERROR([systemd support requested but pkg-config unable 
to query systemd package])
+   with_systemd_unit_dir=no
+   else
+   with_systemd_unit_dir="$def_systemd_unit_dir"
+   fi
+fi
+
+AS_IF([test "x$with_systemd_unit_dir" != "xno"],
+   [AC_SUBST([systemd_unitdir], [$with_systemd_unit_dir])])
+AM_CONDITIONAL([ENABLE_SYSTEMD_UNIT_DIR], [test "x$with_systemd_unit_dir" != 
"xno"])
+
 my_CFLAGS="\
 -Wall \
 -Wchar-subscripts \
@@ -172,6 +193,7 @@ AC_MSG_RESULT([
 sysconfdir: ${sysconfdir}
 libdir: ${libdir}
 includedir: ${includedir}
+   systemd-unit-dir:   ${systemd_unitdir}
 
 compiler:   ${CC}
 cflags: ${CFLAGS}
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index ae3d894..9d008d5 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -47,3 +47,7 @@ monitor_config_file = monitor.conf
 monitor_configdir = /etc/ndctl/
 monitor_config_DATA = $(monitor_config_file)
 EXTRA_DIST += $(monitor_config_file)
+
+if ENABLE_SYSTEMD_UNIT_DIR
+systemd_unit_DATA = ndctl-monitor.service
+endif
diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
new file mode 100644
index 000..44f9326
--- /dev/null
+++ b/ndctl/ndctl-monitor.service
@@ -0,0 +1,7 @@
+[Unit]
+Description=Ndctl Monitor Daemon
+
+[Service]
+Type=forking
+ExecStart=/usr/bin/ndctl monitor --daemon
+ExecStop=/bin/kill ${MAINPID}
-- 
2.17.0.140.g0b0cc9f86


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v6 0/4] ndctl, monitor: add ndctl monitor daemon

[QI Fuli]

This is the v6 patch for ndctl monitor daemon, a tiny daemon to monitor
the smart events of nvdimm DIMMs. Users can run a monitor as a one-shot
command or a daemon in background by using the [--daemon] option. DIMMs
to monitor can be selected by [--dimm] [--bus] [--region] [--namespace]
options, these options support multiple space-seperated arguments.
When a smart event fires, monitor daemon will log the notifications
which including dimm health status to syslog or a logfile by setting
[--logfile=] option. monitor also can output the
notifications to stderr when it run as one-shot command by setting
[--logfile=]. The notifications follow json format and can be
consumed by log collectors like Fluentd. Users can change the
configuration of monitor by editing the default configuration file
/etc/ndctl/monitor.conf or by using [--config-file=] option to
override the default configuration. 

Users can start a monitor daemon by the following command:
 # ndctl monitor --daemon --logfile /var/log/ndctl/monitor.log

Also, a monitor daemon can be started by systemd:
 # systemctl start ndctl-monitor.service
In this case, monitor daemon follows the default configuration file
/etc/ndctl/monitor.conf.

Signed-off-by: QI Fuli 

---
Change log since v5:
 - Fixing systemd unit file cannot be installed bug
 - Adding license to ./util/abspath.c

Change log since v4:
 - Adding OPTION_FILENAME to make sure filename is correct
 - Adding configuration file
 - Adding [--config-file] option to override the default configuration
 - Making some options support multiple space-seperated arguments
 - Making systemctl enable ndctl-monitor.service command work
 - Making systemctl restart ndctl-monitor.service command work
 - Making the directory of systemd unit file to be configurable
 - Changing log_file() and log_syslog() to logreport()
 - Changing date format in notification to nanoseconds since epoch
 - Changing select() to epoll()
 - Adding filter_bus() and filter_region()

Change log since v3:
 - Removing create-monitor, show-monitor, list-monitor, destroy-monitor
 - Adding [--daemon] option to run ndctl monitor as a daemon 
 - Using systemd to manage ndctl monitor daemon
 - Replacing filter_monitor_dimm() with filter_dimm()

Change log since v2:
 - Changing the interface of daemon to the ndctl command line
 - Changing the name of daemon form "nvdimmd" to "monitor"
 - Removing the config file, unit_file, nvdimmd dir
 - Removing nvdimmd_test program
 - Adding ndctl/monitor.c

Change log since v1:
 - Adding a config file(/etc/nvdimmd/nvdimmd.conf)
 - Using struct log_ctx instead of syslog()
- Using log_syslog() to save the notify messages to syslog
- Using log_file() to save the notify messages to special file
 - Adding LOG_NOTICE level to log_priority
 - Using automake instead of Makefile
 - Adding a new util file(nvdimmd/util.c) including helper functions
   needed for nvdimm daemon
 - Adding nvdimmd_test program

QI Fuli (4):
  ndctl, util: add OPTION_FILENAME to parse_opt_type
  ndctl, monitor: add ndctl monitor daemon
  ndctl, monitor: add default configuration file
  ndctl, monitor: add the unit file of systemd for ndctl-monitor service

 Makefile.am |   3 +-
 autogen.sh  |   3 +-
 builtin.h   |   1 +
 configure.ac|  22 ++
 ndctl/Makefile.am   |  12 +-
 ndctl/monitor.c | 460 
 ndctl/monitor.conf  |  37 +++
 ndctl/ndctl-monitor.service |   7 +
 ndctl/ndctl.c   |   1 +
 util/abspath.c  |  29 +++
 util/help.c |   5 -
 util/parse-options.c|  47 +++-
 util/parse-options.h|  11 +-
 util/util.h |   7 +
 14 files changed, 629 insertions(+), 16 deletions(-)
 create mode 100644 ndctl/monitor.c
 create mode 100644 ndctl/monitor.conf
 create mode 100644 ndctl/ndctl-monitor.service
 create mode 100644 util/abspath.c

-- 
2.17.0.140.g0b0cc9f86


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v6 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type

[QI Fuli]

This patch borrows the OPTION_FILENAME from git to ndctl to make sure
filename is correct. Some related refactoring is also included:
  - adds parse_options_prefix() interface
  - moves is_absolute from util/help.c to util/util.c
  - adds a new file util/abspath.c

Signed-off-by: QI Fuli 
---
 Makefile.am  |  3 ++-
 util/abspath.c   | 29 +++
 util/help.c  |  5 -
 util/parse-options.c | 47 ++--
 util/parse-options.h | 11 +--
 util/util.h  |  7 +++
 6 files changed, 88 insertions(+), 14 deletions(-)
 create mode 100644 util/abspath.c

diff --git a/Makefile.am b/Makefile.am
index b538b1f..e0c463a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -69,6 +69,7 @@ libutil_a_SOURCES = \
util/strbuf.c \
util/wrapper.c \
util/filter.c \
-   util/bitmap.c
+   util/bitmap.c \
+   util/abspath.c
 
 nobase_include_HEADERS = daxctl/libdaxctl.h
diff --git a/util/abspath.c b/util/abspath.c
new file mode 100644
index 000..09bbd27
--- /dev/null
+++ b/util/abspath.c
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* originally copied from git/abspath.c */
+
+#include 
+#include 
+
+char *prefix_filename(const char *pfx, const char *arg)
+{
+   struct strbuf path = STRBUF_INIT;
+   size_t pfx_len = pfx ? strlen(pfx) : 0;
+
+   if (!pfx_len)
+   ;
+   else if (is_absolute_path(arg))
+   pfx_len = 0;
+   else
+   strbuf_add(, pfx, pfx_len);
+
+   strbuf_addstr(, arg);
+   return strbuf_detach(, NULL);
+}
+
+void fix_filename(const char *prefix, const char **file)
+{
+   if (!file || !*file || !prefix || is_absolute_path(*file)
+   || !strcmp("-", *file))
+   return;
+   *file = prefix_filename(prefix, *file);
+}
diff --git a/util/help.c b/util/help.c
index 8b8f951..2d57fa1 100644
--- a/util/help.c
+++ b/util/help.c
@@ -89,11 +89,6 @@ static char *cmd_to_page(const char *cmd, char **page, const 
char *util_name)
return *page;
 }
 
-static int is_absolute_path(const char *path)
-{
-   return path[0] == '/';
-}
-
 static const char *system_path(const char *path)
 {
 static const char *prefix = PREFIX;
diff --git a/util/parse-options.c b/util/parse-options.c
index 751c091..c781174 100644
--- a/util/parse-options.c
+++ b/util/parse-options.c
@@ -55,6 +55,7 @@ static int get_value(struct parse_opt_ctx_t *p,
 {
const char *s, *arg = NULL;
const int unset = flags & OPT_UNSET;
+   int err;
 
if (unset && p->opt)
return opterror(opt, "takes no value", flags);
@@ -77,6 +78,7 @@ static int get_value(struct parse_opt_ctx_t *p,
case OPTION_ARGUMENT:
case OPTION_GROUP:
case OPTION_STRING:
+   case OPTION_FILENAME:
case OPTION_INTEGER:
case OPTION_UINTEGER:
case OPTION_LONG:
@@ -121,6 +123,19 @@ static int get_value(struct parse_opt_ctx_t *p,
return get_arg(p, opt, flags, (const char 
**)opt->value);
return 0;
 
+   case OPTION_FILENAME:
+   err = 0;
+   if (unset)
+   *(const char **)opt->value = NULL;
+   else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
+   *(const char **)opt->value = (const char *)opt->defval;
+   else
+   err = get_arg(p, opt, flags, (const char **)opt->value);
+
+   if (!err)
+   fix_filename(p->prefix, (const char **)opt->value);
+   return err;
+
case OPTION_CALLBACK:
if (unset)
return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
@@ -339,13 +354,14 @@ static void check_typos(const char *arg, const struct 
option *options)
}
 }
 
-void parse_options_start(struct parse_opt_ctx_t *ctx,
-int argc, const char **argv, int flags)
+void parse_options_start(struct parse_opt_ctx_t *ctx, int argc,
+   const char **argv, const char *prefix, int flags)
 {
memset(ctx, 0, sizeof(*ctx));
ctx->argc = argc - 1;
ctx->argv = argv + 1;
ctx->out  = argv;
+   ctx->prefix = prefix;
ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
ctx->flags = flags;
if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
@@ -453,8 +469,10 @@ int parse_options_end(struct parse_opt_ctx_t *ctx)
return ctx->cpidx + ctx->argc;
 }
 
-int parse_options_subcommand(int argc, const char **argv, const struct option 
*options,
-   const char *const subcommands[], const char 
*usagestr[], int flags)
+static int parse_options_subcommand_prefix(int argc, const char **argv,
+   const char *prefix, const struct option *options,
+   

ypqianafktavg

[Bounced mail]

The original message was received at Mon, 7 May 2018 11:32:16 +0800
from lists.01.org [155.162.146.209]

- The following addresses had permanent fatal errors -


- Transcript of session follows -
  while talking to lists.01.org.:
>>> MAIL From:"Bounced mail" 
<<< 501 "Bounced mail" ... Refused



___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type

[Qi, Fuli]

> -Original Message-
> From: Dan Williams [mailto:dan.j.willi...@intel.com]
> Sent: Friday, May 4, 2018 12:26 PM
> To: Qi, Fuli/斉 福利 
> Cc: linux-nvdimm 
> Subject: Re: [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type
> 
> On Thu, Apr 26, 2018 at 4:30 AM, QI Fuli  wrote:
> > This patch borrows the OPTION_FILENAME from git to ndctl to make sure
> > filename is correct. Some related refactoring is also included:
> >   - adds parse_options_prefix() interface
> >   - moves is_absolute from util/help.c to util/util.c
> >   - adds a new file util/abspath.c
> >
> > Signed-off-by: QI Fuli 
> >
> [..]
> > diff --git a/util/abspath.c b/util/abspath.c new file mode 100644
> > index 000..fdf4090
> > --- /dev/null
> > +++ b/util/abspath.c
> > @@ -0,0 +1,28 @@
> > +/* originally copied from git */
> 
> Lets say the source file name that it came from in and get and add the 
> license.
> 
> /* SPDX-License-Identifier: GPL-2.0 */
> /* originally copied from git/abspath.c */
> 
> Otherwise, this patch looks good to me.
> 
Thank you for your comment.
I will make a new version and add the license.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH v2] ndctl, filter: fix "keyword 'all' is ignored" in util__filter()

[Qi, Fuli]

> -Original Message-
> From: Verma, Vishal L [mailto:vishal.l.ve...@intel.com]
> Sent: Thursday, May 3, 2018 6:38 AM
> To: linux-nvdimm@lists.01.org; Qi, Fuli/斉 福利 
> Subject: Re: [PATCH v2] ndctl, filter: fix "keyword 'all' is ignored" in
> util__filter()
> 
> On Thu, 2018-04-26 at 09:56 +0900, QI Fuli wrote:
> > This is a follow up patch for commit c70adc3cf6bf ("ndctl, filter:
> > refactor
> > util__filter() to support multiple space-seperated arguments")
> > refactored util__filter() to support multiple space-seperated
> > arguments.
> > But, when the keyword "all" is included in space-seperated arguments,
> > it will be treaded as 's name. This patch fixes it.
> >
> > Signed-off-by: QI Fuli 
> >
> > Change log since v1:
> >  - Removing the strcmp(__ident, "all") == 0 at the top of
> > util__filter()
> >  - Changing the strcmp(ident, "all") == 0 to strcmp(name, "all") == 0
> >
> > ---
> >  util/filter.c | 20 
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> Thanks, applied. Just as a future note, you should have the "changes since v1"
> notes below the '---' marker (between the --- and the first file diffstat). 
> This
> prevents it from being picked up as part of the commit message.
> 
> Thanks,
>   -Vishal
> 

Thank you for your comment.
I will take an eye on it.

Best,
 - Qi

> >
> > diff --git a/util/filter.c b/util/filter.c index 0d3cc02..1734bce
> > 100644
> > --- a/util/filter.c
> > +++ b/util/filter.c
> > @@ -31,7 +31,7 @@ struct ndctl_bus *util_bus_filter(struct ndctl_bus
> > *bus, const char *__ident)
> > unsigned long bus_id, id;
> > const char *provider, *devname, *name;
> >
> > -   if (!__ident || strcmp(__ident, "all") == 0)
> > +   if (!__ident)
> > return bus;
> >
> > ident = strdup(__ident);
> > @@ -40,6 +40,9 @@ struct ndctl_bus *util_bus_filter(struct ndctl_bus
> > *bus, const char *__ident)
> >
> > for (name = strtok_r(ident, " ", ); name;
> > name = strtok_r(NULL, " ", )) {
> > +   if (strcmp(name, "all") == 0)
> > +   break;
> > +
> > bus_id = strtoul(ident, , 0);
> > if (end == ident || end[0])
> > bus_id = ULONG_MAX;
> > @@ -69,7 +72,7 @@ struct ndctl_region *util_region_filter(struct
> > ndctl_region *region,
> > const char *name, *region_name;
> > unsigned long region_id, id;
> >
> > -   if (!__ident || strcmp(__ident, "all") == 0)
> > +   if (!__ident)
> > return region;
> >
> > ident = strdup(__ident);
> > @@ -78,6 +81,9 @@ struct ndctl_region *util_region_filter(struct
> > ndctl_region *region,
> >
> > for (name = strtok_r(ident, " ", ); name;
> > name = strtok_r(NULL, " ", )) {
> > +   if (strcmp(name, "all") == 0)
> > +   break;
> > +
> > region_id = strtoul(ident, , 0);
> > if (end == ident || end[0])
> > region_id = ULONG_MAX;
> > @@ -106,7 +112,7 @@ struct ndctl_namespace
> > *util_namespace_filter(struct ndctl_namespace *ndns,
> > const char *name;
> > char *ident, *save;
> >
> > -   if (!__ident || strcmp(__ident, "all") == 0)
> > +   if (!__ident)
> > return ndns;
> >
> > ident = strdup(__ident);
> > @@ -115,6 +121,9 @@ struct ndctl_namespace
> > *util_namespace_filter(struct ndctl_namespace *ndns,
> >
> > for (name = strtok_r(ident, " ", ); name;
> > name = strtok_r(NULL, " ", )) {
> > +   if (strcmp(name, "all") == 0)
> > +   break;
> > +
> > if (strcmp(name, ndctl_namespace_get_devname(ndns)) ==
> > 0)
> > break;
> >
> > @@ -137,7 +146,7 @@ struct ndctl_dimm *util_dimm_filter(struct
> > ndctl_dimm *dimm,
> > const char *name, *dimm_name;
> > unsigned long dimm_id, id;
> >
> > -   if (!__ident || strcmp(__ident, "all") == 0)
> > +   if (!__ident)
> > return dimm;
> >
> > ident = strdup(__ident);
> > @@ -146,6 +155,9 @@ struct ndctl_dimm *util_dimm_filter(struct
> > ndctl_dimm *dimm,
> >
> > for (name = strtok_r(ident, " ", ); name;
> > name = strtok_r(NULL, " ", )) {
> > +   if (strcmp(name, "all") == 0)
> > +   break;
> > +
> > dimm_id = strtoul(ident, , 0);
> > if (end == ident || end[0])
> > dimm_id = ULONG_MAX;
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource

[Baoquan He]

Hi Wei Yang,

On 04/26/18 at 09:18am, Wei Yang wrote:
> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
> >The struct resource uses singly linked list to link siblings. It's not
> >easy to do reverse iteration on sibling list. So replace it with list_head.
> >
> 
> Hi, Baoquan
> 
> Besides changing the data structure, I have another proposal to do the reverse
> iteration. Which means it would not affect other users, if you just want a
> reverse iteration.
> 
> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
> is a better solution to your first proposal in
> https://patchwork.kernel.org/patch/10300819/.
> 
> Below is my proposal of resource reverse iteration without changing current
> design.

I got your mail and read it, then interrupted by other thing and forgot
replying, sorry.

I am fine with your code change. As I said before, I have tried to change
code per reviewers' comment, then let reviewers decide which way is
better. Please feel free to post formal patches and joining discussion
about this issue.

Thanks
Baoquan

> 
> From 5d7145d44fe48b98572a03884fa3a3aa82e3cef9 Mon Sep 17 00:00:00 2001
> From: Wei Yang 
> Date: Sat, 24 Mar 2018 23:25:46 +0800
> Subject: [PATCH] kernel/resource: add walk_system_ram_res_rev()
> 
> As discussed on https://patchwork.kernel.org/patch/10300819/, this patch
> comes up with a variant implementation of walk_system_ram_res_rev(), which
> uses iteration instead of allocating array to store those resources.
> 
> Signed-off-by: Wei Yang 
> ---
>  include/linux/ioport.h |   3 ++
>  kernel/resource.c  | 113 
> +
>  2 files changed, 116 insertions(+)
> 
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index da0ebaec25f0..473f1d9cb97e 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -277,6 +277,9 @@ extern int
>  walk_system_ram_res(u64 start, u64 end, void *arg,
>   int (*func)(struct resource *, void *));
>  extern int
> +walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> + int (*func)(struct resource *, void *));
> +extern int
>  walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 
> end,
>   void *arg, int (*func)(struct resource *, void *));
>  
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 769109f20fb7..d4ec5fbc6875 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -73,6 +73,38 @@ static struct resource *next_resource(struct resource *p, 
> bool sibling_only)
>   return p->sibling;
>  }
>  
> +static struct resource *prev_resource(struct resource *p, bool sibling_only)
> +{
> + struct resource *prev;
> + if (NULL == iomem_resource.child)
> + return NULL;
> +
> + if (p == NULL) {
> + prev = iomem_resource.child;
> + while (prev->sibling)
> + prev = prev->sibling;
> + } else {
> + if (p->parent->child == p) {
> + return p->parent;
> + }
> +
> + for (prev = p->parent->child; prev->sibling != p;
> + prev = prev->sibling) {}
> + }
> +
> + /* Caller wants to traverse through siblings only */
> + if (sibling_only)
> + return prev;
> +
> + for (;prev->child;) {
> + prev = prev->child;
> +
> + while (prev->sibling)
> + prev = prev->sibling;
> + }
> + return prev;
> +}
> +
>  static void *r_next(struct seq_file *m, void *v, loff_t *pos)
>  {
>   struct resource *p = v;
> @@ -401,6 +433,47 @@ static int find_next_iomem_res(struct resource *res, 
> unsigned long desc,
>   return 0;
>  }
>  
> +/*
> + * Finds the highest iomem resource existing within [res->start.res->end).
> + * The caller must specify res->start, res->end, res->flags, and optionally
> + * desc.  If found, returns 0, res is overwritten, if not found, returns -1.
> + * This function walks the whole tree and not just first level children until
> + * and unless first_level_children_only is true.
> + */
> +static int find_prev_iomem_res(struct resource *res, unsigned long desc,
> +bool first_level_children_only)
> +{
> + struct resource *p;
> +
> + BUG_ON(!res);
> + BUG_ON(res->start >= res->end);
> +
> + read_lock(_lock);
> +
> + for (p = prev_resource(NULL, first_level_children_only); p;
> + p = prev_resource(p, first_level_children_only)) {
> + if ((p->flags & res->flags) != res->flags)
> + continue;
> + if ((desc != IORES_DESC_NONE) && (desc != p->desc))
> + continue;
> + if (p->end < res->start || p->child == iomem_resource.child) {
> + p = NULL;
> + break;
> + }
> + if ((p->end >= res->start) 

Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource

[Baoquan He]

On 04/26/18 at 11:01am, kbuild test robot wrote:
> Hi Baoquan,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc2 next-20180424]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
> config: microblaze-mmu_defconfig (attached as .config)
> compiler: microblaze-linux-gcc (GCC) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=microblaze 
> 
> All errors (new ones prefixed by >>):

Thanks, below patch can fix it. Will repost including the fix.


diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 161f9758c631..56d189cb4be4 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -533,7 +533,9 @@ void pci_process_bridge_OF_ranges(struct pci_controller 
*hose,
res->flags = range.flags;
res->start = range.cpu_addr;
res->end = range.cpu_addr + range.size - 1;
-   res->parent = res->child = res->sibling = NULL;
+   res->parent = NULL;
+   INIT_LIST_HEAD(>child);
+   INIT_LIST_HEAD(>sibling);
}
}
 
@@ -625,28 +627,31 @@ EXPORT_SYMBOL(pcibios_add_device);
 static int __init reparent_resources(struct resource *parent,
 struct resource *res)
 {
-   struct resource *p, **pp;
-   struct resource **firstpp = NULL;
+   struct resource *p, *first = NULL;
 
-   for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
+   list_for_each_entry(p, >child, sibling) {
if (p->end < res->start)
continue;
if (res->end < p->start)
break;
if (p->start < res->start || p->end > res->end)
return -1;  /* not completely contained */
-   if (firstpp == NULL)
-   firstpp = pp;
+   if (first == NULL)
+   first = p;
}
-   if (firstpp == NULL)
+   if (first == NULL)
return -1;  /* didn't find any conflicting entries? */
res->parent = parent;
-   res->child = *firstpp;
-   res->sibling = *pp;
-   *firstpp = res;
-   *pp = NULL;
-   for (p = res->child; p != NULL; p = p->sibling) {
-   p->parent = res;
+   list_add(>sibling, >sibling.prev);
+   INIT_LIST_HEAD(>child);
+
+   /*
+* From first to p's previous sibling, they all fall into
+* res's region, change them as res's children.
+*/
+   list_cut_position(>child, first->sibling.prev, res->sibling.prev);
+   list_for_each_entry(p, >child, sibling) {
+p->parent = new;
pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
 p->name,
 (unsigned long long)p->start,

> 
>arch/microblaze/pci/pci-common.c: In function 
> 'pci_process_bridge_OF_ranges':
> >> arch/microblaze/pci/pci-common.c:536:44: error: incompatible types when 
> >> assigning to type 'struct list_head' from type 'void *'
>res->parent = res->child = res->sibling = NULL;
>^
>arch/microblaze/pci/pci-common.c: In function 'reparent_resources':
> >> arch/microblaze/pci/pci-common.c:631:10: error: assignment from 
> >> incompatible pointer type [-Werror=incompatible-pointer-types]
>  for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
>  ^
>arch/microblaze/pci/pci-common.c:631:50: error: assignment from 
> incompatible pointer type [-Werror=incompatible-pointer-types]
>  for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
>  ^
> >> arch/microblaze/pci/pci-common.c:644:13: error: incompatible types when 
> >> assigning to type 'struct list_head' from type 'struct resource *'
>  res->child = *firstpp;
> ^
>arch/microblaze/pci/pci-common.c:645:15: error: incompatible types when 
> assigning to type 'struct list_head' from type 'struct resource *'
>  res->sibling = *pp;
>   ^
> >> arch/microblaze/pci/pci-common.c:648:9: error: incompatible types when 
> >> assigning to type 'struct resource *' from type 'struct list_head'
>  for (p = res->child; p != NULL; p = p->sibling) {
> ^
>arch/microblaze/pci/pci-common.c:648:36: error: incompatible types when 
> assigning to type 'struct resource *' from type 'struct 

Re: [PATCH v3 1/3] resource: Use list_head to link sibling resource

[Baoquan He]

On 04/26/18 at 11:23am, kbuild test robot wrote:
> Hi Baoquan,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc2 next-20180424]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
> config: xtensa-common_defconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=xtensa 
> 
> All errors (new ones prefixed by >>):
> 
>In file included from arch/xtensa/lib/pci-auto.c:22:0:
>arch/xtensa/include/asm/pci-bridge.h: In function 'pcibios_init_resource':
> >> arch/xtensa/include/asm/pci-bridge.h:74:15: error: incompatible types when 
> >> assigning to type 'struct list_head' from type 'void *'
>  res->sibling = NULL;
>   ^
>arch/xtensa/include/asm/pci-bridge.h:75:13: error: incompatible types when 
> assigning to type 'struct list_head' from type 'void *'
>  res->child = NULL;

Thanks, below fix can fix it.

diff --git a/arch/xtensa/include/asm/pci-bridge.h 
b/arch/xtensa/include/asm/pci-bridge.h
index 0b68c76ec1e6..f487b06817df 100644
--- a/arch/xtensa/include/asm/pci-bridge.h
+++ b/arch/xtensa/include/asm/pci-bridge.h
@@ -71,8 +71,8 @@ static inline void pcibios_init_resource(struct resource *res,
res->flags = flags;
res->name = name;
res->parent = NULL;
-   res->sibling = NULL;
-   res->child = NULL;
+   INIT_LIST_HEAD(>child);
+   INIT_LIST_HEAD(>sibling);
 }
 
 

> ^
> 
> vim +74 arch/xtensa/include/asm/pci-bridge.h
> 
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  65  
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  66  static 
> inline void pcibios_init_resource(struct resource *res,
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  67  
> unsigned long start, unsigned long end, int flags, char *name)
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  68  {
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  69  
> res->start = start;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  70  
> res->end = end;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  71  
> res->flags = flags;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  72  
> res->name = name;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  73  
> res->parent = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 @74  
> res->sibling = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  75  
> res->child = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  76  }
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23  77  
> 
> :: The code at line 74 was first introduced by commit
> :: 9a8fd5589902153a134111ed7a40f9cca1f83254 [PATCH] xtensa: Architecture 
> support for Tensilica Xtensa Part 6
> 
> :: TO: Chris Zankel 
> :: CC: Linus Torvalds 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm