Re: [PATCH v6 2/4] resource: Use list_head to link sibling resource
Hi,
On 07/05/18 at 01:00am, kbuild test robot wrote:
> Hi Baoquan,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.18-rc3 next-20180704]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
Thanks for telling.
I cloned 0day-ci/linut to my local pc.
https://github.com/0day-ci/linux.git
However, I didn't find below branch. And tried to open it in web
broswer, also failed.
> url:
> https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180704-121402
> config: mips-rb532_defconfig (attached as .config)
> compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.2.0 make.cross ARCH=mips
I did find a old one which is for the old version 5 post.
[bhe@linux]$ git remote -v
0day-ci https://github.com/0day-ci/linux.git (fetch)
0day-ci https://github.com/0day-ci/linux.git (push)
[bhe@dhcp-128-28 linux]$ git branch -a| grep Baoquan| grep resource
remotes/0day-ci/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180612-113600
Could you help have a look at this?
Thanks
Baoquan
>
> All error/warnings (new ones prefixed by >>):
>
> >> arch/mips/pci/pci-rc32434.c:57:11: error: initialization from incompatible
> >> pointer type [-Werror=incompatible-pointer-types]
> .child = _res_pci_mem2
> ^
>arch/mips/pci/pci-rc32434.c:57:11: note: (near initialization for
> 'rc32434_res_pci_mem1.child.next')
> >> arch/mips/pci/pci-rc32434.c:51:47: warning: missing braces around
> >> initializer [-Wmissing-braces]
> static struct resource rc32434_res_pci_mem1 = {
> ^
>arch/mips/pci/pci-rc32434.c:60:47: warning: missing braces around
> initializer [-Wmissing-braces]
> static struct resource rc32434_res_pci_mem2 = {
> ^
>cc1: some warnings being treated as errors
>
> vim +57 arch/mips/pci/pci-rc32434.c
>
> 73b4390f Ralf Baechle 2008-07-16 50
> 73b4390f Ralf Baechle 2008-07-16 @51 static struct resource
> rc32434_res_pci_mem1 = {
> 73b4390f Ralf Baechle 2008-07-16 52 .name = "PCI MEM1",
> 73b4390f Ralf Baechle 2008-07-16 53 .start = 0x5000,
> 73b4390f Ralf Baechle 2008-07-16 54 .end = 0x5FFF,
> 73b4390f Ralf Baechle 2008-07-16 55 .flags = IORESOURCE_MEM,
> 73b4390f Ralf Baechle 2008-07-16 56 .sibling = NULL,
> 73b4390f Ralf Baechle 2008-07-16 @57 .child = _res_pci_mem2
> 73b4390f Ralf Baechle 2008-07-16 58 };
> 73b4390f Ralf Baechle 2008-07-16 59
>
> :: The code at line 57 was first introduced by commit
> :: 73b4390fb23456964201abda79f1210fe337d01a [MIPS] Routerboard 532:
> Support for base system
>
> :: TO: Ralf Baechle
> :: CC: Ralf Baechle
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v9 1/3] ndctl, monitor: add ndctl monitor
On Thu, Jun 28, 2018 at 7:30 PM, QI Fuli wrote: > Ndctl monitor is used for monitoring the smart events of nvdimm DIMMs. > When a smart event fires, monitor will output the notifications which > include dimm health status and evnet informations to syslog or a > logfile by setting [--logfile] option. The notifications follow json > format and can be consumed by log collectors like Fluentd. > > The objects to monitor can be selected by setting [--dimm] [--region] > [--namespace] [--bus] options and the event type can be filtered by > setting [--dimm-event] option. These options support multiple > space-separated arguments. > > Ndctl monitor can be forked as a daemon by using [--daemon] option, > such as: ># ndctl monitor --daemon --logfile /var/log/ndctl/monitor.log > > Signed-off-by: QI Fuli > --- > builtin.h | 1 + > ndctl/Makefile.am | 3 +- > ndctl/lib/libndctl.sym | 1 + > ndctl/lib/smart.c | 17 ++ > ndctl/libndctl.h | 6 + > ndctl/monitor.c| 531 + > ndctl/ndctl.c | 1 + > util/filter.h | 9 + > 8 files changed, 568 insertions(+), 1 deletion(-) > create mode 100644 ndctl/monitor.c > > diff --git a/builtin.h b/builtin.h > index d3cc723..675a6ce 100644 > --- a/builtin.h > +++ b/builtin.h > @@ -39,6 +39,7 @@ int cmd_inject_error(int argc, const char **argv, void > *ctx); > int cmd_wait_scrub(int argc, const char **argv, void *ctx); > int cmd_start_scrub(int argc, const char **argv, void *ctx); > int cmd_list(int argc, const char **argv, void *ctx); > +int cmd_monitor(int argc, const char **argv, void *ctx); > #ifdef ENABLE_TEST > int cmd_test(int argc, const char **argv, void *ctx); > #endif > diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am > index d22a379..7dbf223 100644 > --- a/ndctl/Makefile.am > +++ b/ndctl/Makefile.am > @@ -16,7 +16,8 @@ ndctl_SOURCES = ndctl.c \ > util/json-smart.c \ > util/json-firmware.c \ > inject-error.c \ > - inject-smart.c > + inject-smart.c \ > + monitor.c > > if ENABLE_DESTRUCTIVE > ndctl_SOURCES += ../test/blk_namespaces.c \ > diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym > index e939993..f64df56 100644 > --- a/ndctl/lib/libndctl.sym > +++ b/ndctl/lib/libndctl.sym > @@ -366,4 +366,5 @@ global: > ndctl_namespace_inject_error2; > ndctl_namespace_uninject_error2; > ndctl_cmd_ars_stat_get_flag_overflow; > + ndctl_cmd_smart_get_event_flags; > } LIBNDCTL_15; > diff --git a/ndctl/lib/smart.c b/ndctl/lib/smart.c > index 0455252..90a65d0 100644 > --- a/ndctl/lib/smart.c > +++ b/ndctl/lib/smart.c > @@ -101,6 +101,23 @@ NDCTL_EXPORT unsigned int > ndctl_cmd_smart_threshold_get_temperature( > > smart_cmd_op(smart_threshold_get_supported_alarms, unsigned int, 0); > > +NDCTL_EXPORT unsigned int ndctl_cmd_smart_get_event_flags(struct ndctl_cmd > *cmd) My expectation for this ndctl_*_get_event_flags() api was to have it be: ndctl_dimm_get_event_flags() ...and with that in place get rid of the 'struct monitor_dimm' object. Push everything to be retrieved through api calls against a 'struct ndctl_dimm' object. In other words, the usage of 'struct ndctl_cmd' should be hidden and all monitor operations should be done in terms of 'struct ndctl_dimm' helper calls. This allows for other objects like regions and namespace to grow an event flags api in the future: ndctl_namespace_get_event_flags() ndctl_region_get_event_flags() ndctl_bus_get_event_flags() ...where those objects don't have any relationship with a 'struct ndctl_cmd', so neither should dimm event monitoring, at least not for the library api that ndctl-monitor is using. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH v3] ndctl, test: add a new unit test for monitor
On Fri, Jul 6, 2018 at 10:35 PM, QI Fuli wrote:
> Add a new unit test to test the following options of the monitor command.
>--dimm
>--bus
>--region
>--namespace
>--logfile
>--config-file
>
> Based-on-patch-by: Yasunori Goto
> Signed-off-by: QI Fuli
> ---
> v2 -> v3:
> - Add filter_obj instead of hard-coded values
> v1 -> v2:
> - Add init()
> - Add get_filter_dimm() to get the filter dimms by ndctl list command
>instead of hard-cording
> - Add sleep to call_notify()
>
> test/Makefile.am | 3 +-
> test/monitor.sh | 134 +++
> 2 files changed, 136 insertions(+), 1 deletion(-)
> create mode 100755 test/monitor.sh
This test is still failing for me, here is the tail of the
test/test-suite.log output:
+ sync
+ sleep 3
+ check_result
++ cat /tmp/tmp.dT3TJ4l5IE
+ jlog='nmem0: no smart support
no dimms to monitor'
++ jq .dimm.dev
++ sort
++ uniq
++ sed -e ':loop; N; $!b loop; s/\n/:/g'
++ sed 's/\"//g'
parse error: Invalid literal at line 1, column 6
+ notify_dimms=
+ [[ '' == '' ]]
+ stop_monitor
+ kill 39414
./monitor.sh: line 48: kill: (39414) - No such process
++ err 48
+++ basename ./monitor.sh
++ echo test/monitor.sh: failed at line 48
test/monitor.sh: failed at line 48
++ '[' -n '' ']'
++ exit 1
FAIL monitor.sh (exit status: 1)
I notice errors around get_filter_dimm(). Why is that function needed,
it seems redundant now that $filter_obj is being used?
Hmm, if I just delete get_filter_dimm() I get this;
++ jq .dimm.dev
++ sort
++ uniq
++ sed -e ':loop; N; $!b loop; s/\n/:/g'
++ sed 's/\"//g'
+ notify_dimms=nmem3
+ [[ '' == nmem3 ]]
++ err 34
+++ basename ./monitor.sh
++ echo test/monitor.sh: failed at line 34
test/monitor.sh: failed at line 34
++ '[' -n '' ']'
++ exit 1
FAIL monitor.sh (exit status: 1)
I assume this test is passing for you? Can you try running it inside a
virtual machine that has a virtual NVDIMM so you can debug the
collisions between the nfit_test.0 bus objects and the ACPI.NFIT ones?
Here is my test environment where this unit test is failing:
https://gist.github.com/djbw/144dc5ddaf5e935ad58bd66a702a5ea8
>
> diff --git a/test/Makefile.am b/test/Makefile.am
> index cd451e9..8c76462 100644
> --- a/test/Makefile.am
> +++ b/test/Makefile.am
> @@ -21,7 +21,8 @@ TESTS =\
> btt-pad-compat.sh \
> firmware-update.sh \
> ack-shutdown-count-set \
> - rescan-partitions.sh
> + rescan-partitions.sh \
> + monitor.sh
>
> check_PROGRAMS =\
> libndctl \
> diff --git a/test/monitor.sh b/test/monitor.sh
> new file mode 100755
> index 000..4a7d733
> --- /dev/null
> +++ b/test/monitor.sh
> @@ -0,0 +1,134 @@
> +#!/bin/bash -Ex
> +
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright(c) 2018, FUJITSU LIMITED. All rights reserved.
> +
> +rc=77
> +logfile=""
> +conf_file=""
> +filter_dimms=""
> +filter_obj=""
> +monitor_pid=65536
> +
> +. ./common
> +
> +trap 'err $LINENO' ERR
> +
> +check_min_kver "4.15" || do_skip "kernel $KVER may not support monitor
> service"
> +
> +start_monitor()
> +{
> + logfile=$(mktemp)
> + $NDCTL monitor -l $logfile $1 &
> + monitor_pid=$!
> + truncate --size 0 $logfile #remove startup log
> +}
> +
> +get_filter_dimm()
> +{
> + jlist=$($NDCTL list -D -b $NFIT_TEST_BUS0 $1)
> + filter_dimms=$(jq '.[]."dev"?, ."dev"?' <<<$jlist | sort | uniq | sed
> -e ':loop; N; $!b loop; s/\n/:/g' | sed 's/\"//g')
> +}
Can we just remove this?
> +
> +call_notify()
> +{
> + ./smart-notify $NFIT_TEST_BUS0
> + sync; sleep 3
> +}
> +
> +check_result()
> +{
> + jlog=$(cat $logfile)
> + notify_dimms=$(jq ."dimm"."dev" <<<$jlog | sort | uniq | sed -e
> ':loop; N; $!b loop; s/\n/:/g' | sed 's/\"//g')
Can you add a comment here about that sed script is trying to do? If
it's just filtering json I'd rather it just jq directly.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH v2] ndctl, test: add a new unit test for monitor
On Fri, 2018-07-06 at 20:43 -0700, Dan Williams wrote: > On Fri, Jul 6, 2018 at 6:48 PM, QI Fuli wrote: > > Hi Masa, > > > > Thank you for your comments. > > > > On 7/7/2018 12:08 AM, Masayoshi Mizuma wrote: > > > > > > Hi Qi, > > > > > > On 07/06/2018 01:22 AM, QI Fuli wrote: > > > > > > > > Add a new unit test to test the following options of the monitor > > > > command. > > > > --dimm > > > > --bus > > > > --region > > > > --namespace > > > > --logfile > > > > --config-file > > > > > > > > Based-on-patch-by: Yasunori Goto > > > > Acked-by: Masayoshi Mizuma > > > > > > I think this Acked-by is wrong because I'm not a maintainer :-) > > > > I'm sorry for this mistake. > > I'm perfectly fine to take reviewed-by's and acked-by's from > non-maintainers. In fact I'd prefer to have more. I'm sure Vishal > feels the same. Yes, absolutely! > ___ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v2] mm/sparse.c: fix error path in sparse_add_one_section
On Fri, Jul 06, 2018 at 04:33:58PM -0600, Ross Zwisler wrote:
> The following commit in -next:
>
> commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
> remove check")
>
> changed how the error handling in sparse_add_one_section() works.
>
> Previously sparse_index_init() could return -EEXIST, and the function would
> continue on happily. 'ret' would get unconditionally overwritten by the
> result from sparse_init_one_section() and the error code after the 'out:'
> label wouldn't be triggered.
>
> With the above referenced commit, though, an -EEXIST error return from
> sparse_index_init() now takes us through the function and into the error
> case after 'out:'. This eventually causes a kernel BUG, probably because
> we've just freed a memory section that we successfully set up and marked as
> present:
>
> BUG: unable to handle kernel paging request at ea000580
> RIP: 0010:memmap_init_zone+0x154/0x1cf
>
> Call Trace:
>move_pfn_range_to_zone+0x168/0x180
>devm_memremap_pages+0x29b/0x480
>pmem_attach_disk+0x1ae/0x6c0 [nd_pmem]
>? devm_memremap+0x79/0xb0
>nd_pmem_probe+0x7e/0xa0 [nd_pmem]
>nvdimm_bus_probe+0x6e/0x160 [libnvdimm]
>driver_probe_device+0x310/0x480
>__device_attach_driver+0x86/0x100
>? __driver_attach+0x110/0x110
>bus_for_each_drv+0x6e/0xb0
>__device_attach+0xe2/0x160
>device_initial_probe+0x13/0x20
>bus_probe_device+0xa6/0xc0
>device_add+0x41b/0x660
>? lock_acquire+0xa3/0x210
>nd_async_device_register+0x12/0x40 [libnvdimm]
>async_run_entry_fn+0x3e/0x170
>process_one_work+0x230/0x680
>worker_thread+0x3f/0x3b0
>kthread+0x12f/0x150
>? process_one_work+0x680/0x680
>? kthread_create_worker_on_cpu+0x70/0x70
>ret_from_fork+0x3a/0x50
>
> Fix this by clearing 'ret' back to 0 if sparse_index_init() returns
> -EEXIST. This restores the previous behavior.
>
> Signed-off-by: Ross Zwisler
Reviewed-by: Oscar Salvador
--
Oscar Salvador
SUSE L3
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
