[ndctl PATCH v2] ndctl, list: add alarm_enable_ to list
This patch adds alarm_enable_ to list, so that users could check
if the "ndctl inject-smart ---alarm=on/off" works well or not.
Signed-off-by: QI Fuli
---
v1 -> v2:
remove the renaming for list items.
ndctl/util/json-smart.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/ndctl/util/json-smart.c b/ndctl/util/json-smart.c
index 6a1f294..e25483d 100644
--- a/ndctl/util/json-smart.c
+++ b/ndctl/util/json-smart.c
@@ -39,34 +39,61 @@ static void smart_threshold_to_json(struct ndctl_dimm *dimm,
unsigned int temp;
double t;
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_media_temperature", jobj);
temp = ndctl_cmd_smart_threshold_get_temperature(cmd);
t = ndctl_decode_smart_temperature(temp);
jobj = json_object_new_double(t);
if (jobj)
json_object_object_add(jhealth,
"temperature_threshold", jobj);
+ } else {
+ jobj = json_object_new_boolean(false);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_media_temperature", jobj);
}
if (alarm_control & ND_SMART_CTEMP_TRIP) {
unsigned int temp;
double t;
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_ctrl_temperature", jobj);
temp = ndctl_cmd_smart_threshold_get_ctrl_temperature(cmd);
t = ndctl_decode_smart_temperature(temp);
jobj = json_object_new_double(t);
if (jobj)
json_object_object_add(jhealth,
"controller_temperature_threshold", jobj);
+ } else {
+ jobj = json_object_new_boolean(false);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_ctrl_temperature", jobj);
}
if (alarm_control & ND_SMART_SPARE_TRIP) {
unsigned int spares;
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_spares", jobj);
spares = ndctl_cmd_smart_threshold_get_spares(cmd);
jobj = json_object_new_int(spares);
if (jobj)
json_object_object_add(jhealth,
"spares_threshold", jobj);
+ } else {
+ jobj = json_object_new_boolean(false);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_spares", jobj);
}
out:
--
2.18.0
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH V2 2/4] mm: introduce memory type MEMORY_DEVICE_DEV_DAX
On 2018年08月07日 17:11, Jan Kara wrote:
> On Wed 11-07-18 01:01:59, Zhang Yi wrote:
>> Currently, NVDIMM pages will be marked 'PageReserved'. However, unlike
>> other reserved PFNs, pages on NVDIMM shall still behave like normal ones
>> in many cases, i.e. when used as backend memory of KVM guest. This patch
>> introduces a new memory type, MEMORY_DEVICE_DEV_DAX. And set this flag
>> while dax driver hotplug the device memory.
>>
>> Signed-off-by: Zhang Yi
>> Signed-off-by: Zhang Yu
>> ---
>> drivers/dax/pmem.c | 1 +
>> include/linux/memremap.h | 9 +
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
>> index fd49b24..fb3f363 100644
>> --- a/drivers/dax/pmem.c
>> +++ b/drivers/dax/pmem.c
>> @@ -111,6 +111,7 @@ static int dax_pmem_probe(struct device *dev)
>> return rc;
>>
>> dax_pmem->pgmap.ref = _pmem->ref;
>> +dax_pmem->pgmap.type = MEMORY_DEVICE_DEV_DAX;
>> addr = devm_memremap_pages(dev, _pmem->pgmap);
>> if (IS_ERR(addr))
>> return PTR_ERR(addr);
>> diff --git a/include/linux/memremap.h b/include/linux/memremap.h
>> index 5ebfff6..a36bce8 100644
>> --- a/include/linux/memremap.h
>> +++ b/include/linux/memremap.h
>> @@ -53,11 +53,20 @@ struct vmem_altmap {
>> * wakeup event whenever a page is unpinned and becomes idle. This
>> * wakeup is used to coordinate physical address space management (ex:
>> * fs truncate/hole punch) vs pinned pages (ex: device dma).
>> + *
>> + * MEMORY_DEVICE_DEV_DAX:
>> + * DAX driver hotplug the device memory and move it to memory zone, these
>> + * pages will be marked reserved flag. However, some other kernel componet
>> + * will misconceive these pages are reserved mmio (ex: we map these dev_dax
>> + * or fs_dax pages to kvm for DIMM/NVDIMM backend). Together with the type
>> + * MEMORY_DEVICE_FS_DAX, we can differentiate the pages on NVDIMM with the
>> + * normal reserved pages.
> So I believe the description should be in terms of what kind of memory is
> the MEMORY_DEVICE_DEV_DAX type, not how users use this type. See comments
> for other memory types...
>
> Honza
Yes, agree, thanks for your kindly review. Jan.
>
>> */
>> enum memory_type {
>> MEMORY_DEVICE_PRIVATE = 1,
>> MEMORY_DEVICE_PUBLIC,
>> MEMORY_DEVICE_FS_DAX,
>> +MEMORY_DEVICE_DEV_DAX,
>> };
>>
>> /*
>> --
>> 2.7.4
>>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[PATCH v3] ndctl, test: add a new unit test for max_available_extent namespace
From: Masayoshi Mizuma
Add a new unit test to test max_available_extent namespace.
This feature is implemented by the following patches.
kernel side:
https://lists.01.org/pipermail/linux-nvdimm/2018-July/016731.html
https://lists.01.org/pipermail/linux-nvdimm/2018-July/016732.html
ndctl side:
https://lists.01.org/pipermail/linux-nvdimm/2018-July/017176.html
Signed-off-by: Masayoshi Mizuma
---
test/Makefile.am| 3 ++-
test/max_available_extent_ns.sh | 46 +
2 files changed, 48 insertions(+), 1 deletion(-)
create mode 100755 test/max_available_extent_ns.sh
diff --git a/test/Makefile.am b/test/Makefile.am
index 8c55056..9af2464 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -22,7 +22,8 @@ TESTS =\
firmware-update.sh \
ack-shutdown-count-set \
rescan-partitions.sh \
- monitor.sh
+ monitor.sh \
+ max_available_extent_ns.sh
check_PROGRAMS =\
libndctl \
diff --git a/test/max_available_extent_ns.sh b/test/max_available_extent_ns.sh
new file mode 100755
index 000..1c7e7bf
--- /dev/null
+++ b/test/max_available_extent_ns.sh
@@ -0,0 +1,46 @@
+#!/bin/bash -Ex
+
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2018, FUJITSU LIMITED. All rights reserved.
+
+rc=77
+
+. ./common
+
+trap 'err $LINENO' ERR
+
+check_min_kver "4.19" || do_skip "kernel $KVER may not support
max_available_size"
+
+init()
+{
+ $NDCTL disable-region -b $NFIT_TEST_BUS0 all
+ $NDCTL zero-labels -b $NFIT_TEST_BUS0 all
+ $NDCTL enable-region -b $NFIT_TEST_BUS0 all
+}
+
+do_test()
+{
+ region=$($NDCTL list -b $NFIT_TEST_BUS0 -R -t pmem | jq -r
'sort_by(-.size) | .[].dev' | head -1)
+
+ available_sz=$($NDCTL list -r $region | jq -r .[].available_size)
+ size=$(( available_sz/4 ))
+
+ NS=()
+ for ((i=0; i<3; i++))
+ do
+ NS[$i]=$($NDCTL create-namespace -r $region -t pmem -s $size |
jq -r .dev)
+ [[ -n ${NS[$i]} ]]
+ done
+
+ $NDCTL disable-namespace ${NS[1]}
+ $NDCTL destroy-namespace ${NS[1]}
+
+ $NDCTL create-namespace -r $region -t pmem
+}
+
+modprobe nfit_test
+rc=1
+init
+do_test
+_cleanup
+exit 0
--
2.18.0
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
RE: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified
> -Original Message-
> From: Masayoshi Mizuma [mailto:msys.miz...@gmail.com]
> Sent: Wednesday, August 8, 2018 4:30 AM
> To: Qi, Fuli/斉 福利 ; linux-nvdimm@lists.01.org
> Subject: Re: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to
> syslog
> if "--daemon" is specified
>
> Hi Qi,
>
> On 08/07/2018 09:17 AM, QI Fuli wrote:
> > When running monitor as a daemon, if the log destination is "standard"
> > or a relative path for log file, the messages will not be able to be logged.
> > Sometimes, users may not notice that the default log destination is
> > "standard"
> > when they start monitor daemon by systemctl, so they will lose messages.
> > This patch is used to fix the unfriendly interface. When running
> > monitor as a daemon, the default log destination will be changed to
> > syslog. Also, the messages will be forwarded to syslog if the log
> > destination is
> a relative path for log file.
> >
> > Signed-off-by: QI Fuli
> > ---
> > Documentation/ndctl/ndctl-monitor.txt | 16 +++-
> > ndctl/monitor.c | 5 -
> > ndctl/monitor.conf| 2 ++
> > 3 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ndctl/ndctl-monitor.txt
> > b/Documentation/ndctl/ndctl-monitor.txt
> > index 1cba9ea..9a8d76b 100644
> > --- a/Documentation/ndctl/ndctl-monitor.txt
> > +++ b/Documentation/ndctl/ndctl-monitor.txt
> > @@ -67,7 +67,21 @@ OPTIONS
> >
> > -l ::
> > --log=::
> > - Output notifications to , syslog or standard output.
> > + Send log messages to the specified destination.
> > ++
> > +--
> > +::
> > + Send log messages to specified . When fopen() is not able
> > + to open , log messages will be forwarded to syslog.
> > +syslog::
> > + Send messages to syslog.
> > +standard::
> > + Send messages to standard output.
> > +--
> > ++
> > +The default log destination is 'syslog' if "--daemon" is specified,
> > +otherwise 'standard'. Note that standard and relative path for
> > +will not work if "--daemon" is specified.
> >
> > -c::
> > --config-file=::
> > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index bf1f1d3..2f3d751
> > 100644
> > --- a/ndctl/monitor.c
> > +++ b/ndctl/monitor.c
> > @@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority,
> > const
> char *file,
> > f = fopen(monitor.log, "a+");
> > if (!f) {
> > ndctl_set_log_fn(ctx, log_syslog);
> > - err(ctx, "open logfile %s failed\n", monitor.log);
> > + err(ctx, "open logfile %s failed, forward messages to syslog\n",
> > + monitor.log);
> > did_fail = 1;
> > notice(ctx, "%s\n", buf);
> > goto end;
> > @@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> > }
> >
> > if (monitor.daemon) {
>
> Why don't you add './standard' check? Like as:
>
> if (strncmp(monitor.log, "./standard", 10) == 0)
> error("daemon doesn't work for 'standard' log
> option");
> goto out;
>
Hi Masa,
Thank you for your comment.
When running monitor as a daemon, the messages will not be able to be logged in
following cases.
a) Users set the log destination to standard by using [--log] option or setting
value of "log" in config file.
b) The log destination is standard by default.
c) Users set the log destination to a relative path of log file by using
[--log] option or setting value of "log" in config file.
The './standard' check will only works for case a).
Also, it would be more friendly to set a default log destination to monitor
daemon.
Thanks,
QI
> Thanks,
> Masa
>
> > + if (!monitor.log || strncmp(monitor.log, "./", 2) == 0)
> > + ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
>
> > if (daemon(0, 0) != 0) {
> > err((struct ndctl_ctx *)ctx, "daemon start failed\n");
> > goto out;
> > diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf index
> > 857aadf..934e2c0 100644
> > --- a/ndctl/monitor.conf
> > +++ b/ndctl/monitor.conf
> > @@ -38,4 +38,6 @@
> > # to standard output (log=standard) or to write into a special file
> > (log=) # by setting key "log". If this value is in conflict
> > with the value of # [--log=] option, this value will be ignored.
> > +# Note: Setting value to "standard" or relative path for will
> > +not work # when running moniotr as a daemon.
> > # log = /var/log/ndctl/monitor.log
> >
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log messages in log_file()
On 08/07/2018 08:54 PM, Qi, Fuli wrote:
>> -Original Message-
>> From: Masayoshi Mizuma [mailto:msys.miz...@gmail.com]
>> Sent: Wednesday, August 8, 2018 3:40 AM
>> To: Qi, Fuli/斉 福利 ; linux-nvdimm@lists.01.org
>> Subject: Re: [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log
>> messages
>> in log_file()
>>
>> Hi Qi,
>>
>> On 08/07/2018 09:17 AM, QI Fuli wrote:
>>> This patch is used to add timestamp and process id to log messages
>>> when logging messages to a file.
>>>
>>> Signed-off-by: QI Fuli
>>> ---
>>> ndctl/monitor.c | 11 ++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ndctl/monitor.c b/ndctl/monitor.c index 2f3d751..d29e378
>>> 100644
>>> --- a/ndctl/monitor.c
>>> +++ b/ndctl/monitor.c
>>> @@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int
>>> priority, const char *file, {
>>> FILE *f;
>>> char *buf;
>>> + struct timespec ts;
>>> + char timestamp[32];
>>>
>>> if (vasprintf(, format, args) < 0) {
>>> fail("vasprintf error\n");
>>> @@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int
>>> priority, const
>> char *file,
>>> notice(ctx, "%s\n", buf);
>>> goto end;
>>> }
>>> - fprintf(f, "%s", buf);
>>> +
>>> + if (priority != LOG_NOTICE) {
>>
> Hi Masa,
>
> Thanks for your comments.
>
>> Why is the timestamp not needed in case of LOG_NOTICE...?
>
> Because LOG_NOTICE level is only used for smart event notification in monitor.
> Since the timestamp and pid are already added to json format messages by
> notify_dimm_event(),
> so there is no need to add them again.
I see, thanks.
>
>>
>>> + clock_gettime(CLOCK_REALTIME, );
>>> + sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec);
>>> + fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf);
>>
>> What is the pid for...?
>>
> Multiple monitor can be run at the same time and they may send messages to a
> common log file.
> In this case, the pid could help users to know the monitor where each message
> came from.
OK, got it.
Your patch is good to me. Please feel free to add:
Reviewed-by: Masayoshi Mizuma
Thanks,
Masa
>
> Thanks,
> QI
>
>> Thanks,
>> Masa
>>
>>> + } else
>>> + fprintf(f, "%s", buf);
>>> +> fflush(f);
>>> fclose(f);
>>> end:
>>>
>>
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
RE: [ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of invalid path of log file
> -Original Message-
> From: Masayoshi Mizuma [mailto:msys.miz...@gmail.com]
> Sent: Wednesday, August 8, 2018 4:06 AM
> To: Qi, Fuli/斉 福利 ; linux-nvdimm@lists.01.org
> Subject: Re: [ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of
> invalid
> path of log file
>
> Hi Qi,
>
> On 08/07/2018 09:17 AM, QI Fuli wrote:
> > Currently the monitor can be started even with an invalid path of log file.
> > This patch adds a detection of invalid path of log file when starting
> > monitor.
> > If the path of log file is invalid, the monitor will refuse to be started.
> >
> > Signed-off-by: QI Fuli
> > ---
> > ndctl/monitor.c | 11 ++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index f10384b..bf1f1d3
> > 100644
> > --- a/ndctl/monitor.c
> > +++ b/ndctl/monitor.c
> > @@ -603,6 +603,7 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> > struct util_filter_ctx fctx = { 0 };
> > struct monitor_filter_arg mfa = { 0 };
> > int i, rc;
> > + FILE *f;
> >
> > argc = parse_options_prefix(argc, argv, prefix, options, u, 0);
> > for (i = 0; i < argc; i++) {
> > @@ -630,8 +631,16 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
> > else if (strncmp(monitor.log, "./standard", 10) == 0)
> > ; /*default, already set */
> > - else
> > + else {
> > + f = fopen(monitor.log, "a+");
> > + if (!f) {
> > + error("open %s failed\n", monitor.log);
> > + rc = -errno;
> > + goto out;
> > + }
> > + fclose(f);
> > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file);
> > + }
>
Masa,
Thanks for your comments.
> In log_file(), the log file does fallback to syslog if the fopen() fails.
> In my understanding here is that the fallback is needed to save in case of the
> monitor.log in trouble for example, the parent directory is removed.
> And, the new fopen() check, you have added by this patch, to inform the
> invalid log
> path for users.
>
Yes, this is what I wanted to implement.
> Is my understanding correct? If so, make sense to me.
> Please feel free to add:
>
> Reviewed-by: Masayoshi Mizuma
>
Thanks,
QI
> Thanks,
> Masa
>
> > }
> >
> > if (monitor.daemon) {
> >
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
RE: [ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify misc list items
Hi Vishal,
Thanks for your comments.
> -Original Message-
> From: Verma, Vishal L [mailto:vishal.l.ve...@intel.com]
> Sent: Wednesday, August 8, 2018 5:15 AM
> To: linux-nvdimm@lists.01.org; Qi, Fuli/斉 福利
> Subject: Re: [ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify
> misc
> list items
>
>
> On Wed, 2018-08-08 at 00:35 +0900, QI Fuli wrote:
> > This patch adds alarm_enable_ to list. Therefore, users can
> > know
>
> Hi Qi,
>
> Thanks, I was meaning to do this work but you beat me to it :) I just have a
> few
> nits, see below. But otherwise this looks good.
>
> > if the "ndclt inject-smart ---alarm=on/off" works or not.
>
> s/ndclt/ndctl/
>
> >
> > Users may confuse "alarm_enable" with "alarm_flag" and "media_temperature"
> > with "ctrl_temperature", this patch also used to clarify these items.
>
> I'm not sure it is a good idea to change these names since they've been
> released
> for quite some time, and users/scripts might be depending on them, which this
> change
> will break. Let's just add a new field for
> alarm_enabled_* for each smart field, but not change any of the existing ones
> for
> now.
>
Ok, I will make a v2 patch which only includes adding alarm_enable_.
Thanks,
QI
> >
> > Signed-off-by: QI Fuli
> > ---
> > ndctl/util/json-smart.c | 45
> > ++---
> > 1 file changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/ndctl/util/json-smart.c b/ndctl/util/json-smart.c index
> > 6a1f294..a0677a6 100644
> > --- a/ndctl/util/json-smart.c
> > +++ b/ndctl/util/json-smart.c
> > @@ -39,34 +39,61 @@ static void smart_threshold_to_json(struct ndctl_dimm
> > *dimm,
> > unsigned int temp;
> > double t;
> >
> > + jobj = json_object_new_boolean(true);
> > + if (jobj)
> > + json_object_object_add(jhealth,
> > + "alarm_enable_media_temperature", jobj);
>
> Lets reword all instances to alarm_enabled_* Since it is showing the status
> of the
> alarm.
>
> > temp = ndctl_cmd_smart_threshold_get_temperature(cmd);
> > t = ndctl_decode_smart_temperature(temp);
> > jobj = json_object_new_double(t);
> > if (jobj)
> > json_object_object_add(jhealth,
> > - "temperature_threshold", jobj);
> > + "media_temperature_threshold", jobj);
> > + } else {
> > + jobj = json_object_new_boolean(false);
> > + if (jobj)
> > + json_object_object_add(jhealth,
> > + "alarm_enable_media_temperature", jobj);
> > }
> >
> > if (alarm_control & ND_SMART_CTEMP_TRIP) {
> > unsigned int temp;
> > double t;
> >
> > + jobj = json_object_new_boolean(true);
> > + if (jobj)
> > + json_object_object_add(jhealth,
> > + "alarm_enable_ctrl_temperature", jobj);
> > temp = ndctl_cmd_smart_threshold_get_ctrl_temperature(cmd);
> > t = ndctl_decode_smart_temperature(temp);
> > jobj = json_object_new_double(t);
> > if (jobj)
> > json_object_object_add(jhealth,
> > - "controller_temperature_threshold", jobj);
> > + "ctrl_temperature_threshold", jobj);
> > + } else {
> > + jobj = json_object_new_boolean(false);
> > + if (jobj)
> > + json_object_object_add(jhealth,
> > + "alarm_enable_ctrl_temperature", jobj);
> > }
> >
> > if (alarm_control & ND_SMART_SPARE_TRIP) {
> > unsigned int spares;
> >
> > + jobj = json_object_new_boolean(true);
> > + if (jobj)
> > + json_object_object_add(jhealth,
> > + "alarm_enable_spares", jobj);
> > spares = ndctl_cmd_smart_threshold_get_spares(cmd);
> > jobj = json_object_new_int(spares);
> > if (jobj)
> > json_object_object_add(jhealth,
> > "spares_threshold", jobj);
> > + } else {
> > + jobj = json_object_new_boolean(false);
> > + if (jobj)
> > + json_object_object_add(jhealth,
> > + "alarm_enable_spares", jobj);
> > }
> >
> > out:
> > @@ -118,7 +145,8 @@ struct json_object
> > *util_dimm_health_to_json(struct ndctl_dimm *dimm)
> >
> > jobj = json_object_new_double(t);
> > if (jobj)
> > - json_object_object_add(jhealth, "temperature_celsius",
> jobj);
> > + json_object_object_add(jhealth,
> > + "media_temperature_celsius", jobj);
> > }
> >
> > if (flags & ND_SMART_CTEMP_VALID) {
> > @@ -128,7
RE: [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log messages in log_file()
> -Original Message-
> From: Masayoshi Mizuma [mailto:msys.miz...@gmail.com]
> Sent: Wednesday, August 8, 2018 3:40 AM
> To: Qi, Fuli/斉 福利 ; linux-nvdimm@lists.01.org
> Subject: Re: [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log
> messages
> in log_file()
>
> Hi Qi,
>
> On 08/07/2018 09:17 AM, QI Fuli wrote:
> > This patch is used to add timestamp and process id to log messages
> > when logging messages to a file.
> >
> > Signed-off-by: QI Fuli
> > ---
> > ndctl/monitor.c | 11 ++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index 2f3d751..d29e378
> > 100644
> > --- a/ndctl/monitor.c
> > +++ b/ndctl/monitor.c
> > @@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int
> > priority, const char *file, {
> > FILE *f;
> > char *buf;
> > + struct timespec ts;
> > + char timestamp[32];
> >
> > if (vasprintf(, format, args) < 0) {
> > fail("vasprintf error\n");
> > @@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int
> > priority, const
> char *file,
> > notice(ctx, "%s\n", buf);
> > goto end;
> > }
> > - fprintf(f, "%s", buf);
> > +
> > + if (priority != LOG_NOTICE) {
>
Hi Masa,
Thanks for your comments.
> Why is the timestamp not needed in case of LOG_NOTICE...?
Because LOG_NOTICE level is only used for smart event notification in monitor.
Since the timestamp and pid are already added to json format messages by
notify_dimm_event(),
so there is no need to add them again.
>
> > + clock_gettime(CLOCK_REALTIME, );
> > + sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec);
> > + fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf);
>
> What is the pid for...?
>
Multiple monitor can be run at the same time and they may send messages to a
common log file.
In this case, the pid could help users to know the monitor where each message
came from.
Thanks,
QI
> Thanks,
> Masa
>
> > + } else
> > + fprintf(f, "%s", buf);
> > +> fflush(f);
> > fclose(f);
> > end:
> >
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap.
On 08/02/2018 02:32 AM, Zhang,Yi wrote:
>
>
> On 2018年08月02日 03:40, Dave Jiang wrote:
>>
>> On 07/31/2018 04:46 AM, Zhang Yi wrote:
>>> It should be prevent user map an illegal vma range which larger than
>>> dax device phiscal resourse, as we don't have swap logic while page
>>> faulting in dax device.
>> This patch prevents a user mapping an illegal vma range that is larger
>> than a dax device physical resource.
>>
>>> Applications, especailly qemu, map the /dev/dax for virtual nvdimm's
>>> backend device, we defined the v-nvdimm label area at the end of mapped
>>> rang. By using an illegal size that exceeds the physical resource of
>>> /dev/dax, then it will triger qemu a signal fault while accessing these
>>> label area.
>> When qemu maps the dax device for virtual nvdimm's backend device, the
>> v-nvdimm label area is defined at the end of mapped range. By using an
>> illegal size that exceeds the range of the device dax, it will trigger a
>> fault with qemu.
> Thanks Dava, that's could be much better.
>>
>>> Signed-off-by: Zhang Yi
>>> ---
>>> drivers/dax/device.c | 28
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
>>> index aff2c15..c9a50cd 100644
>>> --- a/drivers/dax/device.c
>>> +++ b/drivers/dax/device.c
>>> @@ -177,6 +177,32 @@ static const struct attribute_group
>>> *dax_attribute_groups[] = {
>>> NULL,
>>> };
>>>
>>> +static int check_vma_range(struct dev_dax *dev_dax, struct vm_area_struct
>>> *vma,
>>> + const char *func)
>>> +{
>>> + struct device *dev = _dax->dev;
>>> + struct resource *res;
>>> + unsigned long size;
>>> + int ret, i;
>>> +
>>> + if (!dax_alive(dev_dax->dax_dev))
>>> + return -ENXIO;
>>> +
>>> + size = vma->vm_end - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
>>> + ret = -EINVAL;
>>> + for (i = 0; i < dev_dax->num_resources; i++) {
>>> + res = _dax->res[i];
>>> + if (size > resource_size(res)) {
>>> + dev_info(dev, "%s: %s: fail, vma range is overflow\n",
>>> + current->comm, func);
>>> + ret = -EINVAL;
>>> + continue;
>>> + } else
>>> + return 0;
>>> + }
>>> + return ret;
>>> +}
>>> +
>>> static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>>> const char *func)
>>> {
>>> @@ -465,6 +491,8 @@ static int dax_mmap(struct file *filp, struct
>>> vm_area_struct *vma)
>>> */
>>> id = dax_read_lock();
>>> rc = check_vma(dev_dax, vma, __func__);
>>> + if (!rc)
>>> + rc |= check_vma_range(dev_dax, vma, __func__);
I don't see any reason to logical or the return code. It should be 0, so
you can just assign it to check_vma_range().
>> I think you want to augment check_vma() rather than adding another
>> function? If this is added inside check_vma() then you can also skip the
>> !dax_alive() check. Do you expect this function to be called anywhere else?
> since check_vma range also will be called while dax page faulting. I
> don't wanna this check_vma_range introduce some additional workload
> while page fault. just let it checked in the mmap scenario
Good point. Ok.
>>
>>> dax_read_unlock(id);
>>> if (rc)
>>> return rc;
>>>
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 1/2] Create Intel pmem shutdown latch rule
On Tue, 2018-08-07 at 16:26 -0600, Keith Busch wrote:
> This patch provides a udev rule and the program it runs for Intel
> nvdimms. The program the rule executes runs two commands to allow
> applications to manage unsafe shutdowns.
>
> The first command enables the shutdown latch. Without this, the last
> shutdown status will remain fixed to the status set when the latch was
> last enabled.
>
> The second command retrieves the unclean shutdown counts and saves it
> in a known location. Only root can access the health's shutdown count,
> so we have to stash it somewhere accessible to non-privileged users. A
> successful execution of the rule will write USC to the run time tmpfs
> location. By default, the location will be set to:
>
> /run/ndctl//usc
>
> A distro may change this location using the '--with-tmpfsdir=[DIR]'.
>
> Reading the file will report the count observed when the dimm was
> added.
>
> Signed-off-by: Keith Busch
> ---
> .gitignore | 1 +
> Makefile.am | 3 +
> configure.ac| 10
> contrib/80-intel-pmem.rules | 1 +
> ndctl.spec.in | 5 +-
> ndctl/Makefile.am | 5 ++
> ndctl/latch-shutdown.c | 140
>
> 7 files changed, 164 insertions(+), 1 deletion(-)
> create mode 100644 contrib/80-intel-pmem.rules
> create mode 100644 ndctl/latch-shutdown.c
>
> diff --git a/.gitignore b/.gitignore
> index 1016b3b..1601738 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -25,6 +25,7 @@ daxctl/lib/libdaxctl.pc
> *.a
> ndctl/lib/libndctl.pc
> ndctl/ndctl
> +ndctl/latch-shutdown
> rhel/
> sles/ndctl.spec
> util/log.lo
> diff --git a/Makefile.am b/Makefile.am
> index e0c463a..9d3913e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,9 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
> dist_bashcompletion_DATA = contrib/ndctl
> endif
>
> +udevrulesdir = $(UDEVDIR)/rules.d
> +dist_udevrules_DATA = contrib/80-intel-pmem.rules
> +
> noinst_LIBRARIES = libccan.a
> libccan_a_SOURCES = \
> ccan/str/str.h \
> diff --git a/configure.ac b/configure.ac
> index cf44260..15f336b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -143,7 +143,16 @@ AC_CHECK_FUNCS([ \
> secure_getenv\
> ])
>
> +AC_ARG_WITH([tmpfsdir],
> + [AS_HELP_STRING([--with-tmpfsdir=DIR], [Directory for temporary runtime
> files])],
> + [tmpfsdir=$withval],
> + [tmpfsdir="/run"])
> +
> +UDEVDIR="$(pkg-config udev --variable=udevdir)"
> +AC_SUBST([UDEVDIR])
> +
> my_CFLAGS="\
> +-D DEF_TMPFS_DIR='\"${tmpfsdir}/ndctl\"' \
> -Wall \
> -Wchar-subscripts \
> -Wformat-security \
> @@ -182,6 +191,7 @@ AC_MSG_RESULT([
>
> prefix: ${prefix}
> sysconfdir: ${sysconfdir}
> +tmpfsdir: ${tmpfsdir}
> libdir: ${libdir}
> includedir: ${includedir}
>
> diff --git a/contrib/80-intel-pmem.rules b/contrib/80-intel-pmem.rules
> new file mode 100644
> index 000..20fcef9
> --- /dev/null
> +++ b/contrib/80-intel-pmem.rules
> @@ -0,0 +1 @@
> +ACTION=="add", KERNEL=="nmem*", RUN+="latch-shutdown $kernel"
> diff --git a/ndctl.spec.in b/ndctl.spec.in
> index e2c879c..25edf7e 100644
> --- a/ndctl.spec.in
> +++ b/ndctl.spec.in
> @@ -90,7 +90,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-tmpfsdir=%{_tmpfilesdir}
One more thing - looks like tmpfilesdir expands to:
$ rpm --eval "%{_tmpfilesdir}"
/usr/lib/tmpfiles.d
And using it requires a specific config file as described in the
following places:
https://fedoraproject.org/wiki/Packaging:Tmpfiles.d
http://0pointer.de/public/systemd-man/tmpfiles.d.html
So for this, I think we can simply drop the
--with-tmpfsdir=%{_tmpfilesdir}
to configure above, and let autoconf use its default. If a distro
wanted to change that location, they still have the option of doing it
be passing something else to configure.
> make %{?_smp_mflags}
>
> %install
> @@ -109,6 +109,7 @@ make check
> %postun -n DAX_LNAME -p /sbin/ldconfig
>
> %define bashcompdir %(pkg-config --variable=completionsdir bash-completion)
> +%define udevdir %(pkg-config --variable=udevdir udev)
>
> %files
> %defattr(-,root,root)
> @@ -116,6 +117,8 @@ make check
> %{_bindir}/ndctl
> %{_mandir}/man1/ndctl*
> %{bashcompdir}/
> +%{_udevrulesdir}/80-intel-pmem.rules
> +%{udevdir}/latch-shutdown
>
> %files -n daxctl
> %defattr(-,root,root)
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index 0f56871..155179b 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -41,3 +41,8 @@ ndctl_SOURCES += ../test/libndctl.c \
>../test/core.c \
>test.c
> endif
> +
> +latch_shutdowndir = $(UDEVDIR)
> +latch_shutdown_PROGRAMS = latch-shutdown
>
Re: [PATCH 2/2] ndctl, intel: Fallback to smart cached shutdown_count
On Tue, 2018-08-07 at 16:26 -0600, Keith Busch wrote:
> A user space rule saves the unsafe shutdown count to a fixed location
> for Intel dimms so that non-root users can read it. If the smart command
> submission fails, this patch uses that saved value in a newly added
> ndctl_cmd "handle_error" callback. If the cached value is available,
> the error handler will override the command status to success, and set
> the valid flags for the shutdown count.
>
> Signed-off-by: Keith Busch
> ---
> ndctl/lib/intel.c| 34 +-
> ndctl/lib/libndctl.c | 2 ++
> ndctl/lib/private.h | 1 +
> 3 files changed, 36 insertions(+), 1 deletion(-)
Looks good to me!
>
> diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
> index 0abea1e..83f8203 100644
> --- a/ndctl/lib/intel.c
> +++ b/ndctl/lib/intel.c
> @@ -56,6 +56,38 @@ static struct ndctl_cmd *alloc_intel_cmd(struct ndctl_dimm
> *dimm,
> return cmd;
> }
>
> +/*
> + * If provided, read the cached shutdown count in case the user does not have
> + * access rights to run the smart command, and pretend the command was
> + * successful with only the shutdown_count valid.
> + */
> +static int intel_smart_handle_error(struct ndctl_cmd *cmd)
> +{
> + struct ndctl_dimm *dimm = cmd->dimm;
> + char *path = NULL, shutdown_count[16] = {};
> + int fd, rc = cmd->status;
> +
> + if (asprintf(, DEF_TMPFS_DIR "/%s/usc",
> + ndctl_dimm_get_unique_id(dimm)) < 0)
> + return rc;
> +
> + fd = open(path, O_RDONLY);
> + if (fd < 0)
> + goto free_path;
> +
> + if (read(fd, shutdown_count, sizeof(shutdown_count)) < 0)
> + goto close;
> +
> + cmd->intel->smart.flags = ND_INTEL_SMART_SHUTDOWN_COUNT_VALID;
> + cmd->intel->smart.shutdown_count = strtoull(shutdown_count, NULL, 0);
> + rc = cmd->status = 0;
> + close:
> + close (fd);
> + free_path:
> + free(path);
> + return rc;
> +}
> +
> static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
> {
> struct ndctl_cmd *cmd;
> @@ -67,7 +99,7 @@ static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct
> ndctl_dimm *dimm)
> if (!cmd)
> return NULL;
> cmd->firmware_status = >intel->smart.status;
> -
> + cmd->handle_error = intel_smart_handle_error;
> return cmd;
> }
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 47e005e..d94321e 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -2651,6 +2651,8 @@ NDCTL_EXPORT int ndctl_cmd_submit(struct ndctl_cmd *cmd)
> close(fd);
> out:
> cmd->status = rc;
> + if (rc && cmd->handle_error)
> + rc = cmd->handle_error(cmd);
> return rc;
> }
>
> diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> index a94f894..5812c85 100644
> --- a/ndctl/lib/private.h
> +++ b/ndctl/lib/private.h
> @@ -255,6 +255,7 @@ struct ndctl_cmd {
> int dir;
> } iter;
> struct ndctl_cmd *source;
> + int (*handle_error)(struct ndctl_cmd *cmd);
> union {
> struct nd_cmd_ars_cap ars_cap[0];
> struct nd_cmd_ars_start ars_start[0];
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 1/2] Create Intel pmem shutdown latch rule
On Tue, 2018-08-07 at 16:26 -0600, Keith Busch wrote:
> This patch provides a udev rule and the program it runs for Intel
Like we talked about, I think this should work for all NVDIMMs (at
least any that use the generic DIMM ops APIs that libndctl provides),
so we can remove any Intel specificity here.
> nvdimms. The program the rule executes runs two commands to allow
> applications to manage unsafe shutdowns.
>
> The first command enables the shutdown latch. Without this, the last
> shutdown status will remain fixed to the status set when the latch was
> last enabled.
>
> The second command retrieves the unclean shutdown counts and saves it
> in a known location. Only root can access the health's shutdown count,
> so we have to stash it somewhere accessible to non-privileged users. A
> successful execution of the rule will write USC to the run time tmpfs
> location. By default, the location will be set to:
>
> /run/ndctl//usc
>
> A distro may change this location using the '--with-tmpfsdir=[DIR]'.
For consistency with the rpm macro, I'd suggest changing this to
--with-tmpfilesdir=
>
> Reading the file will report the count observed when the dimm was
> added.
>
> Signed-off-by: Keith Busch
> ---
> .gitignore | 1 +
> Makefile.am | 3 +
> configure.ac| 10
> contrib/80-intel-pmem.rules | 1 +
> ndctl.spec.in | 5 +-
> ndctl/Makefile.am | 5 ++
> ndctl/latch-shutdown.c | 140
>
I think we can also rename this file to something more generic like
"ndctl-udev.c" so that any future udev actions can go here. We are
already doing two things in it, latching and saving the USCs.
> 7 files changed, 164 insertions(+), 1 deletion(-)
> create mode 100644 contrib/80-intel-pmem.rules
> create mode 100644 ndctl/latch-shutdown.c
>
> diff --git a/.gitignore b/.gitignore
> index 1016b3b..1601738 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -25,6 +25,7 @@ daxctl/lib/libdaxctl.pc
> *.a
> ndctl/lib/libndctl.pc
> ndctl/ndctl
> +ndctl/latch-shutdown
> rhel/
> sles/ndctl.spec
> util/log.lo
> diff --git a/Makefile.am b/Makefile.am
> index e0c463a..9d3913e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,9 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
> dist_bashcompletion_DATA = contrib/ndctl
> endif
>
> +udevrulesdir = $(UDEVDIR)/rules.d
> +dist_udevrules_DATA = contrib/80-intel-pmem.rules
> +
> noinst_LIBRARIES = libccan.a
> libccan_a_SOURCES = \
> ccan/str/str.h \
> diff --git a/configure.ac b/configure.ac
> index cf44260..15f336b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -143,7 +143,16 @@ AC_CHECK_FUNCS([ \
> secure_getenv\
> ])
>
> +AC_ARG_WITH([tmpfsdir],
> + [AS_HELP_STRING([--with-tmpfsdir=DIR], [Directory for temporary runtime
> files])],
> + [tmpfsdir=$withval],
> + [tmpfsdir="/run"])
> +
> +UDEVDIR="$(pkg-config udev --variable=udevdir)"
> +AC_SUBST([UDEVDIR])
> +
> my_CFLAGS="\
> +-D DEF_TMPFS_DIR='\"${tmpfsdir}/ndctl\"' \
> -Wall \
> -Wchar-subscripts \
> -Wformat-security \
> @@ -182,6 +191,7 @@ AC_MSG_RESULT([
>
> prefix: ${prefix}
> sysconfdir: ${sysconfdir}
> +tmpfsdir: ${tmpfsdir}
> libdir: ${libdir}
> includedir: ${includedir}
>
> diff --git a/contrib/80-intel-pmem.rules b/contrib/80-intel-pmem.rules
> new file mode 100644
> index 000..20fcef9
> --- /dev/null
> +++ b/contrib/80-intel-pmem.rules
> @@ -0,0 +1 @@
> +ACTION=="add", KERNEL=="nmem*", RUN+="latch-shutdown $kernel"
> diff --git a/ndctl.spec.in b/ndctl.spec.in
> index e2c879c..25edf7e 100644
> --- a/ndctl.spec.in
> +++ b/ndctl.spec.in
> @@ -90,7 +90,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-tmpfsdir=%{_tmpfilesdir}
> make %{?_smp_mflags}
>
> %install
> @@ -109,6 +109,7 @@ make check
> %postun -n DAX_LNAME -p /sbin/ldconfig
>
> %define bashcompdir %(pkg-config --variable=completionsdir bash-completion)
> +%define udevdir %(pkg-config --variable=udevdir udev)
>
> %files
> %defattr(-,root,root)
> @@ -116,6 +117,8 @@ make check
> %{_bindir}/ndctl
> %{_mandir}/man1/ndctl*
> %{bashcompdir}/
> +%{_udevrulesdir}/80-intel-pmem.rules
> +%{udevdir}/latch-shutdown
>
> %files -n daxctl
> %defattr(-,root,root)
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index 0f56871..155179b 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -41,3 +41,8 @@ ndctl_SOURCES += ../test/libndctl.c \
>../test/core.c \
>test.c
> endif
> +
> +latch_shutdowndir = $(UDEVDIR)
> +latch_shutdown_PROGRAMS = latch-shutdown
> +latch_shutdown_SOURCES = latch-shutdown.c
> +latch_shutdown_LDADD = lib/libndctl.la
> diff
Re: [PATCH] ACPI: nfit: adjust annotation for why return 0 if fail to find NFIT at startup
On 08/07/2018 12:25 PM, Verma, Vishal L wrote:
>
> On Tue, 2018-08-07 at 02:15 -0400, Ocean He wrote:
>> From: Ocean He
>
> Just some minor nits in commit message wording:
>
>>
>> Add detail explanation why it's ok to return 0 if fail to find NFIT at
> detailed ^for ^we ^an
>> startup, refer to chapter 9.20.2 NVDIMM Root Device in ACPI 6.2 spec.
> . Refer
>
> Perhaps Dave can fixup while applying..
Applied and updated.
>>
>> Signed-off-by: Ocean He
>> ---
>> v1: https://lists.01.org/pipermail/linux-nvdimm/2018-August/017311.html
>> v2: Per Vishal's comments of v1, it turns out that it's right to keep
>> "return 0" if NFIT not found at boot time. This v2 adjust annotation
>> to avoid misunderstanding of "return 0".
>
> Looks good, thanks for the update. Feel free to add:
> Reviewed-by: Vishal Verma
>
>>
>> drivers/acpi/nfit/core.c | 8 +++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> index 7c47900..1b982fe 100644
>> --- a/drivers/acpi/nfit/core.c
>> +++ b/drivers/acpi/nfit/core.c
>> @@ -3353,7 +3353,13 @@ static int acpi_nfit_add(struct acpi_device *adev)
>>
>> status = acpi_get_table(ACPI_SIG_NFIT, 0, );
>> if (ACPI_FAILURE(status)) {
>> -/* This is ok, we could have an nvdimm hotplugged later */
>> +/* The NVDIMM root device allows OS to trigger enumeration of
>> + * NVDIMMs through NFIT at boot time and re-enumeration at
>> + * root level via the _FIT method during runtime.
>> + * This is ok to return 0 here, we could have an nvdimm
>> + * hotplugged later and evaluate _FIT method which returns
>> + * data in the format of a series of NFIT Structures.
>> + */
>> dev_dbg(dev, "failed to find NFIT at startup\n");
>> return 0;
>> }
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH] libnvdimm, bus: check id immediately following ida_simple_get
On 08/03/2018 05:08 AM, Ocean He wrote:
> From: Ocean He
>
> The id check was not executed immediately following ida_simple_get. Just
> change the codes position, without function change.
>
> Signed-off-by: Ocean He
Applied.
> ---
> drivers/nvdimm/bus.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index 27902a8..ab28e7c 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -350,12 +350,12 @@ struct nvdimm_bus *nvdimm_bus_register(struct device
> *parent,
> INIT_LIST_HEAD(_bus->mapping_list);
> init_waitqueue_head(_bus->probe_wait);
> nvdimm_bus->id = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
> - mutex_init(_bus->reconfig_mutex);
> - badrange_init(_bus->badrange);
> if (nvdimm_bus->id < 0) {
> kfree(nvdimm_bus);
> return NULL;
> }
> + mutex_init(_bus->reconfig_mutex);
> + badrange_init(_bus->badrange);
> nvdimm_bus->nd_desc = nd_desc;
> nvdimm_bus->dev.parent = parent;
> nvdimm_bus->dev.release = nvdimm_bus_release;
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[PATCH 2/2] ndctl, intel: Fallback to smart cached shutdown_count
A user space rule saves the unsafe shutdown count to a fixed location
for Intel dimms so that non-root users can read it. If the smart command
submission fails, this patch uses that saved value in a newly added
ndctl_cmd "handle_error" callback. If the cached value is available,
the error handler will override the command status to success, and set
the valid flags for the shutdown count.
Signed-off-by: Keith Busch
---
ndctl/lib/intel.c| 34 +-
ndctl/lib/libndctl.c | 2 ++
ndctl/lib/private.h | 1 +
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index 0abea1e..83f8203 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -56,6 +56,38 @@ static struct ndctl_cmd *alloc_intel_cmd(struct ndctl_dimm
*dimm,
return cmd;
}
+/*
+ * If provided, read the cached shutdown count in case the user does not have
+ * access rights to run the smart command, and pretend the command was
+ * successful with only the shutdown_count valid.
+ */
+static int intel_smart_handle_error(struct ndctl_cmd *cmd)
+{
+ struct ndctl_dimm *dimm = cmd->dimm;
+ char *path = NULL, shutdown_count[16] = {};
+ int fd, rc = cmd->status;
+
+ if (asprintf(, DEF_TMPFS_DIR "/%s/usc",
+ndctl_dimm_get_unique_id(dimm)) < 0)
+ return rc;
+
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ goto free_path;
+
+ if (read(fd, shutdown_count, sizeof(shutdown_count)) < 0)
+ goto close;
+
+ cmd->intel->smart.flags = ND_INTEL_SMART_SHUTDOWN_COUNT_VALID;
+ cmd->intel->smart.shutdown_count = strtoull(shutdown_count, NULL, 0);
+ rc = cmd->status = 0;
+ close:
+ close (fd);
+ free_path:
+ free(path);
+ return rc;
+}
+
static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
{
struct ndctl_cmd *cmd;
@@ -67,7 +99,7 @@ static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct
ndctl_dimm *dimm)
if (!cmd)
return NULL;
cmd->firmware_status = >intel->smart.status;
-
+ cmd->handle_error = intel_smart_handle_error;
return cmd;
}
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 47e005e..d94321e 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2651,6 +2651,8 @@ NDCTL_EXPORT int ndctl_cmd_submit(struct ndctl_cmd *cmd)
close(fd);
out:
cmd->status = rc;
+ if (rc && cmd->handle_error)
+ rc = cmd->handle_error(cmd);
return rc;
}
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index a94f894..5812c85 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -255,6 +255,7 @@ struct ndctl_cmd {
int dir;
} iter;
struct ndctl_cmd *source;
+ int (*handle_error)(struct ndctl_cmd *cmd);
union {
struct nd_cmd_ars_cap ars_cap[0];
struct nd_cmd_ars_start ars_start[0];
--
2.14.4
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[PATCH 1/2] Create Intel pmem shutdown latch rule
This patch provides a udev rule and the program it runs for Intel
nvdimms. The program the rule executes runs two commands to allow
applications to manage unsafe shutdowns.
The first command enables the shutdown latch. Without this, the last
shutdown status will remain fixed to the status set when the latch was
last enabled.
The second command retrieves the unclean shutdown counts and saves it
in a known location. Only root can access the health's shutdown count,
so we have to stash it somewhere accessible to non-privileged users. A
successful execution of the rule will write USC to the run time tmpfs
location. By default, the location will be set to:
/run/ndctl//usc
A distro may change this location using the '--with-tmpfsdir=[DIR]'.
Reading the file will report the count observed when the dimm was
added.
Signed-off-by: Keith Busch
---
.gitignore | 1 +
Makefile.am | 3 +
configure.ac| 10
contrib/80-intel-pmem.rules | 1 +
ndctl.spec.in | 5 +-
ndctl/Makefile.am | 5 ++
ndctl/latch-shutdown.c | 140
7 files changed, 164 insertions(+), 1 deletion(-)
create mode 100644 contrib/80-intel-pmem.rules
create mode 100644 ndctl/latch-shutdown.c
diff --git a/.gitignore b/.gitignore
index 1016b3b..1601738 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,6 +25,7 @@ daxctl/lib/libdaxctl.pc
*.a
ndctl/lib/libndctl.pc
ndctl/ndctl
+ndctl/latch-shutdown
rhel/
sles/ndctl.spec
util/log.lo
diff --git a/Makefile.am b/Makefile.am
index e0c463a..9d3913e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,9 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
dist_bashcompletion_DATA = contrib/ndctl
endif
+udevrulesdir = $(UDEVDIR)/rules.d
+dist_udevrules_DATA = contrib/80-intel-pmem.rules
+
noinst_LIBRARIES = libccan.a
libccan_a_SOURCES = \
ccan/str/str.h \
diff --git a/configure.ac b/configure.ac
index cf44260..15f336b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -143,7 +143,16 @@ AC_CHECK_FUNCS([ \
secure_getenv\
])
+AC_ARG_WITH([tmpfsdir],
+ [AS_HELP_STRING([--with-tmpfsdir=DIR], [Directory for temporary runtime
files])],
+ [tmpfsdir=$withval],
+ [tmpfsdir="/run"])
+
+UDEVDIR="$(pkg-config udev --variable=udevdir)"
+AC_SUBST([UDEVDIR])
+
my_CFLAGS="\
+-D DEF_TMPFS_DIR='\"${tmpfsdir}/ndctl\"' \
-Wall \
-Wchar-subscripts \
-Wformat-security \
@@ -182,6 +191,7 @@ AC_MSG_RESULT([
prefix: ${prefix}
sysconfdir: ${sysconfdir}
+tmpfsdir: ${tmpfsdir}
libdir: ${libdir}
includedir: ${includedir}
diff --git a/contrib/80-intel-pmem.rules b/contrib/80-intel-pmem.rules
new file mode 100644
index 000..20fcef9
--- /dev/null
+++ b/contrib/80-intel-pmem.rules
@@ -0,0 +1 @@
+ACTION=="add", KERNEL=="nmem*", RUN+="latch-shutdown $kernel"
diff --git a/ndctl.spec.in b/ndctl.spec.in
index e2c879c..25edf7e 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -90,7 +90,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-tmpfsdir=%{_tmpfilesdir}
make %{?_smp_mflags}
%install
@@ -109,6 +109,7 @@ make check
%postun -n DAX_LNAME -p /sbin/ldconfig
%define bashcompdir %(pkg-config --variable=completionsdir bash-completion)
+%define udevdir %(pkg-config --variable=udevdir udev)
%files
%defattr(-,root,root)
@@ -116,6 +117,8 @@ make check
%{_bindir}/ndctl
%{_mandir}/man1/ndctl*
%{bashcompdir}/
+%{_udevrulesdir}/80-intel-pmem.rules
+%{udevdir}/latch-shutdown
%files -n daxctl
%defattr(-,root,root)
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 0f56871..155179b 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -41,3 +41,8 @@ ndctl_SOURCES += ../test/libndctl.c \
../test/core.c \
test.c
endif
+
+latch_shutdowndir = $(UDEVDIR)
+latch_shutdown_PROGRAMS = latch-shutdown
+latch_shutdown_SOURCES = latch-shutdown.c
+latch_shutdown_LDADD = lib/libndctl.la
diff --git a/ndctl/latch-shutdown.c b/ndctl/latch-shutdown.c
new file mode 100644
index 000..704fb8f
--- /dev/null
+++ b/ndctl/latch-shutdown.c
@@ -0,0 +1,140 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * mkdir_p
+ *
+ * Copied from util-linux lib/fileutils.c
+ */
+static int mkdir_p(const char *path, mode_t mode)
+{
+ char *p, *dir;
+ int rc = 0;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ dir = p = strdup(path);
+ if (!dir)
+ return -ENOMEM;
+
+ if (*p == '/')
+ p++;
+
+ while (p && *p) {
+ char *e = strchr(p, '/');
Re: [PATCH 1/2] Create Intel pmem shutdown latch rule
Oops, please ignore this set. Had the send-email pointing to the wrong version of this patch. Will send the correct one in a moment. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
[PATCH 2/2] ndctl, intel: Pre-initialize smart shutdown_count
A user space rule saves the unsafe shutdown count to a fixed location
for Intel dimms so that non-root users can read it. This patch uses that
saved value if it is available in case the command can't be executed due
to user permissions, allowing an application to detect if an unsafe
shutdown occured during a write.
Signed-off-by: Keith Busch
---
ndctl/lib/intel.c | 42 +-
ndctl/util/json-smart.c | 4
2 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index 0abea1e..6df5aff 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -56,6 +56,35 @@ static struct ndctl_cmd *alloc_intel_cmd(struct ndctl_dimm
*dimm,
return cmd;
}
+/*
+ * If provided, prime the shutdown count with the saved value in case the user
+ * does not have access rights to run the smart command.
+ */
+static void intel_prime_shutdown_count(struct ndctl_dimm *dimm,
+ struct ndctl_cmd *cmd)
+{
+ char *path = NULL, shutdown_count[16] = {};
+ int fd;
+
+ if (asprintf(, DEF_TMPFS_DIR "/%s/usc",
+ndctl_dimm_get_unique_id(dimm)) < 0)
+ return;
+
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ goto free_path;
+
+ if (read(fd, shutdown_count, sizeof(shutdown_count)) < 0)
+ goto close;
+
+ cmd->intel->smart.flags |= ND_INTEL_SMART_SHUTDOWN_COUNT_VALID;
+ cmd->intel->smart.shutdown_count = strtoull(shutdown_count, NULL, 0);
+ close:
+ close (fd);
+ free_path:
+ free(path);
+}
+
static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
{
struct ndctl_cmd *cmd;
@@ -67,7 +96,7 @@ static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct
ndctl_dimm *dimm)
if (!cmd)
return NULL;
cmd->firmware_status = >intel->smart.status;
-
+ intel_prime_shutdown_count(dimm, cmd);
return cmd;
}
@@ -95,9 +124,6 @@ static unsigned int intel_cmd_smart_get_flags(struct
ndctl_cmd *cmd)
unsigned int flags = 0;
unsigned int intel_flags;
- if (intel_smart_valid(cmd) < 0)
- return 0;
-
/* translate intel specific flags to libndctl api smart flags */
intel_flags = cmd->intel->smart.flags;
if (intel_flags & ND_INTEL_SMART_HEALTH_VALID)
@@ -142,13 +168,19 @@ static unsigned int intel_cmd_smart_get_health(struct
ndctl_cmd *cmd)
return health;
}
+static unsigned int intel_cmd_smart_get_shutdown_count(struct ndctl_cmd *cmd)
+{
+ if (cmd->intel->smart.flags & ND_INTEL_SMART_SHUTDOWN_COUNT_VALID)
+ return cmd->intel->smart.shutdown_count;
+ return UINT_MAX;
+}
+
intel_smart_get_field(cmd, media_temperature)
intel_smart_get_field(cmd, ctrl_temperature)
intel_smart_get_field(cmd, spares)
intel_smart_get_field(cmd, alarm_flags)
intel_smart_get_field(cmd, life_used)
intel_smart_get_field(cmd, shutdown_state)
-intel_smart_get_field(cmd, shutdown_count)
intel_smart_get_field(cmd, vendor_size)
static unsigned char *intel_cmd_smart_get_vendor_data(struct ndctl_cmd *cmd)
diff --git a/ndctl/util/json-smart.c b/ndctl/util/json-smart.c
index 6a1f294..5f43982 100644
--- a/ndctl/util/json-smart.c
+++ b/ndctl/util/json-smart.c
@@ -93,7 +93,6 @@ struct json_object *util_dimm_health_to_json(struct
ndctl_dimm *dimm)
jobj = json_object_new_string("unknown");
if (jobj)
json_object_object_add(jhealth, "health_state", jobj);
- goto out;
}
flags = ndctl_cmd_smart_get_flags(cmd);
@@ -189,8 +188,5 @@ struct json_object *util_dimm_health_to_json(struct
ndctl_dimm *dimm)
err:
json_object_put(jhealth);
jhealth = NULL;
- out:
- if (cmd)
- ndctl_cmd_unref(cmd);
return jhealth;
}
--
2.14.4
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[PATCH 1/2] Create Intel pmem shutdown latch rule
This patch provides a udev rule and the program it runs for Intel
nvdimms. The program the rule executes runs two commands to allow
applications to manage unsafe shutdowns.
The first command enables the shutdown latch. Without this, the last
shutdown status will remain fixed to the status set when the latch was
last enabled.
The second command retrieves the unclean shutdown counts and saves it
in a known location. Only root can access the health's shutdown count,
so we have to stash it somewhere accessible to non-privileged users. A
successful execution of the rule will write USC to the run time tmpfs
location. By default, the location will be set to:
/run/ndctl//usc
A distro may change this location using the '--with-tmpfsdir=[DIR]'.
Reading the file will report the count observed when the dimm was
added.
Signed-off-by: Keith Busch
---
.gitignore | 1 +
Makefile.am | 3 +
configure.ac| 10
contrib/80-intel-pmem.rules | 1 +
ndctl.spec.in | 5 +-
ndctl/Makefile.am | 5 ++
ndctl/latch-shutdown.c | 140
7 files changed, 164 insertions(+), 1 deletion(-)
create mode 100644 contrib/80-intel-pmem.rules
create mode 100644 ndctl/latch-shutdown.c
diff --git a/.gitignore b/.gitignore
index 1016b3b..1601738 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,6 +25,7 @@ daxctl/lib/libdaxctl.pc
*.a
ndctl/lib/libndctl.pc
ndctl/ndctl
+ndctl/latch-shutdown
rhel/
sles/ndctl.spec
util/log.lo
diff --git a/Makefile.am b/Makefile.am
index e0c463a..9d3913e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,9 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
dist_bashcompletion_DATA = contrib/ndctl
endif
+udevrulesdir = $(UDEVDIR)/rules.d
+dist_udevrules_DATA = contrib/80-intel-pmem.rules
+
noinst_LIBRARIES = libccan.a
libccan_a_SOURCES = \
ccan/str/str.h \
diff --git a/configure.ac b/configure.ac
index cf44260..15f336b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -143,7 +143,16 @@ AC_CHECK_FUNCS([ \
secure_getenv\
])
+AC_ARG_WITH([tmpfsdir],
+ [AS_HELP_STRING([--with-tmpfsdir=DIR], [Directory for temporary runtime
files])],
+ [tmpfsdir=$withval],
+ [tmpfsdir="/run"])
+
+UDEVDIR="$(pkg-config udev --variable=udevdir)"
+AC_SUBST([UDEVDIR])
+
my_CFLAGS="\
+-D DEF_TMPFS_DIR='\"${tmpfsdir}/ndctl\"' \
-Wall \
-Wchar-subscripts \
-Wformat-security \
@@ -182,6 +191,7 @@ AC_MSG_RESULT([
prefix: ${prefix}
sysconfdir: ${sysconfdir}
+tmpfsdir: ${tmpfsdir}
libdir: ${libdir}
includedir: ${includedir}
diff --git a/contrib/80-intel-pmem.rules b/contrib/80-intel-pmem.rules
new file mode 100644
index 000..20fcef9
--- /dev/null
+++ b/contrib/80-intel-pmem.rules
@@ -0,0 +1 @@
+ACTION=="add", KERNEL=="nmem*", RUN+="latch-shutdown $kernel"
diff --git a/ndctl.spec.in b/ndctl.spec.in
index e2c879c..25edf7e 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -90,7 +90,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-tmpfsdir=%{_tmpfilesdir}
make %{?_smp_mflags}
%install
@@ -109,6 +109,7 @@ make check
%postun -n DAX_LNAME -p /sbin/ldconfig
%define bashcompdir %(pkg-config --variable=completionsdir bash-completion)
+%define udevdir %(pkg-config --variable=udevdir udev)
%files
%defattr(-,root,root)
@@ -116,6 +117,8 @@ make check
%{_bindir}/ndctl
%{_mandir}/man1/ndctl*
%{bashcompdir}/
+%{_udevrulesdir}/80-intel-pmem.rules
+%{udevdir}/latch-shutdown
%files -n daxctl
%defattr(-,root,root)
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 0f56871..155179b 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -41,3 +41,8 @@ ndctl_SOURCES += ../test/libndctl.c \
../test/core.c \
test.c
endif
+
+latch_shutdowndir = $(UDEVDIR)
+latch_shutdown_PROGRAMS = latch-shutdown
+latch_shutdown_SOURCES = latch-shutdown.c
+latch_shutdown_LDADD = lib/libndctl.la
diff --git a/ndctl/latch-shutdown.c b/ndctl/latch-shutdown.c
new file mode 100644
index 000..704fb8f
--- /dev/null
+++ b/ndctl/latch-shutdown.c
@@ -0,0 +1,140 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * mkdir_p
+ *
+ * Copied from util-linux lib/fileutils.c
+ */
+static int mkdir_p(const char *path, mode_t mode)
+{
+ char *p, *dir;
+ int rc = 0;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ dir = p = strdup(path);
+ if (!dir)
+ return -ENOMEM;
+
+ if (*p == '/')
+ p++;
+
+ while (p && *p) {
+ char *e = strchr(p, '/');
[PATCH 1/2] ext4: Close race between direct IO and ext4_break_layouts()
From: Ross Zwisler
If the refcount of a page is lowered between the time that it is returned
by dax_busy_page() and when the refcount is again checked in
ext4_break_layouts() => ___wait_var_event(), the waiting function
ext4_wait_dax_page() will never be called. This means that
ext4_break_layouts() will still have 'retry' set to false, so we'll stop
looping and never check the refcount of other pages in this inode.
Instead, always continue looping as long as dax_layout_busy_page() gives us
a page which it found with an elevated refcount.
Note that this works around the race exposed by my unit test, but I think
that there is another race that needs to be addressed, probably with
additional synchronization added between direct I/O and
{ext4,xfs}_break_layouts().
Signed-off-by: Ross Zwisler
Reviewed-by: Jan Kara
---
fs/ext4/inode.c |9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8f6ad7667974..d2663a1e3ec2 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4191,9 +4191,8 @@ int ext4_update_disksize_before_punch(struct inode
*inode, loff_t offset,
return 0;
}
-static void ext4_wait_dax_page(struct ext4_inode_info *ei, bool *did_unlock)
+static void ext4_wait_dax_page(struct ext4_inode_info *ei)
{
- *did_unlock = true;
up_write(>i_mmap_sem);
schedule();
down_write(>i_mmap_sem);
@@ -4203,14 +4202,12 @@ int ext4_break_layouts(struct inode *inode)
{
struct ext4_inode_info *ei = EXT4_I(inode);
struct page *page;
- bool retry;
int error;
if (WARN_ON_ONCE(!rwsem_is_locked(>i_mmap_sem)))
return -EINVAL;
do {
- retry = false;
page = dax_layout_busy_page(inode->i_mapping);
if (!page)
return 0;
@@ -4218,8 +4215,8 @@ int ext4_break_layouts(struct inode *inode)
error = ___wait_var_event(>_refcount,
atomic_read(>_refcount) == 1,
TASK_INTERRUPTIBLE, 0, 0,
- ext4_wait_dax_page(ei, ));
- } while (error == 0 && retry);
+ ext4_wait_dax_page(ei));
+ } while (error == 0);
return error;
}
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[PATCH 2/2] [PATCH] xfs: Close race between direct IO and xfs_break_layouts()
This patch is the duplicate of ross's fix for ext4 for xfs.
If the refcount of a page is lowered between the time that it is returned
by dax_busy_page() and when the refcount is again checked in
xfs_break_layouts() => ___wait_var_event(), the waiting function
xfs_wait_dax_page() will never be called. This means that
xfs_break_layouts() will still have 'retry' set to false, so we'll stop
looping and never check the refcount of other pages in this inode.
Instead, always continue looping as long as dax_layout_busy_page() gives us
a page which it found with an elevated refcount.
Signed-off-by: Dave Jiang
---
fs/xfs/xfs_file.c |7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index a3e7767a5715..666c93fe5284 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -721,12 +721,10 @@ xfs_file_write_iter(
static void
xfs_wait_dax_page(
- struct inode*inode,
- bool*did_unlock)
+ struct inode*inode)
{
struct xfs_inode*ip = XFS_I(inode);
- *did_unlock = true;
xfs_iunlock(ip, XFS_MMAPLOCK_EXCL);
schedule();
xfs_ilock(ip, XFS_MMAPLOCK_EXCL);
@@ -746,9 +744,10 @@ xfs_break_dax_layouts(
if (!page)
return 0;
+ *did_unlock = true;
return ___wait_var_event(>_refcount,
atomic_read(>_refcount) == 1, TASK_INTERRUPTIBLE,
- 0, 0, xfs_wait_dax_page(inode, did_unlock));
+ 0, 0, xfs_wait_dax_page(inode));
}
int
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify misc list items
On Wed, 2018-08-08 at 00:35 +0900, QI Fuli wrote:
> This patch adds alarm_enable_ to list. Therefore, users can know
Hi Qi,
Thanks, I was meaning to do this work but you beat me to it :)
I just have a few nits, see below. But otherwise this looks good.
> if the "ndclt inject-smart ---alarm=on/off" works or not.
s/ndclt/ndctl/
>
> Users may confuse "alarm_enable" with "alarm_flag" and "media_temperature"
> with "ctrl_temperature", this patch also used to clarify these items.
I'm not sure it is a good idea to change these names since they've been
released for quite some time, and users/scripts might be depending on
them, which this change will break. Let's just add a new field for
alarm_enabled_* for each smart field, but not change any of the
existing ones for now.
>
> Signed-off-by: QI Fuli
> ---
> ndctl/util/json-smart.c | 45 ++---
> 1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/ndctl/util/json-smart.c b/ndctl/util/json-smart.c
> index 6a1f294..a0677a6 100644
> --- a/ndctl/util/json-smart.c
> +++ b/ndctl/util/json-smart.c
> @@ -39,34 +39,61 @@ static void smart_threshold_to_json(struct ndctl_dimm
> *dimm,
> unsigned int temp;
> double t;
>
> + jobj = json_object_new_boolean(true);
> + if (jobj)
> + json_object_object_add(jhealth,
> + "alarm_enable_media_temperature", jobj);
Lets reword all instances to alarm_enabled_*
Since it is showing the status of the alarm.
> temp = ndctl_cmd_smart_threshold_get_temperature(cmd);
> t = ndctl_decode_smart_temperature(temp);
> jobj = json_object_new_double(t);
> if (jobj)
> json_object_object_add(jhealth,
> - "temperature_threshold", jobj);
> + "media_temperature_threshold", jobj);
> + } else {
> + jobj = json_object_new_boolean(false);
> + if (jobj)
> + json_object_object_add(jhealth,
> + "alarm_enable_media_temperature", jobj);
> }
>
> if (alarm_control & ND_SMART_CTEMP_TRIP) {
> unsigned int temp;
> double t;
>
> + jobj = json_object_new_boolean(true);
> + if (jobj)
> + json_object_object_add(jhealth,
> + "alarm_enable_ctrl_temperature", jobj);
> temp = ndctl_cmd_smart_threshold_get_ctrl_temperature(cmd);
> t = ndctl_decode_smart_temperature(temp);
> jobj = json_object_new_double(t);
> if (jobj)
> json_object_object_add(jhealth,
> - "controller_temperature_threshold", jobj);
> + "ctrl_temperature_threshold", jobj);
> + } else {
> + jobj = json_object_new_boolean(false);
> + if (jobj)
> + json_object_object_add(jhealth,
> + "alarm_enable_ctrl_temperature", jobj);
> }
>
> if (alarm_control & ND_SMART_SPARE_TRIP) {
> unsigned int spares;
>
> + jobj = json_object_new_boolean(true);
> + if (jobj)
> + json_object_object_add(jhealth,
> + "alarm_enable_spares", jobj);
> spares = ndctl_cmd_smart_threshold_get_spares(cmd);
> jobj = json_object_new_int(spares);
> if (jobj)
> json_object_object_add(jhealth,
> "spares_threshold", jobj);
> + } else {
> + jobj = json_object_new_boolean(false);
> + if (jobj)
> + json_object_object_add(jhealth,
> + "alarm_enable_spares", jobj);
> }
>
> out:
> @@ -118,7 +145,8 @@ struct json_object *util_dimm_health_to_json(struct
> ndctl_dimm *dimm)
>
> jobj = json_object_new_double(t);
> if (jobj)
> - json_object_object_add(jhealth, "temperature_celsius",
> jobj);
> + json_object_object_add(jhealth,
> + "media_temperature_celsius", jobj);
> }
>
> if (flags & ND_SMART_CTEMP_VALID) {
> @@ -128,7 +156,7 @@ struct json_object *util_dimm_health_to_json(struct
> ndctl_dimm *dimm)
> jobj = json_object_new_double(t);
> if (jobj)
> json_object_object_add(jhealth,
> - "controller_temperature_celsius", jobj);
> + "ctrl_temperature_celsius", jobj);
> }
>
> if (flags & ND_SMART_SPARES_VALID) {
> @@ -147,15 +175,18 @@ struct json_object
Hello!
Dear Job Seeker, Position Name: Project Manager (Supply Chain); Company: DSC Logistics Functional area: Logistics Services; Area of Interest: Logistics Services; Salary: $98,000/year; Job Offer Available: 15th August 2018; Job Type: Permanent Position; Location: USA/15 States (can be telecommute) Travel Requirement: All in state The Position: This position is in the area of Project Management. You will manage all aspects of production planning and scheduling. The position requires a manager who can manage projects in different areas across the business such as procurement and become a liaison between company and our clients. Experience/Skills Required: - US citizen only! - Bachelor Degree - Highly proficient in MS software - Ability to multi-task - Excellent written and verbal skills - Desired management experience in performance management. If you are interested in the position, please respond with your CV and resume attached. Our contact: of...@logistictravels.info Thank you! ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified
Hi Qi,
On 08/07/2018 09:17 AM, QI Fuli wrote:
> When running monitor as a daemon, if the log destination is "standard" or
> a relative path for log file, the messages will not be able to be logged.
> Sometimes, users may not notice that the default log destination is "standard"
> when they start monitor daemon by systemctl, so they will lose messages.
> This patch is used to fix the unfriendly interface. When running monitor as a
> daemon, the default log destination will be changed to syslog. Also, the
> messages
> will be forwarded to syslog if the log destination is a relative path for log
> file.
>
> Signed-off-by: QI Fuli
> ---
> Documentation/ndctl/ndctl-monitor.txt | 16 +++-
> ndctl/monitor.c | 5 -
> ndctl/monitor.conf| 2 ++
> 3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ndctl/ndctl-monitor.txt
> b/Documentation/ndctl/ndctl-monitor.txt
> index 1cba9ea..9a8d76b 100644
> --- a/Documentation/ndctl/ndctl-monitor.txt
> +++ b/Documentation/ndctl/ndctl-monitor.txt
> @@ -67,7 +67,21 @@ OPTIONS
>
> -l ::
> --log=::
> - Output notifications to , syslog or standard output.
> + Send log messages to the specified destination.
> ++
> +--
> +::
> + Send log messages to specified . When fopen() is not able
> + to open , log messages will be forwarded to syslog.
> +syslog::
> + Send messages to syslog.
> +standard::
> + Send messages to standard output.
> +--
> ++
> +The default log destination is 'syslog' if "--daemon" is specified,
> +otherwise 'standard'. Note that standard and relative path for
> +will not work if "--daemon" is specified.
>
> -c::
> --config-file=::
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> index bf1f1d3..2f3d751 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority,
> const char *file,
> f = fopen(monitor.log, "a+");
> if (!f) {
> ndctl_set_log_fn(ctx, log_syslog);
> - err(ctx, "open logfile %s failed\n", monitor.log);
> + err(ctx, "open logfile %s failed, forward messages to syslog\n",
> + monitor.log);
> did_fail = 1;
> notice(ctx, "%s\n", buf);
> goto end;
> @@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> }
>
> if (monitor.daemon) {
Why don't you add './standard' check? Like as:
if (strncmp(monitor.log, "./standard", 10) == 0)
error("daemon doesn't work for 'standard' log option");
goto out;
Thanks,
Masa
> + if (!monitor.log || strncmp(monitor.log, "./", 2) == 0)
> + ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
> if (daemon(0, 0) != 0) {
> err((struct ndctl_ctx *)ctx, "daemon start failed\n");
> goto out;
> diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf
> index 857aadf..934e2c0 100644
> --- a/ndctl/monitor.conf
> +++ b/ndctl/monitor.conf
> @@ -38,4 +38,6 @@
> # to standard output (log=standard) or to write into a special file
> (log=)
> # by setting key "log". If this value is in conflict with the value of
> # [--log=] option, this value will be ignored.
> +# Note: Setting value to "standard" or relative path for will not work
> +# when running moniotr as a daemon.
> # log = /var/log/ndctl/monitor.log
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH] ndctl, documentation: Clarify the dimm id for ndctl list d option
On Mon, 2018-08-06 at 17:21 -0400, Masayoshi Mizuma wrote: > From: Masayoshi Mizuma > > User may be confused the dimm id description of ndctl list -d > option because ndctl list says id, but the id is not useful to > d option. > > ]# ndctl list -d nmem7 | jq -r .id > cdab-0a-07e0-fefffeff > ]# ndctl list -d cdab-0a-07e0-fefffeff > ]# > > The appropriate id here is X in 'nmemX' (Above example, X is 7). > Let's clarify the description. > > Signed-off-by: Masayoshi Mizuma > --- > Documentation/ndctl/ndctl-list.txt | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) Looks good, Applied. > > diff --git a/Documentation/ndctl/ndctl-list.txt > b/Documentation/ndctl/ndctl-list.txt > index 13ebdcd..1b5882a 100644 > --- a/Documentation/ndctl/ndctl-list.txt > +++ b/Documentation/ndctl/ndctl-list.txt > @@ -64,9 +64,10 @@ include::xable-region-options.txt[] > > -d:: > --dimm=:: > - An 'nmemX' device name, or dimm id number. Filter listing by > - devices that reference the given dimm. For example to see > all > - namespaces comprised of storage capacity on nmem0: > + An 'nmemX' device name, or dimm id number. The dimm id > number > + here is X in 'nmemX'. Filter listing by devices that > reference > + the given dimm. For example to see all namespaces comprised > + of storage capacity on nmem0: > > # ndctl list --dimm=nmem0 --namespaces > ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH] ACPI: nfit: adjust annotation for why return 0 if fail to find NFIT at startup
On Tue, 2018-08-07 at 02:15 -0400, Ocean He wrote:
> From: Ocean He
Just some minor nits in commit message wording:
>
> Add detail explanation why it's ok to return 0 if fail to find NFIT at
detailed ^for ^we ^an
> startup, refer to chapter 9.20.2 NVDIMM Root Device in ACPI 6.2 spec.
. Refer
Perhaps Dave can fixup while applying..
>
> Signed-off-by: Ocean He
> ---
> v1: https://lists.01.org/pipermail/linux-nvdimm/2018-August/017311.html
> v2: Per Vishal's comments of v1, it turns out that it's right to keep
> "return 0" if NFIT not found at boot time. This v2 adjust annotation
> to avoid misunderstanding of "return 0".
Looks good, thanks for the update. Feel free to add:
Reviewed-by: Vishal Verma
>
> drivers/acpi/nfit/core.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 7c47900..1b982fe 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -3353,7 +3353,13 @@ static int acpi_nfit_add(struct acpi_device *adev)
>
> status = acpi_get_table(ACPI_SIG_NFIT, 0, );
> if (ACPI_FAILURE(status)) {
> - /* This is ok, we could have an nvdimm hotplugged later */
> + /* The NVDIMM root device allows OS to trigger enumeration of
> + * NVDIMMs through NFIT at boot time and re-enumeration at
> + * root level via the _FIT method during runtime.
> + * This is ok to return 0 here, we could have an nvdimm
> + * hotplugged later and evaluate _FIT method which returns
> + * data in the format of a series of NFIT Structures.
> + */
> dev_dbg(dev, "failed to find NFIT at startup\n");
> return 0;
> }
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of invalid path of log file
Hi Qi,
On 08/07/2018 09:17 AM, QI Fuli wrote:
> Currently the monitor can be started even with an invalid path of log file.
> This patch adds a detection of invalid path of log file when starting monitor.
> If the path of log file is invalid, the monitor will refuse to be started.
>
> Signed-off-by: QI Fuli
> ---
> ndctl/monitor.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> index f10384b..bf1f1d3 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -603,6 +603,7 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> struct util_filter_ctx fctx = { 0 };
> struct monitor_filter_arg mfa = { 0 };
> int i, rc;
> + FILE *f;
>
> argc = parse_options_prefix(argc, argv, prefix, options, u, 0);
> for (i = 0; i < argc; i++) {
> @@ -630,8 +631,16 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
> else if (strncmp(monitor.log, "./standard", 10) == 0)
> ; /*default, already set */
> - else
> + else {
> + f = fopen(monitor.log, "a+");
> + if (!f) {
> + error("open %s failed\n", monitor.log);
> + rc = -errno;
> + goto out;
> + }
> + fclose(f);
> ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file);
> + }
In log_file(), the log file does fallback to syslog if the fopen() fails.
In my understanding here is that the fallback is needed to save in case of
the monitor.log in trouble for example, the parent directory is removed.
And, the new fopen() check, you have added by this patch, to inform the
invalid log path for users.
Is my understanding correct? If so, make sense to me.
Please feel free to add:
Reviewed-by: Masayoshi Mizuma
Thanks,
Masa
> }
>
> if (monitor.daemon) {
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH 4/4] ndctl, monitor: add [Install] Section to systemd unit file of ndctl-monitor
Hi Qi,
Looks good to me. Please free to add:
Reviewed-by: Masayoshi Mizuma
Thanks,
Masa
On 08/07/2018 09:17 AM, QI Fuli wrote:
> This patch is used to add "[Install]" section to systemd unit file of
> ndctl-monitor. Therefore, users can set ndctl-monitor unit enable.
>
> Signed-off-by: QI Fuli
> ---
> ndctl/ndctl-monitor.service | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
> index 44f9326..342a1b1 100644
> --- a/ndctl/ndctl-monitor.service
> +++ b/ndctl/ndctl-monitor.service
> @@ -5,3 +5,6 @@ Description=Ndctl Monitor Daemon
> Type=forking
> ExecStart=/usr/bin/ndctl monitor --daemon
> ExecStop=/bin/kill ${MAINPID}
> +
> +[Install]
> +WantedBy=multi-user.target
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log messages in log_file()
Hi Qi,
On 08/07/2018 09:17 AM, QI Fuli wrote:
> This patch is used to add timestamp and process id to log messages when
> logging
> messages to a file.
>
> Signed-off-by: QI Fuli
> ---
> ndctl/monitor.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> index 2f3d751..d29e378 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority,
> const char *file,
> {
> FILE *f;
> char *buf;
> + struct timespec ts;
> + char timestamp[32];
>
> if (vasprintf(, format, args) < 0) {
> fail("vasprintf error\n");
> @@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int priority,
> const char *file,
> notice(ctx, "%s\n", buf);
> goto end;
> }
> - fprintf(f, "%s", buf);
> +
> + if (priority != LOG_NOTICE) {
Why is the timestamp not needed in case of LOG_NOTICE...?
> + clock_gettime(CLOCK_REALTIME, );
> + sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec);
> + fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf);
What is the pid for...?
Thanks,
Masa
> + } else
> + fprintf(f, "%s", buf);
> +>fflush(f);
> fclose(f);
> end:
>
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
转发：<合伙人制度> 8月11日·苏州站
转发邮件信息 发件人：nkk...@gxw.net 发送日期：2018-8-8 0:35:13 收件人：linux-nvdimm@lists.01.org 全国畅销书<合伙人制度> 企业家，创业者，成功者创业路上必看！ 作者：郑指梁老师来昆山现场指导了！！ 邀请对象：企业董事长、总经理、集团总裁、资源大咖、投资大咖、创业者、有项目需要合伙者等。 企业是否面临以下难题: 1、在企业发展过程中优秀员工与高管不断流失，外部人才难进入，如何通过合伙人制度解决? 2、在企业初创期如何实现新员工梦想、调动老员工动力、解决企业元老发挥余热？ 3、企业老板知道（期股、岗位股、技术股、业绩股、分红股、期权等）股权激励，但是具体不知道怎么做？如何平衡和设立切实可行的方案? 4、企业家面临投资人投资，公司估值如何测算？保留自己留下的‘江山’不被投资者独占? 5、在企业发展过程中，企业上下游无法形成产业链的共赢格局？无法形成利益共同体？如何通过合伙人制度设定一项好的方式‘报团’做大做强! 您是否急切需要解决以下问题？ 1、如何设计自己公司股东合伙人方案？ 2、给了股权，怎么设计（出资、估值、分钱、退出）? 3、如何引进投资人而不丧失控股权，公司估值如何测算？ 4、如何让员工当老板，解决员工自驱力问题？ 5、如何解决一线部门“吃肉”，职能部门“喝汤”的难题？ 6、员工有钱、有资源，如何设计裂变式创业方案？ 7、连锁门店如何通过事业合伙留住和复制人才？ 9、如何设计员工的持股平台？ 10、如何让客户、经销商变成合伙人，做大公司业绩而无利益输送的担忧？ ★★★全景案例背后是什么，现场为您解密！ - ★活动时间：8月11日·苏州站（星期六）9:00—17:00 地点：苏州独墅湖书香世家酒店（苏州吴中区通达路2699号，近星港街） ★活动时间：9月15日南京（星期六）9:00—17:00 ★活动时间：9月16日上海（星期日）9:00—17:00 - 报名费：680元/位 （含午餐，发票） 可微信/支付宝转账，若需要发票请备注开票信息。 报名电话：18890700600〔微信同号〕 ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
[ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify misc list items
This patch adds alarm_enable_ to list. Therefore, users can know
if the "ndclt inject-smart ---alarm=on/off" works or not.
Users may confuse "alarm_enable" with "alarm_flag" and "media_temperature"
with "ctrl_temperature", this patch also used to clarify these items.
Signed-off-by: QI Fuli
---
ndctl/util/json-smart.c | 45 ++---
1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/ndctl/util/json-smart.c b/ndctl/util/json-smart.c
index 6a1f294..a0677a6 100644
--- a/ndctl/util/json-smart.c
+++ b/ndctl/util/json-smart.c
@@ -39,34 +39,61 @@ static void smart_threshold_to_json(struct ndctl_dimm *dimm,
unsigned int temp;
double t;
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_media_temperature", jobj);
temp = ndctl_cmd_smart_threshold_get_temperature(cmd);
t = ndctl_decode_smart_temperature(temp);
jobj = json_object_new_double(t);
if (jobj)
json_object_object_add(jhealth,
- "temperature_threshold", jobj);
+ "media_temperature_threshold", jobj);
+ } else {
+ jobj = json_object_new_boolean(false);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_media_temperature", jobj);
}
if (alarm_control & ND_SMART_CTEMP_TRIP) {
unsigned int temp;
double t;
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_ctrl_temperature", jobj);
temp = ndctl_cmd_smart_threshold_get_ctrl_temperature(cmd);
t = ndctl_decode_smart_temperature(temp);
jobj = json_object_new_double(t);
if (jobj)
json_object_object_add(jhealth,
- "controller_temperature_threshold", jobj);
+ "ctrl_temperature_threshold", jobj);
+ } else {
+ jobj = json_object_new_boolean(false);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_ctrl_temperature", jobj);
}
if (alarm_control & ND_SMART_SPARE_TRIP) {
unsigned int spares;
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_spares", jobj);
spares = ndctl_cmd_smart_threshold_get_spares(cmd);
jobj = json_object_new_int(spares);
if (jobj)
json_object_object_add(jhealth,
"spares_threshold", jobj);
+ } else {
+ jobj = json_object_new_boolean(false);
+ if (jobj)
+ json_object_object_add(jhealth,
+ "alarm_enable_spares", jobj);
}
out:
@@ -118,7 +145,8 @@ struct json_object *util_dimm_health_to_json(struct
ndctl_dimm *dimm)
jobj = json_object_new_double(t);
if (jobj)
- json_object_object_add(jhealth, "temperature_celsius",
jobj);
+ json_object_object_add(jhealth,
+ "media_temperature_celsius", jobj);
}
if (flags & ND_SMART_CTEMP_VALID) {
@@ -128,7 +156,7 @@ struct json_object *util_dimm_health_to_json(struct
ndctl_dimm *dimm)
jobj = json_object_new_double(t);
if (jobj)
json_object_object_add(jhealth,
- "controller_temperature_celsius", jobj);
+ "ctrl_temperature_celsius", jobj);
}
if (flags & ND_SMART_SPARES_VALID) {
@@ -147,15 +175,18 @@ struct json_object *util_dimm_health_to_json(struct
ndctl_dimm *dimm)
jobj = json_object_new_boolean(temp_flag);
if (jobj)
- json_object_object_add(jhealth, "alarm_temperature",
jobj);
+ json_object_object_add(jhealth,
+ "alarm_flag_media_temperature", jobj);
jobj = json_object_new_boolean(ctrl_temp_flag);
if (jobj)
- json_object_object_add(jhealth,
"alarm_controller_temperature", jobj);
+ json_object_object_add(jhealth,
+ "alarm_flag_ctrl_temperature", jobj);
[ndctl PATCH 4/4] ndctl, monitor: add [Install] Section to systemd unit file of ndctl-monitor
This patch is used to add "[Install]" section to systemd unit file of
ndctl-monitor. Therefore, users can set ndctl-monitor unit enable.
Signed-off-by: QI Fuli
---
ndctl/ndctl-monitor.service | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
index 44f9326..342a1b1 100644
--- a/ndctl/ndctl-monitor.service
+++ b/ndctl/ndctl-monitor.service
@@ -5,3 +5,6 @@ Description=Ndctl Monitor Daemon
Type=forking
ExecStart=/usr/bin/ndctl monitor --daemon
ExecStop=/bin/kill ${MAINPID}
+
+[Install]
+WantedBy=multi-user.target
--
2.18.0
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified
When running monitor as a daemon, if the log destination is "standard" or
a relative path for log file, the messages will not be able to be logged.
Sometimes, users may not notice that the default log destination is "standard"
when they start monitor daemon by systemctl, so they will lose messages.
This patch is used to fix the unfriendly interface. When running monitor as a
daemon, the default log destination will be changed to syslog. Also, the
messages
will be forwarded to syslog if the log destination is a relative path for log
file.
Signed-off-by: QI Fuli
---
Documentation/ndctl/ndctl-monitor.txt | 16 +++-
ndctl/monitor.c | 5 -
ndctl/monitor.conf| 2 ++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/Documentation/ndctl/ndctl-monitor.txt
b/Documentation/ndctl/ndctl-monitor.txt
index 1cba9ea..9a8d76b 100644
--- a/Documentation/ndctl/ndctl-monitor.txt
+++ b/Documentation/ndctl/ndctl-monitor.txt
@@ -67,7 +67,21 @@ OPTIONS
-l ::
--log=::
- Output notifications to , syslog or standard output.
+ Send log messages to the specified destination.
++
+--
+::
+ Send log messages to specified . When fopen() is not able
+ to open , log messages will be forwarded to syslog.
+syslog::
+ Send messages to syslog.
+standard::
+ Send messages to standard output.
+--
++
+The default log destination is 'syslog' if "--daemon" is specified,
+otherwise 'standard'. Note that standard and relative path for
+will not work if "--daemon" is specified.
-c::
--config-file=::
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index bf1f1d3..2f3d751 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority,
const char *file,
f = fopen(monitor.log, "a+");
if (!f) {
ndctl_set_log_fn(ctx, log_syslog);
- err(ctx, "open logfile %s failed\n", monitor.log);
+ err(ctx, "open logfile %s failed, forward messages to syslog\n",
+ monitor.log);
did_fail = 1;
notice(ctx, "%s\n", buf);
goto end;
@@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
}
if (monitor.daemon) {
+ if (!monitor.log || strncmp(monitor.log, "./", 2) == 0)
+ ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
if (daemon(0, 0) != 0) {
err((struct ndctl_ctx *)ctx, "daemon start failed\n");
goto out;
diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf
index 857aadf..934e2c0 100644
--- a/ndctl/monitor.conf
+++ b/ndctl/monitor.conf
@@ -38,4 +38,6 @@
# to standard output (log=standard) or to write into a special file
(log=)
# by setting key "log". If this value is in conflict with the value of
# [--log=] option, this value will be ignored.
+# Note: Setting value to "standard" or relative path for will not work
+# when running moniotr as a daemon.
# log = /var/log/ndctl/monitor.log
--
2.18.0
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log messages in log_file()
This patch is used to add timestamp and process id to log messages when logging
messages to a file.
Signed-off-by: QI Fuli
---
ndctl/monitor.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index 2f3d751..d29e378 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority,
const char *file,
{
FILE *f;
char *buf;
+ struct timespec ts;
+ char timestamp[32];
if (vasprintf(, format, args) < 0) {
fail("vasprintf error\n");
@@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int priority,
const char *file,
notice(ctx, "%s\n", buf);
goto end;
}
- fprintf(f, "%s", buf);
+
+ if (priority != LOG_NOTICE) {
+ clock_gettime(CLOCK_REALTIME, );
+ sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec);
+ fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf);
+ } else
+ fprintf(f, "%s", buf);
+
fflush(f);
fclose(f);
end:
--
2.18.0
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of invalid path of log file
Currently the monitor can be started even with an invalid path of log file.
This patch adds a detection of invalid path of log file when starting monitor.
If the path of log file is invalid, the monitor will refuse to be started.
Signed-off-by: QI Fuli
---
ndctl/monitor.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index f10384b..bf1f1d3 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -603,6 +603,7 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
struct util_filter_ctx fctx = { 0 };
struct monitor_filter_arg mfa = { 0 };
int i, rc;
+ FILE *f;
argc = parse_options_prefix(argc, argv, prefix, options, u, 0);
for (i = 0; i < argc; i++) {
@@ -630,8 +631,16 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
else if (strncmp(monitor.log, "./standard", 10) == 0)
; /*default, already set */
- else
+ else {
+ f = fopen(monitor.log, "a+");
+ if (!f) {
+ error("open %s failed\n", monitor.log);
+ rc = -errno;
+ goto out;
+ }
+ fclose(f);
ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file);
+ }
}
if (monitor.daemon) {
--
2.18.0
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[ndctl PATCH 0/4] Some fixups and improvements ndctl monitor
Fix a couple of issues about log collection, and add [Install] section to systemd unit file of ndclt-monitor. Signed-off-by: QI Fuli --- QI Fuli (4): ndctl, monitor: fix the lack of detection of invalid path of log file ndctl, monitor: set default log destination to syslog if "--daemon" is specified ndctl, monitor: add timestamp and pid to log messages in log_file() ndctl, monitor: add [Install] Section to systemd unit file of ndctl-monitor Documentation/ndctl/ndctl-monitor.txt | 16 +++- ndctl/monitor.c | 27 --- ndctl/monitor.conf| 2 ++ ndctl/ndctl-monitor.service | 3 +++ 4 files changed, 44 insertions(+), 4 deletions(-) -- 2.18.0 ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH V2 3/4] mm: add a function to differentiate the pages is from DAX device memory
On Wed 11-07-18 01:03:51, Zhang Yi wrote:
> DAX driver hotplug the device memory and move it to memory zone, these
> pages will be marked reserved flag, however, some other kernel componet
> will misconceive these pages are reserved mmio (ex: we map these dev_dax
> or fs_dax pages to kvm for DIMM/NVDIMM backend). Together with the type
> MEMORY_DEVICE_FS_DAX, we can use is_dax_page() to differentiate the pages
> is DAX device memory or not.
>
> Signed-off-by: Zhang Yi
> Signed-off-by: Zhang Yu
The patch looks OK to me but I don't really feel too confident about this
part of the kernel... But feel free to add my:
Acked-by: Jan Kara
Honza
> ---
> include/linux/mm.h | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6e19265..9f0f690 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -856,6 +856,13 @@ static inline bool is_device_public_page(const struct
> page *page)
> page->pgmap->type == MEMORY_DEVICE_PUBLIC;
> }
>
> +static inline bool is_dax_page(const struct page *page)
> +{
> + return is_zone_device_page(page) &&
> + (page->pgmap->type == MEMORY_DEVICE_FS_DAX ||
> + page->pgmap->type == MEMORY_DEVICE_DEV_DAX);
> +}
> +
> #else /* CONFIG_DEV_PAGEMAP_OPS */
> static inline void dev_pagemap_get_ops(void)
> {
> @@ -879,6 +886,11 @@ static inline bool is_device_public_page(const struct
> page *page)
> {
> return false;
> }
> +
> +static inline bool is_dax_page(const struct page *page)
> +{
> + return false;
> +}
> #endif /* CONFIG_DEV_PAGEMAP_OPS */
>
> static inline void get_page(struct page *page)
> --
> 2.7.4
>
--
Jan Kara
SUSE Labs, CR
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH V2 2/4] mm: introduce memory type MEMORY_DEVICE_DEV_DAX
On Wed 11-07-18 01:01:59, Zhang Yi wrote:
> Currently, NVDIMM pages will be marked 'PageReserved'. However, unlike
> other reserved PFNs, pages on NVDIMM shall still behave like normal ones
> in many cases, i.e. when used as backend memory of KVM guest. This patch
> introduces a new memory type, MEMORY_DEVICE_DEV_DAX. And set this flag
> while dax driver hotplug the device memory.
>
> Signed-off-by: Zhang Yi
> Signed-off-by: Zhang Yu
> ---
> drivers/dax/pmem.c | 1 +
> include/linux/memremap.h | 9 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
> index fd49b24..fb3f363 100644
> --- a/drivers/dax/pmem.c
> +++ b/drivers/dax/pmem.c
> @@ -111,6 +111,7 @@ static int dax_pmem_probe(struct device *dev)
> return rc;
>
> dax_pmem->pgmap.ref = _pmem->ref;
> + dax_pmem->pgmap.type = MEMORY_DEVICE_DEV_DAX;
> addr = devm_memremap_pages(dev, _pmem->pgmap);
> if (IS_ERR(addr))
> return PTR_ERR(addr);
> diff --git a/include/linux/memremap.h b/include/linux/memremap.h
> index 5ebfff6..a36bce8 100644
> --- a/include/linux/memremap.h
> +++ b/include/linux/memremap.h
> @@ -53,11 +53,20 @@ struct vmem_altmap {
> * wakeup event whenever a page is unpinned and becomes idle. This
> * wakeup is used to coordinate physical address space management (ex:
> * fs truncate/hole punch) vs pinned pages (ex: device dma).
> + *
> + * MEMORY_DEVICE_DEV_DAX:
> + * DAX driver hotplug the device memory and move it to memory zone, these
> + * pages will be marked reserved flag. However, some other kernel componet
> + * will misconceive these pages are reserved mmio (ex: we map these dev_dax
> + * or fs_dax pages to kvm for DIMM/NVDIMM backend). Together with the type
> + * MEMORY_DEVICE_FS_DAX, we can differentiate the pages on NVDIMM with the
> + * normal reserved pages.
So I believe the description should be in terms of what kind of memory is
the MEMORY_DEVICE_DEV_DAX type, not how users use this type. See comments
for other memory types...
Honza
> */
> enum memory_type {
> MEMORY_DEVICE_PRIVATE = 1,
> MEMORY_DEVICE_PUBLIC,
> MEMORY_DEVICE_FS_DAX,
> + MEMORY_DEVICE_DEV_DAX,
> };
>
> /*
> --
> 2.7.4
>
--
Jan Kara
SUSE Labs, CR
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v4 0/2] ext4: fix DAX dma vs truncate/hole-punch
On Fri 27-07-18 10:28:51, Ross Zwisler wrote:
> + fsdevel and the xfs list.
>
> On Wed, Jul 25, 2018 at 4:28 PM Ross Zwisler
> wrote:
> > On Wed, Jul 11, 2018 at 10:17:41AM +0200, Jan Kara wrote:
> > > On Tue 10-07-18 13:10:29, Ross Zwisler wrote:
> > > > Changes since v3:
> > > > * Added an ext4_break_layouts() call to ext4_insert_range() to ensure
> > > >that the {ext4,xfs}_break_layouts() calls have the same meaning.
> > > >(Dave, Darrick and Jan)
> > >
> > > How about the occasional WARN_ON_ONCE you mention below. Were you able to
> > > hunt them down?
> >
> > The root cause of this issue is that while the ei->i_mmap_sem provides
> > synchronization between ext4_break_layouts() and page faults, it doesn't
> > provide synchronize us with the direct I/O path. This exact same issue
> > exists
> > in XFS AFAICT, with the synchronization tool there being the XFS_MMAPLOCK.
> >
> > This allows the direct I/O path to do I/O and raise & lower page->_refcount
> > while we're executing a truncate/hole punch. This leads to us trying to
> > free
> > a page with an elevated refcount.
> >
> > Here's one instance of the race:
> >
> > CPU 0 CPU 1
> > - -
> > ext4_punch_hole()
> > ext4_break_layouts() # all pages have refcount=1
> >
> > ext4_direct_IO()
> > ... lots of layers ...
> > follow_page_pte()
> > get_page() # elevates refcount
> >
> > truncate_pagecache_range()
> >... a few layers ...
> >dax_disassociate_entry() # sees elevated refcount, WARN_ON_ONCE()
> >
So this is a very different race from the one below. And it should be
impossible to happen. This race is exactly the reason why
dax_layout_busy_page() has unmap_mapping_range() call to force GUP to fault
which blocks on ei->i_mmap_sem / XFS_MMAPLOCK and thus avoids the race.
> > A similar race occurs when the refcount is being dropped while we're running
> > ext4_break_layouts(), and this is the one that my test was actually hitting:
> >
> > CPU 0 CPU 1
> > - -
> > ext4_direct_IO()
> > ... lots of layers ...
> > follow_page_pte()
> > get_page()
> > # elevates refcount of page X
> > ext4_punch_hole()
> > ext4_break_layouts() # two pages, X & Y, have refcount == 2
> > __wait_var_event() # called for page X
> >
> > __put_devmap_managed_page()
> > # drops refcount of X to 1
> >
> ># __wait_var_events() checks X's refcount in "if (condition)", and
> > breaks.
> ># We never actually called ext4_wait_dax_page(), so 'retry' in
> ># ext4_break_layouts() is still false. Exit do/while loop in
> ># ext4_break_layouts, never attempting to wait on page Y which still has
> > an
> ># elevated refcount of 2.
> >
> > truncate_pagecache_range()
> >... a few layers ...
> >dax_disassociate_entry() # sees elevated refcount for Y, WARN_ON_ONCE()
> >
> > This second race can be fixed with the patch at the end of this function,
> > which I think should go in, unless there is a benfit to the current retry
> > scheme which relies on the 'retry' variable in {ext4,xfs}_break_layouts()?
> > With this patch applied I've been able to run my unit test through
> > thousands of iterations, where it used to failed consistently within 10 or
> > so.
> >
> > Even so, I wonder if the real solution is to add synchronization between
> > the direct I/O path and {ext4,xfs}_break_layouts()? Other ideas on how
> > this should be handled?
> >
> > --- >8 ---
> >
> > From a4519b0f40362f0a63ae96acaf986092aff0f0d3 Mon Sep 17 00:00:00 2001
> > From: Ross Zwisler
> > Date: Wed, 25 Jul 2018 16:16:05 -0600
> > Subject: [PATCH] ext4: Close race between direct IO and ext4_break_layouts()
> >
> > If the refcount of a page is lowered between the time that it is returned
> > by dax_busy_page() and when the refcount is again checked in
> > ext4_break_layouts() => ___wait_var_event(), the waiting function
> > ext4_wait_dax_page() will never be called. This means that
> > ext4_break_layouts() will still have 'retry' set to false, so we'll stop
> > looping and never check the refcount of other pages in this inode.
> >
> > Instead, always continue looping as long as dax_layout_busy_page() gives us
> > a page which it found with an elevated refcount.
> >
> > Note that this works around the race exposed by my unit test, but I think
> > that there is another race that needs to be addressed, probably with
> > additional synchronization added between
[PATCH] ACPI: nfit: adjust annotation for why return 0 if fail to find NFIT at startup
From: Ocean He
Add detail explanation why it's ok to return 0 if fail to find NFIT at
startup, refer to chapter 9.20.2 NVDIMM Root Device in ACPI 6.2 spec.
Signed-off-by: Ocean He
---
v1: https://lists.01.org/pipermail/linux-nvdimm/2018-August/017311.html
v2: Per Vishal's comments of v1, it turns out that it's right to keep
"return 0" if NFIT not found at boot time. This v2 adjust annotation
to avoid misunderstanding of "return 0".
drivers/acpi/nfit/core.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 7c47900..1b982fe 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3353,7 +3353,13 @@ static int acpi_nfit_add(struct acpi_device *adev)
status = acpi_get_table(ACPI_SIG_NFIT, 0, );
if (ACPI_FAILURE(status)) {
- /* This is ok, we could have an nvdimm hotplugged later */
+ /* The NVDIMM root device allows OS to trigger enumeration of
+* NVDIMMs through NFIT at boot time and re-enumeration at
+* root level via the _FIT method during runtime.
+* This is ok to return 0 here, we could have an nvdimm
+* hotplugged later and evaluate _FIT method which returns
+* data in the format of a series of NFIT Structures.
+*/
dev_dbg(dev, "failed to find NFIT at startup\n");
return 0;
}
--
1.8.3.1
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
