Re: [PATCH v5 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()

2019-06-28 Thread Luis Chamberlain
On Fri, Jun 28, 2019 at 01:01:54AM -0700, Brendan Higgins wrote:
> On Wed, Jun 26, 2019 at 11:10 PM Luis Chamberlain  wrote:
> >
> > On Wed, Jun 26, 2019 at 09:07:43PM -0700, Iurii Zaikin wrote:
> > > On Tue, Jun 25, 2019 at 7:17 PM Luis Chamberlain  
> > > wrote:
> > > > > +static void sysctl_test_dointvec_table_maxlen_unset(struct kunit 
> > > > > *test)
> > > > > +{
> > > > > + struct ctl_table table = {
> > > > > + .procname = "foo",
> > > > > + .data   = _data.int_0001,
> > > > > + .maxlen = 0,
> > > > > + .mode   = 0644,
> > > > > + .proc_handler   = proc_dointvec,
> > > > > + .extra1 = _zero,
> > > > > + .extra2 = _one_hundred,
> > > > > + };
> > > > > + void  *buffer = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > > + size_t len;
> > > > > + loff_t pos;
> > > > > +
> > > > > + len = 1234;
> > > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 0, buffer, , 
> > > > > ));
> > > > > + KUNIT_EXPECT_EQ(test, (size_t)0, len);
> > > > > + len = 1234;
> > > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 1, buffer, , 
> > > > > ));
> > > > > + KUNIT_EXPECT_EQ(test, (size_t)0, len);
> > > > > +}
> > > >
> > > > In a way this is also testing for general kernel API changes. This is 
> > > > and the
> > > > last one were good examples. So this is not just testing functionality
> > > > here. There is no wrong or write answer if 0 or -EINVAL was returned
> > > > other than the fact that we have been doing this for years.
> > > >
> > > > Its a perhaps small but important difference for some of these tests.  I
> > > > *do* think its worth clarifying through documentation which ones are
> > > > testing for API consistency Vs proper correctness.
> > >
> > > You make a good point that the test codifies the existing behavior of
> > > the function in lieu of formal documentation.  However, the test cases
> > > were derived from examining the source code of the function under test
> > > and attempting to cover all branches. The assertions were added only
> > > for the values that appeared to be set deliberately in the
> > > implementation. And it makes sense to me to test that the code does
> > > exactly what the implementation author intended.
> >
> > I'm not arguing against adding them. I'm suggesting that it is different
> > to test for API than for correctness of intended functionality, and
> > it would be wise to make it clear which test cases are for API and which
> > for correctness.
> 
> I see later on that some of the API stuff you are talking about is
> public APIs from the standpoint of user (outside of LInux) visible.

Right, UAPI.

> To
> be clear, is that what you mean by public APIs throughout, or would
> you distinguish between correctness tests, internal API tests, and
> external API tests?

I would definitely recommend distingishing between all of these.
Kernel API (or just call it API), UAPI, and correctness.

> > This will come up later for other kunit tests and it would be great
> > to set precendent so that other kunit tests can follow similar
> > practices to ensure its clear what is API realted Vs correctness of
> > intended functionality.
> >
> > In fact, I'm not yet sure if its possible to test public kernel API to
> > userspace with kunit, but if it is possible... well, that could make
> > linux-api folks happy as they could enable us to codify interpreation of
> > what is expected into kunit test cases, and we'd ensure that the
> > codified interpretation is not only documented in man pages but also
> > through formal kunit test cases.
> >
> > A regression in linux-api then could be formalized through a proper
> > kunit tests case. And if an API evolves, it would force developers to
> > update the respective kunit which codifies that contract.
> 
> Yep, I think that is long term hope. Some of the file system interface
> stuff that requires a filesystem to be mounted somewhere might get a
> little weird/difficult, but I suspect we should be able to do it
> eventually. I mean it's all just C code right? Should mostly boil down
> to someone figuring out how to do it the first time.

There used to be hacks in the kernel the call syscalls in a few places.
This was cleaned up and addressed via Dominik Brodowski's series last
year in March:

http://lkml.kernel.org/r/20180325162527.ga17...@light.dominikbrodowski.net

An example commit: d300b610812f3 ("kernel: use kernel_wait4() instead of
sys_wait4()").

So it would seem the work is done, and you'd just have to use the
respective exposed kernel_syscallname() calls, or add some if you
want to test a specific syscall in kernel space.

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


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 12:02 PM Christoph Hellwig  wrote:
>
> On Fri, Jun 28, 2019 at 11:59:19AM -0700, Dan Williams wrote:
> > It's a bug that the call to put_devmap_managed_page() was gated by
> > MEMORY_DEVICE_PUBLIC in release_pages(). That path is also applicable
> > to MEMORY_DEVICE_FSDAX because it needs to trigger the ->page_free()
> > callback to wake up wait_on_var() via fsdax_pagefree().
> >
> > So I guess you could argue that the MEMORY_DEVICE_PUBLIC removal patch
> > left the original bug in place. In that sense we're no worse off, but
> > since we know about the bug, the fix and the patches have not been
> > applied yet, why not fix it now?
>
> The fix it now would simply be to apply Ira original patch now, but
> given that we are at -rc6 is this really a good time?  And if we don't
> apply it now based on the quilt based -mm worflow it just seems a lot
> easier to apply it after my series.  Unless we want to include it in
> the series, in which case I can do a quick rebase, we'd just need to
> make sure Andrew pulls it from -mm.

I believe -mm auto drops patches when they appear in the -next
baseline. So it should "just work" to pull it into the series and send
it along for -next inclusion.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


[ndctl PATCH v5 06/13] libdaxctl: add an interface to get the mode for a dax device

2019-06-28 Thread Vishal Verma
In preparation for a reconfigure-device command, add an interface to
retrieve the 'mode' of a dax device. This will allow the
reconfigure-device command (and via daxctl_dev_to_json(), also
daxctl-list) to print the mode in device listings via a 'daxctl-list'
command or immediately after a mode change.

Add a 'state' attribute to the json listings for devices, since a device
could end up in a state where it is not bound to any driver, and hence,
'disabled'. The state attribute is only displayed for disabled devices.

Cc: Dave Hansen 
Cc: Dan Williams 
Signed-off-by: Vishal Verma 
---
 daxctl/lib/libdaxctl.c   | 37 +
 daxctl/lib/libdaxctl.sym |  1 +
 daxctl/libdaxctl.h   |  1 +
 util/json.c  | 17 +
 4 files changed, 56 insertions(+)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index b22df50..9ff6235 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -12,6 +12,8 @@
  */
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -880,6 +882,41 @@ DAXCTL_EXPORT int daxctl_dev_disable(struct daxctl_dev 
*dev)
return 0;
 }
 
+DAXCTL_EXPORT enum daxctl_dev_mode daxctl_dev_get_mode(struct daxctl_dev *dev)
+{
+   const char *devname = daxctl_dev_get_devname(dev);
+   struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+   int rc = DAXCTL_DEV_MODE_UNKNOWN;
+   char path[200];
+   const int len = sizeof(path);
+   char *mod_path, *mod_base;
+
+   if (!daxctl_dev_is_enabled(dev))
+   return rc;
+
+   if (snprintf(path, len, "%s/driver/module", dev->dev_path) >= len) {
+   err(ctx, "%s: buffer too small!\n", devname);
+   return -ENXIO;
+   }
+
+   mod_path = realpath(path, NULL);
+   if (!mod_path) {
+   rc = -errno;
+   err(ctx, "%s:  unable to determine module: %s\n", devname,
+   strerror(errno));
+   return rc;
+   }
+
+   mod_base = basename(mod_path);
+   if (strcmp(mod_base, dax_modules[DAXCTL_DEV_MODE_RAM]) == 0)
+   rc = DAXCTL_DEV_MODE_RAM;
+   else if (strcmp(mod_base, dax_modules[DAXCTL_DEV_MODE_DEVDAX]) == 0)
+   rc = DAXCTL_DEV_MODE_DEVDAX;
+
+   free(mod_path);
+   return rc;
+}
+
 DAXCTL_EXPORT struct daxctl_ctx *daxctl_dev_get_ctx(struct daxctl_dev *dev)
 {
return dev->region->ctx;
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index 53eb700..9ce5096 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -67,4 +67,5 @@ global:
daxctl_memory_set_online;
daxctl_memory_set_offline;
daxctl_memory_is_online;
+   daxctl_dev_get_mode;
 } LIBDAXCTL_5;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index a5a2bab..6d1c17d 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -74,6 +74,7 @@ int daxctl_dev_disable(struct daxctl_dev *dev);
 int daxctl_dev_enable_devdax(struct daxctl_dev *dev);
 int daxctl_dev_enable_ram(struct daxctl_dev *dev);
 int daxctl_dev_get_target_node(struct daxctl_dev *dev);
+enum daxctl_dev_mode daxctl_dev_get_mode(struct daxctl_dev *dev);
 
 struct daxctl_memory;
 struct daxctl_memory *daxctl_dev_get_memory(struct daxctl_dev *dev);
diff --git a/util/json.c b/util/json.c
index f521337..1673f1d 100644
--- a/util/json.c
+++ b/util/json.c
@@ -271,6 +271,7 @@ struct json_object *util_daxctl_dev_to_json(struct 
daxctl_dev *dev,
 {
const char *devname = daxctl_dev_get_devname(dev);
struct json_object *jdev, *jobj;
+   enum daxctl_dev_mode mode;
int node;
 
jdev = json_object_new_object();
@@ -292,6 +293,22 @@ struct json_object *util_daxctl_dev_to_json(struct 
daxctl_dev *dev,
json_object_object_add(jdev, "target_node", jobj);
}
 
+   mode = daxctl_dev_get_mode(dev);
+   if (mode >= 0) {
+   if (mode == DAXCTL_DEV_MODE_RAM)
+   jobj = json_object_new_string("system-ram");
+   else
+   jobj = json_object_new_string("devdax");
+   if (jobj)
+   json_object_object_add(jdev, "mode", jobj);
+   }
+
+   if (!daxctl_dev_is_enabled(dev)) {
+   jobj = json_object_new_string("disabled");
+   if (jobj)
+   json_object_object_add(jdev, "state", jobj);
+   }
+
return jdev;
 }
 
-- 
2.20.1

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


[ndctl PATCH v5 13/13] test: Add a unit test for daxctl-reconfigure-device and friends

2019-06-28 Thread Vishal Verma
Add a new unit test to test dax device reconfiguration and memory
operations. This teaches test/common about daxctl, and adds an ACPI.NFIT
bus variable. Since we have to operate on the ACPI.NFIT bus, the test is
marked as destructive.

Cc: Dan Williams 
Signed-off-by: Vishal Verma 
---
 test/Makefile.am   |  3 +-
 test/common| 19 --
 test/daxctl-devices.sh | 81 ++
 3 files changed, 99 insertions(+), 4 deletions(-)
 create mode 100755 test/daxctl-devices.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index 874c4bb..84474d0 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -49,7 +49,8 @@ TESTS +=\
dax.sh \
device-dax \
device-dax-fio.sh \
-   mmap.sh
+   mmap.sh \
+   daxctl-devices.sh
 
 if ENABLE_KEYUTILS
 TESTS += security.sh
diff --git a/test/common b/test/common
index 1b9d3da..1814a0c 100644
--- a/test/common
+++ b/test/common
@@ -15,12 +15,25 @@ else
exit 1
 fi
 
-# NFIT_TEST_BUS[01]
+# DAXCTL
 #
-NFIT_TEST_BUS0=nfit_test.0
-NFIT_TEST_BUS1=nfit_test.1
+if [ -f "../daxctl/daxctl" ] && [ -x "../daxctl/daxctl" ]; then
+   export DAXCTL=../daxctl/daxctl
+elif [ -f "./daxctl/daxctl" ] && [ -x "./daxctl/daxctl" ]; then
+   export DAXCTL=./daxctl/daxctl
+else
+   echo "Couldn't find an daxctl binary"
+   exit 1
+fi
 
 
+# NFIT_TEST_BUS[01]
+#
+NFIT_TEST_BUS0="nfit_test.0"
+NFIT_TEST_BUS1="nfit_test.1"
+ACPI_BUS="ACPI.NFIT"
+E820_BUS="e820"
+
 # Functions
 
 # err
diff --git a/test/daxctl-devices.sh b/test/daxctl-devices.sh
new file mode 100755
index 000..cfd9362
--- /dev/null
+++ b/test/daxctl-devices.sh
@@ -0,0 +1,81 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2019 Intel Corporation. All rights reserved.
+
+rc=77
+. ./common
+
+trap 'cleanup $LINENO' ERR
+
+cleanup()
+{
+   printf "Error at line %d\n" "$1"
+   [[ $testdev ]] && reset_dev
+   exit $rc
+}
+
+find_testdev()
+{
+   local rc=77
+
+   # find a victim device
+   testbus="$ACPI_BUS"
+   testdev=$("$NDCTL" list -b "$testbus" -Ni | jq -er '.[0].dev | .//""')
+   if [[ ! $testdev  ]]; then
+   printf "Unable to find a victim device\n"
+   exit "$rc"
+   fi
+   printf "Found victim dev: %s on bus: %s\n" "$testdev" "$testbus"
+}
+
+setup_dev()
+{
+   test -n "$testbus"
+   test -n "$testdev"
+
+   "$NDCTL" destroy-namespace -f -b "$testbus" "$testdev"
+   testdev=$("$NDCTL" create-namespace -b "$testbus" -m devdax -fe 
"$testdev" -s 256M | \
+   jq -er '.dev')
+   test -n "$testdev"
+}
+
+reset_dev()
+{
+   "$NDCTL" destroy-namespace -f -b "$testbus" "$testdev"
+}
+
+daxctl_get_dev()
+{
+   "$NDCTL" list -n "$1" -X | jq -er '.[].daxregion.devices[0].chardev'
+}
+
+daxctl_get_mode()
+{
+   "$DAXCTL" list -d "$1" | jq -er '.[].mode'
+}
+
+daxctl_test()
+{
+   local daxdev
+
+   daxdev=$(daxctl_get_dev "$testdev")
+   test -n "$daxdev"
+
+   "$DAXCTL" reconfigure-device -N -m system-ram "$daxdev"
+   [[ $(daxctl_get_mode "$daxdev") == "system-ram" ]]
+   "$DAXCTL" online-memory "$daxdev"
+   "$DAXCTL" offline-memory "$daxdev"
+   "$DAXCTL" reconfigure-device -m devdax "$daxdev"
+   [[ $(daxctl_get_mode "$daxdev") == "devdax" ]]
+   "$DAXCTL" reconfigure-device -m system-ram "$daxdev"
+   [[ $(daxctl_get_mode "$daxdev") == "system-ram" ]]
+   "$DAXCTL" reconfigure-device -O -m devdax "$daxdev"
+   [[ $(daxctl_get_mode "$daxdev") == "devdax" ]]
+}
+
+find_testdev
+setup_dev
+rc=1
+daxctl_test
+reset_dev
+exit 0
-- 
2.20.1

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


[ndctl PATCH v5 08/13] Documentation/daxctl: add a man page for daxctl-reconfigure-device

2019-06-28 Thread Vishal Verma
Add a man page describing the new daxctl-reconfigure-device command.

Cc: Pavel Tatashin 
Cc: Dave Hansen 
Cc: Dan Williams 
Signed-off-by: Vishal Verma 
---
 Documentation/daxctl/Makefile.am  |   3 +-
 .../daxctl/daxctl-reconfigure-device.txt  | 139 ++
 2 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/daxctl/daxctl-reconfigure-device.txt

diff --git a/Documentation/daxctl/Makefile.am b/Documentation/daxctl/Makefile.am
index 6aba035..715fbad 100644
--- a/Documentation/daxctl/Makefile.am
+++ b/Documentation/daxctl/Makefile.am
@@ -28,7 +28,8 @@ endif
 man1_MANS = \
daxctl.1 \
daxctl-list.1 \
-   daxctl-migrate-device-model.1
+   daxctl-migrate-device-model.1 \
+   daxctl-reconfigure-device.1
 
 CLEANFILES = $(man1_MANS)
 
diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt 
b/Documentation/daxctl/daxctl-reconfigure-device.txt
new file mode 100644
index 000..fb2b36b
--- /dev/null
+++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+
+daxctl-reconfigure-device(1)
+
+
+NAME
+
+daxctl-reconfigure-device - Reconfigure a dax device into a different mode
+
+SYNOPSIS
+
+[verse]
+'daxctl reconfigure-device'  [...] []
+
+EXAMPLES
+
+
+* Reconfigure dax0.0 to system-ram mode, don't online the memory
+
+# daxctl reconfigure-device --mode=system-ram --no-online dax0.0
+[
+  {
+"chardev":"dax0.0",
+"size":16777216000,
+"target_node":2,
+"mode":"system-ram"
+  }
+]
+
+
+* Reconfigure dax0.0 to devdax mode, attempt to offline the memory
+
+# daxctl reconfigure-device --human --mode=devdax --attempt-offline dax0.0
+{
+  "chardev":"dax0.0",
+  "size":"15.63 GiB (16.78 GB)",
+  "target_node":2,
+  "mode":"devdax"
+}
+
+
+* Reconfigure all dax devices on region0 to system-ram mode
+
+# daxctl reconfigure-device --mode=system-ram --region=0 all
+[
+  {
+"chardev":"dax0.0",
+"size":16777216000,
+"target_node":2,
+"mode":"system-ram"
+  },
+  {
+"chardev":"dax0.1",
+"size":16777216000,
+"target_node":3,
+"mode":"system-ram"
+  }
+]
+
+
+* Run a process called 'some-service' using numactl to restrict its cpu
+nodes to '0' and '1', and  memory allocations to node 2 (determined using
+daxctl_dev_get_target_node() or 'daxctl list')
+
+# daxctl reconfigure-device --mode=system-ram --no-online dax0.0
+[
+  {
+"chardev":"dax0.0",
+"size":16777216000,
+"target_node":2,
+"mode":"system-ram"
+  }
+]
+
+# numactl --cpunodebind=0-1 --membind=2 -- some-service --opt1 --opt2
+
+
+DESCRIPTION
+---
+
+Reconfigure the operational mode of a dax device. This can be used to convert
+a regular 'devdax' mode device to the 'system-ram' mode which allows for the 
dax
+range to be hot-plugged into the system as regular memory.
+
+NOTE: This is a destructive operation. Any data on the dax device *will* be
+lost.
+
+NOTE: Device reconfiguration depends on the dax-bus device model. If dax-class 
is
+in use (via the dax_pmem_compat driver), the reconfiguration will fail. See
+linkdaxctl:daxctl-migrate-device-model[1] for more information.
+
+OPTIONS
+---
+-r::
+--region=::
+   Restrict the operation to devices belonging to the specified region(s).
+   A device-dax region is a contiguous range of memory that hosts one or
+   more /dev/daxX.Y devices, where X is the region id and Y is the device
+   instance id.
+
+-m::
+--mode=::
+   Specify the mode to which the dax device(s) should be reconfigured.
+   - "system-ram": hotplug the device into system memory.
+
+   - "devdax": switch to the normal "device dax" mode. This requires the
+ kernel to support hot-unplugging 'kmem' based memory. If this is not
+ available, a reboot is the only way to switch back to 'devdax' mode.
+
+-N::
+--no-online::
+   By default, memory sections provided by system-ram devices will be
+   brought online automatically and immediately with the 'online_movable'
+   policy. Use this option to disable the automatic onlining behavior.
+
+-O::
+--attempt-offline::
+   When converting from "system-ram" mode to "devdax", it is expected
+   that all the memory sections are first made offline. By default,
+   daxctl won't touch online memory. However with this option, attempt
+   to offline the memory on the NUMA node associated with the dax device
+   before converting it back to "devdax" mode.
+
+-u::
+--human::
+   By default the command will output machine-friendly raw-integer
+   data. Instead, with this flag, numbers representing storage size
+   will be formatted as human readable strings with units, other
+   fields are converted to hexadecimal strings.
+
+-v::
+--verbose::
+   Emit more debug messages
+
+include::../copyright.txt[]
+
+SEE ALSO
+

[ndctl PATCH v5 10/13] Documentation: Add man pages for daxctl-{on, off}line-memory

2019-06-28 Thread Vishal Verma
Add man pages for the two new commands: daxctl-online-memory, and
daxctl-offline-memory.

Cc: Dan Williams 
Cc: Dave Hansen 
Signed-off-by: Vishal Verma 
---
 Documentation/daxctl/Makefile.am  |  4 +-
 .../daxctl/daxctl-offline-memory.txt  | 72 +
 Documentation/daxctl/daxctl-online-memory.txt | 80 +++
 3 files changed, 155 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/daxctl/daxctl-offline-memory.txt
 create mode 100644 Documentation/daxctl/daxctl-online-memory.txt

diff --git a/Documentation/daxctl/Makefile.am b/Documentation/daxctl/Makefile.am
index 715fbad..37c3bde 100644
--- a/Documentation/daxctl/Makefile.am
+++ b/Documentation/daxctl/Makefile.am
@@ -29,7 +29,9 @@ man1_MANS = \
daxctl.1 \
daxctl-list.1 \
daxctl-migrate-device-model.1 \
-   daxctl-reconfigure-device.1
+   daxctl-reconfigure-device.1 \
+   daxctl-online-memory.1 \
+   daxctl-offline-memory.1
 
 CLEANFILES = $(man1_MANS)
 
diff --git a/Documentation/daxctl/daxctl-offline-memory.txt 
b/Documentation/daxctl/daxctl-offline-memory.txt
new file mode 100644
index 000..ba06287
--- /dev/null
+++ b/Documentation/daxctl/daxctl-offline-memory.txt
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+
+daxctl-offline-memory(1)
+
+
+NAME
+
+daxctl-offline-memory - Offline the memory for a device that is in system-ram 
mode
+
+SYNOPSIS
+
+[verse]
+'daxctl offline-memory'  [...] []
+
+EXAMPLES
+
+
+* Reconfigure dax0.0 to system-ram mode
+
+# daxctl reconfigure-device --mode=system-ram --human dax0.0
+{
+  "chardev":"dax0.0",
+  "size":"7.87 GiB (8.45 GB)",
+  "target_node":2,
+  "mode":"system-ram"
+}
+
+
+* Offline the memory
+
+# daxctl offline-memory dax0.0
+dax0.0: 62 sections offlined
+offlined memory for 1 device
+
+
+DESCRIPTION
+---
+
+Offline the memory sections associated with a device that has been converted
+to the system-ram mode. If one or more blocks are already offline, attempt to
+offline the remaining blocks. If all blocks were already offline, print a
+message and return success without actually doing anything.
+
+This is complementary to the 'daxctl-online-memory' command, and may be used
+when it is wished to offline the memory sections, but not convert the device
+back to 'devdax' mode.
+
+OPTIONS
+---
+-r::
+--region=::
+   Restrict the operation to devices belonging to the specified region(s).
+   A device-dax region is a contiguous range of memory that hosts one or
+   more /dev/daxX.Y devices, where X is the region id and Y is the device
+   instance id.
+
+-u::
+--human::
+   By default the command will output machine-friendly raw-integer
+   data. Instead, with this flag, numbers representing storage size
+   will be formatted as human readable strings with units, other
+   fields are converted to hexadecimal strings.
+
+-v::
+--verbose::
+   Emit more debug messages
+
+include::../copyright.txt[]
+
+SEE ALSO
+
+linkdaxctl:daxctl-reconfigure-device[1],daxctl-online-memory[1]
diff --git a/Documentation/daxctl/daxctl-online-memory.txt 
b/Documentation/daxctl/daxctl-online-memory.txt
new file mode 100644
index 000..5ac1cbf
--- /dev/null
+++ b/Documentation/daxctl/daxctl-online-memory.txt
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+daxctl-online-memory(1)
+===
+
+NAME
+
+daxctl-online-memory - Online the memory for a device that is in system-ram 
mode
+
+SYNOPSIS
+
+[verse]
+'daxctl online-memory'  [...] []
+
+EXAMPLES
+
+
+* Reconfigure dax0.0 to system-ram mode, don't online the memory
+
+# daxctl reconfigure-device --mode=system-ram --no-online --human dax0.0
+{
+  "chardev":"dax0.0",
+  "size":"7.87 GiB (8.45 GB)",
+  "target_node":2,
+  "mode":"system-ram"
+}
+
+
+* Online the memory separately
+
+# daxctl online-memory dax0.0
+dax0.0: 62 new sections onlined
+onlined memory for 1 device
+
+
+* Onlining memory when some sections were already online
+
+# daxctl online-memory dax0.0
+dax0.0: 1 section already online
+dax0.0: 61 new sections onlined
+onlined memory for 1 device
+
+
+DESCRIPTION
+---
+
+Online the memory sections associated with a device that has been converted
+to the system-ram mode. If one or more blocks are already online, print a
+message about them, and attempt to online the remaining blocks.
+
+This is complementary to the 'daxctl-reconfigure-device' command, when used 
with
+the '--no-online' option to skip onlining memory sections immediately after the
+reconfigure. In these scenarios, the memory can be onlined at a later time 
using
+'daxctl-online-memory'.
+
+OPTIONS
+---
+-r::
+--region=::
+   Restrict the operation to devices belonging to the specified region(s).
+   A device-dax region is a contiguous range of memory that hosts one or
+   more /dev/daxX.Y devices, 

[ndctl PATCH v5 07/13] daxctl: add a new reconfigure-device command

2019-06-28 Thread Vishal Verma
Add a new command 'daxctl-reconfigure-device'. This is used to switch
the mode of a dax device between regular 'device_dax' and
'system-memory'. The command also uses the memory hotplug sysfs
interfaces to online the newly available memory when converting to
'system-ram', and to attempt to offline the memory when converting back
to a DAX device.

Cc: Pavel Tatashin 
Cc: Dave Hansen 
Cc: Dan Williams 
Signed-off-by: Vishal Verma 
---
 daxctl/Makefile.am |   2 +
 daxctl/builtin.h   |   1 +
 daxctl/daxctl.c|   1 +
 daxctl/device.c| 348 +
 4 files changed, 352 insertions(+)
 create mode 100644 daxctl/device.c

diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am
index 94f73f9..66dcc7f 100644
--- a/daxctl/Makefile.am
+++ b/daxctl/Makefile.am
@@ -15,10 +15,12 @@ daxctl_SOURCES =\
daxctl.c \
list.c \
migrate.c \
+   device.c \
../util/json.c
 
 daxctl_LDADD =\
lib/libdaxctl.la \
../libutil.a \
$(UUID_LIBS) \
+   $(KMOD_LIBS) \
$(JSON_LIBS)
diff --git a/daxctl/builtin.h b/daxctl/builtin.h
index 00ef5e9..756ba2a 100644
--- a/daxctl/builtin.h
+++ b/daxctl/builtin.h
@@ -6,4 +6,5 @@
 struct daxctl_ctx;
 int cmd_list(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 #endif /* _DAXCTL_BUILTIN_H_ */
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index 2e41747..e1ba7b8 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -71,6 +71,7 @@ static struct cmd_struct commands[] = {
{ "list", .d_fn = cmd_list },
{ "help", .d_fn = cmd_help },
{ "migrate-device-model", .d_fn = cmd_migrate },
+   { "reconfigure-device", .d_fn = cmd_reconfig_device },
 };
 
 int main(int argc, const char **argv)
diff --git a/daxctl/device.c b/daxctl/device.c
new file mode 100644
index 000..a1fb698
--- /dev/null
+++ b/daxctl/device.c
@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2019 Intel Corporation. All rights reserved. */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static struct {
+   const char *dev;
+   const char *mode;
+   int region_id;
+   bool no_online;
+   bool do_offline;
+   bool human;
+   bool verbose;
+} param = {
+   .region_id = -1,
+};
+
+static enum daxctl_dev_mode reconfig_mode = DAXCTL_DEV_MODE_UNKNOWN;
+static unsigned long flags;
+
+enum device_action {
+   ACTION_RECONFIG,
+};
+
+#define BASE_OPTIONS() \
+OPT_INTEGER('r', "region", _id, "restrict to the given region"), \
+OPT_BOOLEAN('u', "human", , "use human friendly number formats"), \
+OPT_BOOLEAN('v', "verbose", , "emit more debug messages")
+
+#define RECONFIG_OPTIONS() \
+OPT_STRING('m', "mode", , "mode", "mode to switch the device to"), \
+OPT_BOOLEAN('N', "no-online", _online, \
+   "don't auto-online memory sections"), \
+OPT_BOOLEAN('O', "attempt-offline", _offline, \
+   "attempt to offline memory sections")
+
+static const struct option reconfig_options[] = {
+   BASE_OPTIONS(),
+   RECONFIG_OPTIONS(),
+   OPT_END(),
+};
+
+static const char *parse_device_options(int argc, const char **argv,
+   enum device_action action, const struct option *options,
+   const char *usage, struct daxctl_ctx *ctx)
+{
+   const char * const u[] = {
+   usage,
+   NULL
+   };
+   int i, rc = 0;
+
+   argc = parse_options(argc, argv, options, u, 0);
+
+   /* Handle action-agnostic non-option arguments */
+   if (argc == 0) {
+   char *action_string;
+
+   switch (action) {
+   case ACTION_RECONFIG:
+   action_string = "reconfigure";
+   break;
+   default:
+   action_string = "<>";
+   break;
+   }
+   fprintf(stderr, "specify a device to %s, or \"all\"\n",
+   action_string);
+   rc = -EINVAL;
+   }
+   for (i = 1; i < argc; i++) {
+   fprintf(stderr, "unknown extra parameter \"%s\"\n", argv[i]);
+   rc = -EINVAL;
+   }
+
+   if (rc) {
+   usage_with_options(u, options);
+   return NULL;
+   }
+
+   /* Handle action-agnostic options */
+   if (param.verbose)
+   daxctl_set_log_priority(ctx, LOG_DEBUG);
+   if (param.human)
+   flags |= UTIL_JSON_HUMAN;
+
+   /* Handle action-specific options */
+   switch (action) {
+   case ACTION_RECONFIG:
+   if (!param.mode) {
+   fprintf(stderr, "error: a 'mode' option is required\n");
+   

[ndctl PATCH v5 09/13] daxctl: add commands to online and offline memory

2019-06-28 Thread Vishal Verma
Add two new commands:

  daxctl-online-memory
  daxctl-offline-memory

to manage the state of hot-plugged memory from the system-ram mode for
dax devices. This provides a way for the user to online/offline the
memory as a separate step from the reconfiguration. Without this, a user
that reconfigures a device into the system-ram mode with the --no-online
option, would have no way to later online the memory, and would have to
resort to shell scripting to online them manually via sysfs.

Cc: Dan Williams 
Cc: Dave Hansen 
Signed-off-by: Vishal Verma 
---
 daxctl/builtin.h |   2 +
 daxctl/daxctl.c  |   2 +
 daxctl/device.c  | 138 ++-
 3 files changed, 141 insertions(+), 1 deletion(-)

diff --git a/daxctl/builtin.h b/daxctl/builtin.h
index 756ba2a..f5a0147 100644
--- a/daxctl/builtin.h
+++ b/daxctl/builtin.h
@@ -7,4 +7,6 @@ struct daxctl_ctx;
 int cmd_list(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_online_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_offline_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
 #endif /* _DAXCTL_BUILTIN_H_ */
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index e1ba7b8..1ab0732 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -72,6 +72,8 @@ static struct cmd_struct commands[] = {
{ "help", .d_fn = cmd_help },
{ "migrate-device-model", .d_fn = cmd_migrate },
{ "reconfigure-device", .d_fn = cmd_reconfig_device },
+   { "online-memory", .d_fn = cmd_online_memory },
+   { "offline-memory", .d_fn = cmd_offline_memory },
 };
 
 int main(int argc, const char **argv)
diff --git a/daxctl/device.c b/daxctl/device.c
index a1fb698..22cf0c8 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -30,6 +30,8 @@ static unsigned long flags;
 
 enum device_action {
ACTION_RECONFIG,
+   ACTION_ONLINE,
+   ACTION_OFFLINE,
 };
 
 #define BASE_OPTIONS() \
@@ -50,6 +52,11 @@ static const struct option reconfig_options[] = {
OPT_END(),
 };
 
+static const struct option memory_options[] = {
+   BASE_OPTIONS(),
+   OPT_END(),
+};
+
 static const char *parse_device_options(int argc, const char **argv,
enum device_action action, const struct option *options,
const char *usage, struct daxctl_ctx *ctx)
@@ -70,6 +77,12 @@ static const char *parse_device_options(int argc, const char 
**argv,
case ACTION_RECONFIG:
action_string = "reconfigure";
break;
+   case ACTION_ONLINE:
+   action_string = "online memory for";
+   break;
+   case ACTION_OFFLINE:
+   action_string = "offline memory for";
+   break;
default:
action_string = "<>";
break;
@@ -118,6 +131,10 @@ static const char *parse_device_options(int argc, const 
char **argv,
}
}
break;
+   case ACTION_ONLINE:
+   case ACTION_OFFLINE:
+   /* nothing special */
+   break;
}
if (rc) {
usage_with_options(u, options);
@@ -287,10 +304,82 @@ static int do_reconfig(struct daxctl_dev *dev, enum 
daxctl_dev_mode mode,
return rc;
 }
 
+static int do_xline(struct daxctl_dev *dev, enum device_action action)
+{
+   struct daxctl_memory *mem = daxctl_dev_get_memory(dev);
+   const char *devname = daxctl_dev_get_devname(dev);
+   enum daxctl_dev_mode mode;
+   int rc, num_online;
+
+   if (!daxctl_dev_is_enabled(dev)) {
+   fprintf(stderr,
+   "%s: memory operations not possible when disabled\n",
+   devname);
+   return -ENXIO;
+   }
+
+   mode = daxctl_dev_get_mode(dev);
+   if (mode < 0) {
+   fprintf(stderr, "%s: unable to determine current mode: %s\n",
+   devname, strerror(-mode));
+   return rc;
+   }
+   if (mode == DAXCTL_DEV_MODE_DEVDAX) {
+   fprintf(stderr,
+   "%s: memory operations are not applicable in devdax 
mode\n",
+   devname);
+   return -ENXIO;
+   }
+
+   /* We are enabled, and in the correct mode. Proceed. */
+   num_online = daxctl_memory_is_online(mem);
+   if (num_online < 0) {
+   fprintf(stderr, "%s: unable to determine online state: %s\n",
+   devname, strerror(-num_online));
+   return num_online;
+   }
+
+   switch (action) {
+   case ACTION_ONLINE:
+   if (num_online > 0)
+   fprintf(stderr, "%s: %d section%s already online\n",
+

[ndctl PATCH v5 04/13] libdaxctl: add a 'daxctl_memory' object for memory based operations

2019-06-28 Thread Vishal Verma
Introduce a new 'daxctl_memory' object, which will be used for
operations related to managing dax devices in 'system-memory' modes.

Add libdaxctl APIs to get the target_node of a DAX device, and to
online, offline, and query the state of hotplugged memory sections
associated with a given device.

This adds the following new interfaces:

  daxctl_dev_get_target_node
  daxctl_dev_get_memory;
  daxctl_memory_get_dev;
  daxctl_memory_get_node_path;
  daxctl_memory_get_block_size;
  daxctl_memory_set_online
  daxctl_memory_set_offline
  daxctl_memory_is_online

Cc: Pavel Tatashin 
Cc: Dave Hansen 
Cc: Dan Williams 
Signed-off-by: Vishal Verma 
---
 daxctl/lib/libdaxctl-private.h |  14 ++
 daxctl/lib/libdaxctl.c | 333 +
 daxctl/lib/libdaxctl.sym   |   8 +
 daxctl/libdaxctl.h |  10 +
 4 files changed, 365 insertions(+)

diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
index eb7c1ec..673be0f 100644
--- a/daxctl/lib/libdaxctl-private.h
+++ b/daxctl/lib/libdaxctl-private.h
@@ -33,6 +33,11 @@ static const char *dax_modules[] = {
[DAXCTL_DEV_MODE_RAM] = "kmem",
 };
 
+enum memory_state {
+   MEM_OFFLINE,
+   MEM_ONLINE,
+};
+
 /**
  * struct daxctl_region - container for dax_devices
  */
@@ -64,8 +69,17 @@ struct daxctl_dev {
struct kmod_module *module;
struct kmod_list *kmod_list;
struct daxctl_region *region;
+   struct daxctl_memory *mem;
+   int target_node;
 };
 
+struct daxctl_memory {
+struct daxctl_dev *dev;
+   char *node_path;
+   unsigned long block_size;
+};
+
+
 static inline int check_kmod(struct kmod_ctx *kmod_ctx)
 {
return kmod_ctx ? 0 : -ENXIO;
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 5431063..b22df50 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -200,6 +200,12 @@ DAXCTL_EXPORT void daxctl_region_get_uuid(struct 
daxctl_region *region, uuid_t u
uuid_copy(uu, region->uuid);
 }
 
+static void free_mem(struct daxctl_memory *mem)
+{
+   free(mem->node_path);
+   free(mem);
+}
+
 static void free_dev(struct daxctl_dev *dev, struct list_head *head)
 {
if (head)
@@ -207,6 +213,7 @@ static void free_dev(struct daxctl_dev *dev, struct 
list_head *head)
kmod_module_unref_list(dev->kmod_list);
free(dev->dev_buf);
free(dev->dev_path);
+   free_mem(dev->mem);
free(dev);
 }
 
@@ -343,6 +350,44 @@ static struct kmod_list *to_module_list(struct daxctl_ctx 
*ctx,
return list;
 }
 
+static struct daxctl_memory *daxctl_dev_init_mem(struct daxctl_dev *dev)
+{
+   const char *size_path = "/sys/devices/system/memory/block_size_bytes";
+   const char *node_base = "/sys/devices/system/node/node";
+   const char *devname = daxctl_dev_get_devname(dev);
+   struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+   struct daxctl_memory *mem;
+   char buf[SYSFS_ATTR_SIZE];
+   int node_num;
+
+   mem = calloc(1, sizeof(*mem));
+   if (!mem)
+   return NULL;
+
+   mem->dev = dev;
+
+   /*
+* Everything here is best-effort, we won't fail the device add
+* for anything other than the ENOMEM case above.
+*/
+   if (sysfs_read_attr(ctx, size_path, buf) == 0) {
+   mem->block_size = strtoul(buf, NULL, 16);
+   if (mem->block_size == 0 || mem->block_size == ULONG_MAX) {
+   err(ctx, "%s: Unable to determine memblock size: %s\n",
+   devname, strerror(errno));
+   mem->block_size = 0;
+   }
+   }
+
+   node_num = daxctl_dev_get_target_node(dev);
+   if (node_num >= 0) {
+   if (asprintf(>node_path, "%s%d", node_base, node_num) < 0)
+   err(ctx, "%s: Unable to set node_path\n", devname);
+   }
+
+   return mem;
+}
+
 static void *add_dax_dev(void *parent, int id, const char *daxdev_base)
 {
const char *devname = devpath_to_devname(daxdev_base);
@@ -398,6 +443,16 @@ static void *add_dax_dev(void *parent, int id, const char 
*daxdev_base)
if (rc == 0)
dev->kmod_list = to_module_list(ctx, buf);
 
+   sprintf(path, "%s/target_node", daxdev_base);
+   if (sysfs_read_attr(ctx, path, buf) == 0)
+   dev->target_node = strtol(buf, NULL, 0);
+   else
+   dev->target_node = -1;
+
+   dev->mem = daxctl_dev_init_mem(dev);
+   if (!dev->mem)
+   goto err_read;
+
daxctl_dev_foreach(region, dev_dup)
if (dev_dup->id == dev->id) {
free_dev(dev, NULL);
@@ -894,3 +949,281 @@ DAXCTL_EXPORT unsigned long long 
daxctl_dev_get_size(struct daxctl_dev *dev)
 {
return dev->size;
 }
+
+DAXCTL_EXPORT int daxctl_dev_get_target_node(struct daxctl_dev *dev)
+{
+   return dev->target_node;
+}
+
+DAXCTL_EXPORT struct daxctl_memory 

[ndctl PATCH v5 05/13] daxctl/list: add target_node for device listings

2019-06-28 Thread Vishal Verma
The kernel provides a 'target_node' attribute for dax devices. When
converting a dax device to the system-ram mode, the memory is hotplugged
into this numa node. It would be helpful to print this in device
listings so that it is easy for applications to detect the node to
which the new memory belongs.

Cc: Dan Williams 
Cc: Dave Hansen 
Signed-off-by: Vishal Verma 
---
 util/json.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/util/json.c b/util/json.c
index babdc8c..f521337 100644
--- a/util/json.c
+++ b/util/json.c
@@ -271,6 +271,7 @@ struct json_object *util_daxctl_dev_to_json(struct 
daxctl_dev *dev,
 {
const char *devname = daxctl_dev_get_devname(dev);
struct json_object *jdev, *jobj;
+   int node;
 
jdev = json_object_new_object();
if (!devname || !jdev)
@@ -284,6 +285,13 @@ struct json_object *util_daxctl_dev_to_json(struct 
daxctl_dev *dev,
if (jobj)
json_object_object_add(jdev, "size", jobj);
 
+   node = daxctl_dev_get_target_node(dev);
+   if (node >= 0) {
+   jobj = json_object_new_int(node);
+   if (jobj)
+   json_object_object_add(jdev, "target_node", jobj);
+   }
+
return jdev;
 }
 
-- 
2.20.1

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


[ndctl PATCH v5 02/13] libdaxctl: add interfaces to enable/disable devices

2019-06-28 Thread Vishal Verma
Add new libdaxctl interfaces to disable a device_dax based device, and
to enable it into the given mode. The modes available are 'devdax',
and 'system-ram', where devdax is the normal device DAX mode used
via a character device, and 'system-ram' uses the kernel's 'kmem'
facility to hotplug the device making it usable as normal memory.

This adds the following new interfaces:

  daxctl_dev_disable;
  daxctl_dev_enable_devdax;
  daxctl_dev_enable_ram;

Cc: Dave Hansen 
Cc: Dan Williams 
Signed-off-by: Vishal Verma 
---
 daxctl/lib/Makefile.am |   3 +-
 daxctl/lib/libdaxctl-private.h |  15 +++
 daxctl/lib/libdaxctl.c | 234 +
 daxctl/lib/libdaxctl.sym   |   3 +
 daxctl/libdaxctl.h |   9 ++
 5 files changed, 263 insertions(+), 1 deletion(-)

diff --git a/daxctl/lib/Makefile.am b/daxctl/lib/Makefile.am
index d3d4852..9f0e444 100644
--- a/daxctl/lib/Makefile.am
+++ b/daxctl/lib/Makefile.am
@@ -16,7 +16,8 @@ libdaxctl_la_SOURCES =\
libdaxctl.c
 
 libdaxctl_la_LIBADD =\
-   $(UUID_LIBS)
+   $(UUID_LIBS) \
+   $(KMOD_LIBS)
 
 daxctl_modprobe_data_DATA = daxctl.conf
 
diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
index 4a462e7..e64d0a7 100644
--- a/daxctl/lib/libdaxctl-private.h
+++ b/daxctl/lib/libdaxctl-private.h
@@ -13,6 +13,8 @@
 #ifndef _LIBDAXCTL_PRIVATE_H_
 #define _LIBDAXCTL_PRIVATE_H_
 
+#include 
+
 #define DAXCTL_EXPORT __attribute__ ((visibility("default")))
 
 enum dax_subsystem {
@@ -26,6 +28,11 @@ static const char *dax_subsystems[] = {
[DAX_BUS] = "/sys/bus/dax/devices",
 };
 
+static const char *dax_modules[] = {
+   [DAXCTL_DEV_MODE_DEVDAX] = "device_dax",
+   [DAXCTL_DEV_MODE_RAM] = "kmem",
+};
+
 /**
  * struct daxctl_region - container for dax_devices
  */
@@ -53,6 +60,14 @@ struct daxctl_dev {
char *dev_path;
struct list_node list;
unsigned long long size;
+   struct kmod_module *module;
+   struct kmod_list *kmod_list;
struct daxctl_region *region;
 };
+
+static inline int check_kmod(struct kmod_ctx *kmod_ctx)
+{
+   return kmod_ctx ? 0 : -ENXIO;
+}
+
 #endif /* _LIBDAXCTL_PRIVATE_H_ */
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 70f896b..05f013b 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -46,6 +46,7 @@ struct daxctl_ctx {
void *userdata;
int regions_init;
struct list_head regions;
+   struct kmod_ctx *kmod_ctx;
 };
 
 /**
@@ -84,20 +85,32 @@ DAXCTL_EXPORT void daxctl_set_userdata(struct daxctl_ctx 
*ctx, void *userdata)
  */
 DAXCTL_EXPORT int daxctl_new(struct daxctl_ctx **ctx)
 {
+   struct kmod_ctx *kmod_ctx;
struct daxctl_ctx *c;
+   int rc = 0;
 
c = calloc(1, sizeof(struct daxctl_ctx));
if (!c)
return -ENOMEM;
 
+   kmod_ctx = kmod_new(NULL, NULL);
+   if (check_kmod(kmod_ctx) != 0) {
+   rc = -ENXIO;
+   goto out;
+   }
+
c->refcount = 1;
log_init(>ctx, "libdaxctl", "DAXCTL_LOG");
info(c, "ctx %p created\n", c);
dbg(c, "log_priority=%d\n", c->ctx.log_priority);
*ctx = c;
list_head_init(>regions);
+   c->kmod_ctx = kmod_ctx;
 
return 0;
+out:
+   free(c);
+   return rc;
 }
 
 /**
@@ -132,6 +145,7 @@ DAXCTL_EXPORT void daxctl_unref(struct daxctl_ctx *ctx)
list_for_each_safe(>regions, region, _r, list)
free_region(region, >regions);
 
+   kmod_unref(ctx->kmod_ctx);
info(ctx, "context %p released\n", ctx);
free(ctx);
 }
@@ -189,6 +203,7 @@ static void free_dev(struct daxctl_dev *dev, struct 
list_head *head)
 {
if (head)
list_del_from(head, >list);
+   kmod_module_unref_list(dev->kmod_list);
free(dev->dev_buf);
free(dev->dev_path);
free(dev);
@@ -306,6 +321,27 @@ DAXCTL_EXPORT struct daxctl_region 
*daxctl_new_region(struct daxctl_ctx *ctx,
return region;
 }
 
+static struct kmod_list *to_module_list(struct daxctl_ctx *ctx,
+   const char *alias)
+{
+   struct kmod_list *list = NULL;
+   int rc;
+
+   if (!ctx->kmod_ctx || !alias)
+   return NULL;
+   if (alias[0] == 0)
+   return NULL;
+
+   rc = kmod_module_new_from_lookup(ctx->kmod_ctx, alias, );
+   if (rc < 0 || !list) {
+   dbg(ctx, "failed to find modules for alias: %s %d list: %s\n",
+   alias, rc, list ? "populated" : "empty");
+   return NULL;
+   }
+
+   return list;
+}
+
 static void *add_dax_dev(void *parent, int id, const char *daxdev_base)
 {
const char *devname = devpath_to_devname(daxdev_base);
@@ -315,6 +351,7 @@ static void *add_dax_dev(void *parent, int id, const char 
*daxdev_base)
struct daxctl_dev *dev, *dev_dup;
char buf[SYSFS_ATTR_SIZE];
struct stat st;
+   int rc;

[ndctl PATCH v5 01/13] libdaxctl: add interfaces to get ctx and check device state

2019-06-28 Thread Vishal Verma
In preparation for libdaxctl and daxctl to grow operational modes for
DAX devices, add the following supporting APIs:

  daxctl_dev_get_ctx
  daxctl_dev_is_enabled

Cc: Dan Williams 
Signed-off-by: Vishal Verma 
---
 daxctl/lib/libdaxctl.c   | 30 ++
 daxctl/lib/libdaxctl.sym |  6 ++
 daxctl/libdaxctl.h   |  2 ++
 3 files changed, 38 insertions(+)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index c2e3a52..70f896b 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -559,6 +559,36 @@ static void dax_regions_init(struct daxctl_ctx *ctx)
}
 }
 
+static int is_enabled(const char *drvpath)
+{
+   struct stat st;
+
+   if (lstat(drvpath, ) < 0 || !S_ISLNK(st.st_mode))
+   return 0;
+   else
+   return 1;
+}
+
+DAXCTL_EXPORT int daxctl_dev_is_enabled(struct daxctl_dev *dev)
+{
+   struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+   char *path = dev->dev_buf;
+   int len = dev->buf_len;
+
+   if (snprintf(path, len, "%s/driver", dev->dev_path) >= len) {
+   err(ctx, "%s: buffer too small!\n",
+   daxctl_dev_get_devname(dev));
+   return 0;
+   }
+
+   return is_enabled(path);
+}
+
+DAXCTL_EXPORT struct daxctl_ctx *daxctl_dev_get_ctx(struct daxctl_dev *dev)
+{
+   return dev->region->ctx;
+}
+
 DAXCTL_EXPORT struct daxctl_dev *daxctl_dev_get_first(struct daxctl_region 
*region)
 {
dax_devices_init(region);
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index 84d3a69..c4af9a7 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -50,3 +50,9 @@ LIBDAXCTL_5 {
 global:
daxctl_region_get_path;
 } LIBDAXCTL_4;
+
+LIBDAXCTL_6 {
+global:
+   daxctl_dev_get_ctx;
+   daxctl_dev_is_enabled;
+} LIBDAXCTL_5;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index 1d13ea2..e20ccb4 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -67,6 +67,8 @@ const char *daxctl_dev_get_devname(struct daxctl_dev *dev);
 int daxctl_dev_get_major(struct daxctl_dev *dev);
 int daxctl_dev_get_minor(struct daxctl_dev *dev);
 unsigned long long daxctl_dev_get_size(struct daxctl_dev *dev);
+struct daxctl_ctx *daxctl_dev_get_ctx(struct daxctl_dev *dev);
+int daxctl_dev_is_enabled(struct daxctl_dev *dev);
 
 #define daxctl_dev_foreach(region, dev) \
 for (dev = daxctl_dev_get_first(region); \
-- 
2.20.1

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


[ndctl PATCH v5 00/13] daxctl: add a new reconfigure-device command

2019-06-28 Thread Vishal Verma
Changes in v5:
 - device.c: correctly set loglevel for daxctl_ctx for --verbose
 - drop the subsys caching, its complexity started to exceed its
   benefit. dax-class device models will simply error out during
   reconfigure. (Dan)
 - Add a note to the man page for the above.
 - Clarify the onlining policy (online_movable) in the man page
 - rename "numa_node" to "target_node" in device listings (Dan)
 - When printing a device 'mode', assume devdax if !system-ram,
   avoiding a "mode: unknown" situation which can be confusing. (Dan)
 - Add a "state: disabled" attribute to the device listing if a driver
   is not bound. This is more apt than the previous "mode: unknown"
   listing.
 - add an api to get 'dev->resource' parsing /proc/iomem as a
   fallback for when the kernel doesn't provide the attribute (Dan)
 - convert node_* apis to 'memory_* apis that act on a new daxctl_memory
   object (Dan)
 - online only memory sections belonging to the device in question by
   cross referencing block indices with the dax device resource (Dan)
 - Refuse to reconfigure a device that is already in the target mode.
   Until now, reconfiguring a system-ram device back to system-ram would
   result in a 'online memory may not be hot-removed' kernel warning.
 - If the device was already in the system-ram mode, skip
   disabling/enabling, but still try to online the memory unless the
   --no-online option is in effect.
 - In daxctl_unbind, also 'remove_id' to prevent devices automatically
   binding to the kmem driver on a disable + re-enable, which can be
   surprising (Dan).
 - Rewrite the top half of daxctl/device.c to borrow elements from
   ndctl/namespace.c so that it can support growing additional commands
   that operate on devices (online-memory and offline-memory)
 - Refactor the bottom half of daxctl/device.c so we only do the
   disabling/offlining steps if the device was enabled.
 - Add new commands to online and offline memory sections (Dan)
   associated with a given dax device (Dan)
 - Add a new test - daxctl-device.sh - to test daxctl reconfigure-device,
   online-memory, and offline-memory commands.
 - Add an example in documentation demonstrating how to use numactl
   to bind a process to a node surfaced from a dax device (Andy Rudoff)

Changes in v4:
 - Don't fail add_dax_dev for kmod failures. Instead fail only when the kmod
   list is actually used, i.e. during daxctl-reconfigure-device

Changes in v3:
 - In daxctl_dev_get_mode(), remove the subsystem warning, detect dax-class
   and simply make it return devdax

Changes in v2:
 - Add examples to the documentation page (Dave Hansen)
 - Clarify documentation regarding the conversion from system-ram to devdax
 - Remove any references to a persistent config from the documentation -
   those can be added when the feature is added.
 - device.c: validate option compatibility
 - daxctl-list: display numa_node for device listings
 - daxctl-list: display mode for device listings
 - make the options more consistent by adding a '-O' short option
   for --attempt-offline

Add a new daxctl-reconfigure-device command that lets us reconfigure DAX
devices back and forth between 'system-ram' and 'device-dax' modes. It
also includes facilities to online any newly hot-plugged memory
(default), and attempt to offline memory before converting away from the
system-ram mode (not default, requires a --attempt-offline option).

Currently missing from this series is a way to persistently store which
devices have been 'marked' for use as system-ram. This depends on a
config system overhaul in ndctl, and patches for those will follow
separately and are independent of this work.

Example invocations:

1. Reconfigure dax0.0 to system-ram mode, don’t online the memory
# daxctl reconfigure-device --mode=system-ram --no-online dax0.0
[
  {
"chardev":"dax0.0",
"size":16777216000,
"target_node":2,
"mode":"system-ram"
  }
]

2. Reconfigure dax0.0 to devdax mode, attempt to offline the memory
# daxctl reconfigure-device --human --mode=devdax --attempt-offline dax0.0
{
  "chardev":"dax0.0",
  "size":"15.63 GiB (16.78 GB)",
  "target_node":2,
  "mode":"devdax"
}

3. Reconfigure all dax devices on region0 to system-ram mode
# daxctl reconfigure-device --mode=system-ram --region=0 all
[
  {
"chardev":"dax0.0",
"size":16777216000,
"target_node":2,
"mode":"system-ram"
  },
  {
"chardev":"dax0.1",
"size":16777216000,
"target_node":3,
"mode":"system-ram"
  }
]

These patches can also be found in the 'kmem-pending' branch on github:
https://github.com/pmem/ndctl/tree/kmem-pending

Cc: Dan Williams 
Cc: Dave Hansen 
Cc: Pavel Tatashin 


Vishal Verma (13):
  libdaxctl: add interfaces to get ctx and check device state
  libdaxctl: add interfaces to enable/disable devices
  libdaxctl: add an interface to retrieve the device 

[ndctl PATCH v5 03/13] libdaxctl: add an interface to retrieve the device resource

2019-06-28 Thread Vishal Verma
Add an interface to retrieve the 'resource' attribute for a dax device.

Attempt to retrieve it as usual via sysfs, but since older kernels may
be missing this attribute, as a fallback, attempt to retrieve it from
/proc/iomem

Cc: Dan Williams 
[fscanf format string problem and diagnosis]
Reported-by: Tony Luck 
Signed-off-by: Vishal Verma 
---
 Makefile.am|  3 ++-
 daxctl/lib/Makefile.am |  2 ++
 daxctl/lib/libdaxctl-private.h |  1 +
 daxctl/lib/libdaxctl.c | 12 +++
 daxctl/lib/libdaxctl.sym   |  1 +
 daxctl/libdaxctl.h |  1 +
 util/iomem.c   | 37 ++
 util/iomem.h   | 12 +++
 8 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 util/iomem.c
 create mode 100644 util/iomem.h

diff --git a/Makefile.am b/Makefile.am
index df8797e..8d10a10 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -74,6 +74,7 @@ libutil_a_SOURCES = \
util/wrapper.c \
util/filter.c \
util/bitmap.c \
-   util/abspath.c
+   util/abspath.c \
+   util/iomem.c
 
 nobase_include_HEADERS = daxctl/libdaxctl.h
diff --git a/daxctl/lib/Makefile.am b/daxctl/lib/Makefile.am
index 9f0e444..7704b1b 100644
--- a/daxctl/lib/Makefile.am
+++ b/daxctl/lib/Makefile.am
@@ -9,6 +9,8 @@ lib_LTLIBRARIES = libdaxctl.la
 libdaxctl_la_SOURCES =\
../libdaxctl.h \
libdaxctl-private.h \
+   ../../util/iomem.c \
+   ../../util/iomem.h \
../../util/sysfs.c \
../../util/sysfs.h \
../../util/log.c \
diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
index e64d0a7..eb7c1ec 100644
--- a/daxctl/lib/libdaxctl-private.h
+++ b/daxctl/lib/libdaxctl-private.h
@@ -59,6 +59,7 @@ struct daxctl_dev {
size_t buf_len;
char *dev_path;
struct list_node list;
+   unsigned long long resource;
unsigned long long size;
struct kmod_module *module;
struct kmod_list *kmod_list;
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 05f013b..5431063 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -24,6 +24,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include "libdaxctl-private.h"
 
@@ -369,6 +370,12 @@ static void *add_dax_dev(void *parent, int id, const char 
*daxdev_base)
dev->major = major(st.st_rdev);
dev->minor = minor(st.st_rdev);
 
+   sprintf(path, "%s/resource", daxdev_base);
+   if (sysfs_read_attr(ctx, path, buf) == 0)
+   dev->resource = strtoull(buf, NULL, 0);
+   else
+   dev->resource = iomem_get_dev_resource(ctx, daxdev_base);
+
sprintf(path, "%s/size", daxdev_base);
if (sysfs_read_attr(ctx, path, buf) < 0)
goto err_read;
@@ -878,6 +885,11 @@ DAXCTL_EXPORT int daxctl_dev_get_minor(struct daxctl_dev 
*dev)
return dev->minor;
 }
 
+DAXCTL_EXPORT unsigned long long daxctl_dev_get_resource(struct daxctl_dev 
*dev)
+{
+   return dev->resource;
+}
+
 DAXCTL_EXPORT unsigned long long daxctl_dev_get_size(struct daxctl_dev *dev)
 {
return dev->size;
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index 19904a2..1692624 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -58,4 +58,5 @@ global:
daxctl_dev_disable;
daxctl_dev_enable_devdax;
daxctl_dev_enable_ram;
+   daxctl_dev_get_resource;
 } LIBDAXCTL_5;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index b80488e..7214cd3 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -66,6 +66,7 @@ int daxctl_dev_get_id(struct daxctl_dev *dev);
 const char *daxctl_dev_get_devname(struct daxctl_dev *dev);
 int daxctl_dev_get_major(struct daxctl_dev *dev);
 int daxctl_dev_get_minor(struct daxctl_dev *dev);
+unsigned long long daxctl_dev_get_resource(struct daxctl_dev *dev);
 unsigned long long daxctl_dev_get_size(struct daxctl_dev *dev);
 struct daxctl_ctx *daxctl_dev_get_ctx(struct daxctl_dev *dev);
 int daxctl_dev_is_enabled(struct daxctl_dev *dev);
diff --git a/util/iomem.c b/util/iomem.c
new file mode 100644
index 000..a3c23f5
--- /dev/null
+++ b/util/iomem.c
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2019 Intel Corporation. All rights reserved. */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+unsigned long long __iomem_get_dev_resource(struct log_ctx *ctx,
+   const char *devpath)
+{
+   const char *devname = devpath_to_devname(devpath);
+   FILE *fp = fopen("/proc/iomem", "r");
+   unsigned long long res;
+   char name[256];
+
+   if (fp == NULL) {
+   log_err(ctx, "%s: open /proc/iomem: %s\n", devname,
+   strerror(errno));
+   return 0;
+   }
+
+   while (fscanf(fp, "%llx-%*x : %254[^\n]\n", , name) == 2) {
+   if (strcmp(name, devname) == 0) {
+

Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Christoph Hellwig
On Fri, Jun 28, 2019 at 11:59:19AM -0700, Dan Williams wrote:
> It's a bug that the call to put_devmap_managed_page() was gated by
> MEMORY_DEVICE_PUBLIC in release_pages(). That path is also applicable
> to MEMORY_DEVICE_FSDAX because it needs to trigger the ->page_free()
> callback to wake up wait_on_var() via fsdax_pagefree().
> 
> So I guess you could argue that the MEMORY_DEVICE_PUBLIC removal patch
> left the original bug in place. In that sense we're no worse off, but
> since we know about the bug, the fix and the patches have not been
> applied yet, why not fix it now?

The fix it now would simply be to apply Ira original patch now, but
given that we are at -rc6 is this really a good time?  And if we don't
apply it now based on the quilt based -mm worflow it just seems a lot
easier to apply it after my series.  Unless we want to include it in
the series, in which case I can do a quick rebase, we'd just need to
make sure Andrew pulls it from -mm.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 11:52 AM Christoph Hellwig  wrote:
>
> On Fri, Jun 28, 2019 at 11:44:35AM -0700, Dan Williams wrote:
> > There is a problem with the series in CH's tree. It removes the
> > ->page_free() callback from the release_pages() path because it goes
> > too far and removes the put_devmap_managed_page() call.
>
> release_pages only called put_devmap_managed_page for device public
> pages.  So I can't see how that is in any way a problem.

It's a bug that the call to put_devmap_managed_page() was gated by
MEMORY_DEVICE_PUBLIC in release_pages(). That path is also applicable
to MEMORY_DEVICE_FSDAX because it needs to trigger the ->page_free()
callback to wake up wait_on_var() via fsdax_pagefree().

So I guess you could argue that the MEMORY_DEVICE_PUBLIC removal patch
left the original bug in place. In that sense we're no worse off, but
since we know about the bug, the fix and the patches have not been
applied yet, why not fix it now?
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Christoph Hellwig
On Fri, Jun 28, 2019 at 11:44:35AM -0700, Dan Williams wrote:
> There is a problem with the series in CH's tree. It removes the
> ->page_free() callback from the release_pages() path because it goes
> too far and removes the put_devmap_managed_page() call.

release_pages only called put_devmap_managed_page for device public
pages.  So I can't see how that is in any way a problem.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 11:29 AM Jason Gunthorpe  wrote:
>
> On Fri, Jun 28, 2019 at 10:10:12AM -0700, Dan Williams wrote:
> > On Fri, Jun 28, 2019 at 10:08 AM Dan Williams  
> > wrote:
> > >
> > > On Fri, Jun 28, 2019 at 10:02 AM Jason Gunthorpe  
> > > wrote:
> > > >
> > > > On Fri, Jun 28, 2019 at 09:27:44AM -0700, Dan Williams wrote:
> > > > > On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe  
> > > > > wrote:
> > > > > >
> > > > > > On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote:
> > > > > > > The functionality is identical to the one currently open coded in
> > > > > > > device-dax.
> > > > > > >
> > > > > > > Signed-off-by: Christoph Hellwig 
> > > > > > > Reviewed-by: Ira Weiny 
> > > > > > >  drivers/dax/dax-private.h |  4 
> > > > > > >  drivers/dax/device.c  | 43 
> > > > > > > ---
> > > > > > >  2 files changed, 47 deletions(-)
> > > > > >
> > > > > > DanW: I think this series has reached enough review, did you want
> > > > > > to ack/test any further?
> > > > > >
> > > > > > This needs to land in hmm.git soon to make the merge window.
> > > > >
> > > > > I was awaiting a decision about resolving the collision with Ira's
> > > > > patch before testing the final result again [1]. You can go ahead and
> > > > > add my reviewed-by for the series, but my tested-by should be on the
> > > > > final state of the series.
> > > >
> > > > The conflict looks OK to me, I think we can let Andrew and Linus
> > > > resolve it.
> > >
> > > Andrew's tree effectively always rebases since it's a quilt series.
> > > I'd recommend pulling Ira's patch out of -mm and applying it with the
> > > rest of hmm reworks. Any other git tree I'd agree with just doing the
> > > late conflict resolution, but I'm not clear on what's the best
> > > practice when conflicting with -mm.
>
> What happens depends on timing as things arrive to Linus. I promised
> to send hmm.git early, so I understand that Andrew will quilt rebase
> his tree to Linus's and fix the conflict in Ira's patch before he
> sends it.
>
> > Regardless the patch is buggy. If you want to do the conflict
> > resolution it should be because the DEVICE_PUBLIC removal effectively
> > does the same fix otherwise we're knowingly leaving a broken point in
> > the history.
>
> I'm not sure I understand your concern, is there something wrong with
> CH's series as it stands? hmm is a non-rebasing git tree, so as long
> as the series is correct *when I apply it* there is no broken history.
>
> I assumed the conflict resolution for Ira's patch was to simply take
> the deletion of the if block from CH's series - right?
>
> If we do need to take Ira's patch into hmm.git it will go after CH's
> series (and Ira will have to rebase/repost it), so I think there is
> nothing to do at this moment - unless you are saying there is a
> problem with the series in CH's git tree?

There is a problem with the series in CH's tree. It removes the
->page_free() callback from the release_pages() path because it goes
too far and removes the put_devmap_managed_page() call.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Jason Gunthorpe
On Fri, Jun 28, 2019 at 10:10:12AM -0700, Dan Williams wrote:
> On Fri, Jun 28, 2019 at 10:08 AM Dan Williams  
> wrote:
> >
> > On Fri, Jun 28, 2019 at 10:02 AM Jason Gunthorpe  wrote:
> > >
> > > On Fri, Jun 28, 2019 at 09:27:44AM -0700, Dan Williams wrote:
> > > > On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe  
> > > > wrote:
> > > > >
> > > > > On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote:
> > > > > > The functionality is identical to the one currently open coded in
> > > > > > device-dax.
> > > > > >
> > > > > > Signed-off-by: Christoph Hellwig 
> > > > > > Reviewed-by: Ira Weiny 
> > > > > >  drivers/dax/dax-private.h |  4 
> > > > > >  drivers/dax/device.c  | 43 
> > > > > > ---
> > > > > >  2 files changed, 47 deletions(-)
> > > > >
> > > > > DanW: I think this series has reached enough review, did you want
> > > > > to ack/test any further?
> > > > >
> > > > > This needs to land in hmm.git soon to make the merge window.
> > > >
> > > > I was awaiting a decision about resolving the collision with Ira's
> > > > patch before testing the final result again [1]. You can go ahead and
> > > > add my reviewed-by for the series, but my tested-by should be on the
> > > > final state of the series.
> > >
> > > The conflict looks OK to me, I think we can let Andrew and Linus
> > > resolve it.
> >
> > Andrew's tree effectively always rebases since it's a quilt series.
> > I'd recommend pulling Ira's patch out of -mm and applying it with the
> > rest of hmm reworks. Any other git tree I'd agree with just doing the
> > late conflict resolution, but I'm not clear on what's the best
> > practice when conflicting with -mm.

What happens depends on timing as things arrive to Linus. I promised
to send hmm.git early, so I understand that Andrew will quilt rebase
his tree to Linus's and fix the conflict in Ira's patch before he
sends it.

> Regardless the patch is buggy. If you want to do the conflict
> resolution it should be because the DEVICE_PUBLIC removal effectively
> does the same fix otherwise we're knowingly leaving a broken point in
> the history.

I'm not sure I understand your concern, is there something wrong with
CH's series as it stands? hmm is a non-rebasing git tree, so as long
as the series is correct *when I apply it* there is no broken history.

I assumed the conflict resolution for Ira's patch was to simply take
the deletion of the if block from CH's series - right?

If we do need to take Ira's patch into hmm.git it will go after CH's
series (and Ira will have to rebase/repost it), so I think there is
nothing to do at this moment - unless you are saying there is a
problem with the series in CH's git tree?

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


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 10:08 AM Dan Williams  wrote:
>
> On Fri, Jun 28, 2019 at 10:02 AM Jason Gunthorpe  wrote:
> >
> > On Fri, Jun 28, 2019 at 09:27:44AM -0700, Dan Williams wrote:
> > > On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe  wrote:
> > > >
> > > > On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote:
> > > > > The functionality is identical to the one currently open coded in
> > > > > device-dax.
> > > > >
> > > > > Signed-off-by: Christoph Hellwig 
> > > > > Reviewed-by: Ira Weiny 
> > > > >  drivers/dax/dax-private.h |  4 
> > > > >  drivers/dax/device.c  | 43 
> > > > > ---
> > > > >  2 files changed, 47 deletions(-)
> > > >
> > > > DanW: I think this series has reached enough review, did you want
> > > > to ack/test any further?
> > > >
> > > > This needs to land in hmm.git soon to make the merge window.
> > >
> > > I was awaiting a decision about resolving the collision with Ira's
> > > patch before testing the final result again [1]. You can go ahead and
> > > add my reviewed-by for the series, but my tested-by should be on the
> > > final state of the series.
> >
> > The conflict looks OK to me, I think we can let Andrew and Linus
> > resolve it.
> >
>
> Andrew's tree effectively always rebases since it's a quilt series.
> I'd recommend pulling Ira's patch out of -mm and applying it with the
> rest of hmm reworks. Any other git tree I'd agree with just doing the
> late conflict resolution, but I'm not clear on what's the best
> practice when conflicting with -mm.

Regardless the patch is buggy. If you want to do the conflict
resolution it should be because the DEVICE_PUBLIC removal effectively
does the same fix otherwise we're knowingly leaving a broken point in
the history.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 10:02 AM Jason Gunthorpe  wrote:
>
> On Fri, Jun 28, 2019 at 09:27:44AM -0700, Dan Williams wrote:
> > On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe  wrote:
> > >
> > > On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote:
> > > > The functionality is identical to the one currently open coded in
> > > > device-dax.
> > > >
> > > > Signed-off-by: Christoph Hellwig 
> > > > Reviewed-by: Ira Weiny 
> > > >  drivers/dax/dax-private.h |  4 
> > > >  drivers/dax/device.c  | 43 ---
> > > >  2 files changed, 47 deletions(-)
> > >
> > > DanW: I think this series has reached enough review, did you want
> > > to ack/test any further?
> > >
> > > This needs to land in hmm.git soon to make the merge window.
> >
> > I was awaiting a decision about resolving the collision with Ira's
> > patch before testing the final result again [1]. You can go ahead and
> > add my reviewed-by for the series, but my tested-by should be on the
> > final state of the series.
>
> The conflict looks OK to me, I think we can let Andrew and Linus
> resolve it.
>

Andrew's tree effectively always rebases since it's a quilt series.
I'd recommend pulling Ira's patch out of -mm and applying it with the
rest of hmm reworks. Any other git tree I'd agree with just doing the
late conflict resolution, but I'm not clear on what's the best
practice when conflicting with -mm.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Jason Gunthorpe
On Fri, Jun 28, 2019 at 09:27:44AM -0700, Dan Williams wrote:
> On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe  wrote:
> >
> > On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote:
> > > The functionality is identical to the one currently open coded in
> > > device-dax.
> > >
> > > Signed-off-by: Christoph Hellwig 
> > > Reviewed-by: Ira Weiny 
> > >  drivers/dax/dax-private.h |  4 
> > >  drivers/dax/device.c  | 43 ---
> > >  2 files changed, 47 deletions(-)
> >
> > DanW: I think this series has reached enough review, did you want
> > to ack/test any further?
> >
> > This needs to land in hmm.git soon to make the merge window.
> 
> I was awaiting a decision about resolving the collision with Ira's
> patch before testing the final result again [1]. You can go ahead and
> add my reviewed-by for the series, but my tested-by should be on the
> final state of the series.

The conflict looks OK to me, I think we can let Andrew and Linus
resolve it.

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


Re: [PATCH] filesystem-dax: Disable PMD support

2019-06-28 Thread Matthew Wilcox
On Fri, Jun 28, 2019 at 09:39:01AM -0700, Dan Williams wrote:
> On Fri, Jun 28, 2019 at 9:37 AM Matthew Wilcox  wrote:
> > That was the conclusion I came to; that one thread holding the mmap sem
> > for read isn't being woken up when it should be.  Just need to find it ...
> > obviously it's something to do with the PMD entries.
> 
> Can you explain to me one more time, yes I'm slow on the uptake on
> this, the difference between xas_load() and xas_find_conflict() and
> why it's ok for dax_lock_page() to use xas_load() while
> grab_mapping_entry() uses xas_find_conflict()?

When used with a non-zero 'order', xas_find_conflict() will return
an entry whereas xas_load() might return a pointer to a node.
dax_lock_page() always uses a zero order, so they would always do the
same thing (xas_find_conflict() would be less efficient).
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH] filesystem-dax: Disable PMD support

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 9:37 AM Matthew Wilcox  wrote:
>
> On Thu, Jun 27, 2019 at 07:39:37PM -0700, Dan Williams wrote:
> > On Thu, Jun 27, 2019 at 12:59 PM Matthew Wilcox  wrote:
> > > On Thu, Jun 27, 2019 at 12:09:29PM -0700, Dan Williams wrote:
> > > > > This bug feels like we failed to unlock, or unlocked the wrong entry
> > > > > and this hunk in the bisected commit looks suspect to me. Why do we
> > > > > still need to drop the lock now that the radix_tree_preload() calls
> > > > > are gone?
> > > >
> > > > Nevermind, unmapp_mapping_pages() takes a sleeping lock, but then I
> > > > wonder why we don't restart the lookup like the old implementation.
> > >
> > > If something can remove a locked entry, then that would seem like the
> > > real bug.  Might be worth inserting a lookup there to make sure that it
> > > hasn't happened, I suppose?
> >
> > Nope, added a check, we do in fact get the same locked entry back
> > after dropping the lock.
>
> Okay, good, glad to have ruled that out.
>
> > The deadlock revolves around the mmap_sem. One thread holds it for
> > read and then gets stuck indefinitely in get_unlocked_entry(). Once
> > that happens another rocksdb thread tries to mmap and gets stuck
> > trying to take the mmap_sem for write. Then all new readers, including
> > ps and top that try to access a remote vma, then get queued behind
> > that write.
> >
> > It could also be the case that we're missing a wake up.
>
> That was the conclusion I came to; that one thread holding the mmap sem
> for read isn't being woken up when it should be.  Just need to find it ...
> obviously it's something to do with the PMD entries.

Can you explain to me one more time, yes I'm slow on the uptake on
this, the difference between xas_load() and xas_find_conflict() and
why it's ok for dax_lock_page() to use xas_load() while
grab_mapping_entry() uses xas_find_conflict()?
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH] filesystem-dax: Disable PMD support

2019-06-28 Thread Matthew Wilcox
On Thu, Jun 27, 2019 at 07:39:37PM -0700, Dan Williams wrote:
> On Thu, Jun 27, 2019 at 12:59 PM Matthew Wilcox  wrote:
> > On Thu, Jun 27, 2019 at 12:09:29PM -0700, Dan Williams wrote:
> > > > This bug feels like we failed to unlock, or unlocked the wrong entry
> > > > and this hunk in the bisected commit looks suspect to me. Why do we
> > > > still need to drop the lock now that the radix_tree_preload() calls
> > > > are gone?
> > >
> > > Nevermind, unmapp_mapping_pages() takes a sleeping lock, but then I
> > > wonder why we don't restart the lookup like the old implementation.
> >
> > If something can remove a locked entry, then that would seem like the
> > real bug.  Might be worth inserting a lookup there to make sure that it
> > hasn't happened, I suppose?
> 
> Nope, added a check, we do in fact get the same locked entry back
> after dropping the lock.

Okay, good, glad to have ruled that out.

> The deadlock revolves around the mmap_sem. One thread holds it for
> read and then gets stuck indefinitely in get_unlocked_entry(). Once
> that happens another rocksdb thread tries to mmap and gets stuck
> trying to take the mmap_sem for write. Then all new readers, including
> ps and top that try to access a remote vma, then get queued behind
> that write.
> 
> It could also be the case that we're missing a wake up.

That was the conclusion I came to; that one thread holding the mmap sem
for read isn't being woken up when it should be.  Just need to find it ...
obviously it's something to do with the PMD entries.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 30/43] docs: nvdimm: convert to ReST

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 5:21 AM Mauro Carvalho Chehab
 wrote:
>
> Rename the nvdimm documentation files to ReST, add an
> index for them and adjust in order to produce a nice html
> output via the Sphinx build system.
>
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
>
> Signed-off-by: Mauro Carvalho Chehab 

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


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Dan Williams
On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe  wrote:
>
> On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote:
> > The functionality is identical to the one currently open coded in
> > device-dax.
> >
> > Signed-off-by: Christoph Hellwig 
> > Reviewed-by: Ira Weiny 
> > ---
> >  drivers/dax/dax-private.h |  4 
> >  drivers/dax/device.c  | 43 ---
> >  2 files changed, 47 deletions(-)
>
> DanW: I think this series has reached enough review, did you want
> to ack/test any further?
>
> This needs to land in hmm.git soon to make the merge window.

I was awaiting a decision about resolving the collision with Ira's
patch before testing the final result again [1]. You can go ahead and
add my reviewed-by for the series, but my tested-by should be on the
final state of the series.

[1]: 
https://lore.kernel.org/lkml/CAPcyv4gTOf+EWzSGrFrh2GrTZt5HVR=e+xicukepiy57px8...@mail.gmail.com/
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH 16/25] device-dax: use the dev_pagemap internal refcount

2019-06-28 Thread Jason Gunthorpe
On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote:
> The functionality is identical to the one currently open coded in
> device-dax.
> 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Ira Weiny 
> ---
>  drivers/dax/dax-private.h |  4 
>  drivers/dax/device.c  | 43 ---
>  2 files changed, 47 deletions(-)

DanW: I think this series has reached enough review, did you want
to ack/test any further?

This needs to land in hmm.git soon to make the merge window.

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


[PATCH 04/39] docs: nvdimm: add it to the driver-api book

2019-06-28 Thread Mauro Carvalho Chehab
The descriptions here are from Kernel driver's PoV.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/driver-api/index.rst | 1 +
 Documentation/{ => driver-api}/nvdimm/btt.rst  | 0
 Documentation/{ => driver-api}/nvdimm/index.rst| 2 --
 Documentation/{ => driver-api}/nvdimm/nvdimm.rst   | 0
 Documentation/{ => driver-api}/nvdimm/security.rst | 0
 drivers/nvdimm/Kconfig | 2 +-
 6 files changed, 2 insertions(+), 3 deletions(-)
 rename Documentation/{ => driver-api}/nvdimm/btt.rst (100%)
 rename Documentation/{ => driver-api}/nvdimm/index.rst (94%)
 rename Documentation/{ => driver-api}/nvdimm/nvdimm.rst (100%)
 rename Documentation/{ => driver-api}/nvdimm/security.rst (100%)

diff --git a/Documentation/driver-api/index.rst 
b/Documentation/driver-api/index.rst
index 93c6c9a98c41..41f5ce7dc34c 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -43,6 +43,7 @@ available subsections can be seen below.
mtdnand
miscellaneous
mei/index
+   nvdimm/index
w1
rapidio/index
s390-drivers
diff --git a/Documentation/nvdimm/btt.rst 
b/Documentation/driver-api/nvdimm/btt.rst
similarity index 100%
rename from Documentation/nvdimm/btt.rst
rename to Documentation/driver-api/nvdimm/btt.rst
diff --git a/Documentation/nvdimm/index.rst 
b/Documentation/driver-api/nvdimm/index.rst
similarity index 94%
rename from Documentation/nvdimm/index.rst
rename to Documentation/driver-api/nvdimm/index.rst
index 1a3402d3775e..19dc8ee371dc 100644
--- a/Documentation/nvdimm/index.rst
+++ b/Documentation/driver-api/nvdimm/index.rst
@@ -1,5 +1,3 @@
-:orphan:
-
 ===
 Non-Volatile Memory Device (NVDIMM)
 ===
diff --git a/Documentation/nvdimm/nvdimm.rst 
b/Documentation/driver-api/nvdimm/nvdimm.rst
similarity index 100%
rename from Documentation/nvdimm/nvdimm.rst
rename to Documentation/driver-api/nvdimm/nvdimm.rst
diff --git a/Documentation/nvdimm/security.rst 
b/Documentation/driver-api/nvdimm/security.rst
similarity index 100%
rename from Documentation/nvdimm/security.rst
rename to Documentation/driver-api/nvdimm/security.rst
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index e89c1c332407..a5fde15e91d3 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -33,7 +33,7 @@ config BLK_DEV_PMEM
  Documentation/admin-guide/kernel-parameters.rst).  This driver 
converts
  these persistent memory ranges into block devices that are
  capable of DAX (direct-access) file system mappings.  See
- Documentation/nvdimm/nvdimm.rst for more details.
+ Documentation/driver-api/nvdimm/nvdimm.rst for more details.
 
  Say Y if you want to use an NVDIMM
 
-- 
2.21.0

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


[PATCH 30/43] docs: nvdimm: convert to ReST

2019-06-28 Thread Mauro Carvalho Chehab
Rename the nvdimm documentation files to ReST, add an
index for them and adjust in order to produce a nice html
output via the Sphinx build system.

At its new index.rst, let's add a :orphan: while this is not linked to
the main index.rst file, in order to avoid build warnings.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/nvdimm/{btt.txt => btt.rst} | 140 ++---
 Documentation/nvdimm/index.rst|  12 +
 .../nvdimm/{nvdimm.txt => nvdimm.rst} | 518 ++
 .../nvdimm/{security.txt => security.rst} |   4 +-
 drivers/nvdimm/Kconfig|   2 +-
 5 files changed, 387 insertions(+), 289 deletions(-)
 rename Documentation/nvdimm/{btt.txt => btt.rst} (71%)
 create mode 100644 Documentation/nvdimm/index.rst
 rename Documentation/nvdimm/{nvdimm.txt => nvdimm.rst} (60%)
 rename Documentation/nvdimm/{security.txt => security.rst} (99%)

diff --git a/Documentation/nvdimm/btt.txt b/Documentation/nvdimm/btt.rst
similarity index 71%
rename from Documentation/nvdimm/btt.txt
rename to Documentation/nvdimm/btt.rst
index e293fb664924..2d8269f834bd 100644
--- a/Documentation/nvdimm/btt.txt
+++ b/Documentation/nvdimm/btt.rst
@@ -1,9 +1,10 @@
+=
 BTT - Block Translation Table
 =
 
 
 1. Introduction

+===
 
 Persistent memory based storage is able to perform IO at byte (or more
 accurately, cache line) granularity. However, we often want to expose such
@@ -25,7 +26,7 @@ provides atomic sector updates.
 
 
 2. Static Layout
-
+
 
 The underlying storage on which a BTT can be laid out is not limited in any 
way.
 The BTT, however, splits the available space into chunks of up to 512 GiB,
@@ -33,43 +34,43 @@ called "Arenas".
 
 Each arena follows the same layout for its metadata, and all references in an
 arena are internal to it (with the exception of one field that points to the
-next arena). The following depicts the "On-disk" metadata layout:
+next arena). The following depicts the "On-disk" metadata layout::
 
 
-  Backing Store +--->  Arena
-+---+   |   +--+
-|   |   |   | Arena info block |
-|Arena 0+---+   |   4K |
-| 512G  |   +--+
-|   |   |  |
-+---+   |  |
-|   |   |  |
-|Arena 1|   |   Data Blocks|
-| 512G  |   |  |
-|   |   |  |
-+---+   |  |
-|   .   |   |  |
-|   .   |   |  |
-|   .   |   |  |
-|   |   |  |
-|   |   |  |
-+---+   +--+
-|  |
-| BTT Map  |
-|  |
-|  |
-+--+
-|  |
-| BTT Flog |
-|  |
-+--+
-| Info block copy  |
-|   4K |
-+--+
+Backing Store +--->  Arena
+  +---+   |   +--+
+  |   |   |   | Arena info block |
+  |Arena 0+---+   |   4K |
+  | 512G  |   +--+
+  |   |   |  |
+  +---+   |  |
+  |   |   |  |
+  |Arena 1|   |   Data Blocks|
+  | 512G  |   |  |
+  |   |   |  |
+  +---+   |  |
+  |   .   |   |  |
+  |   .   |   |  |
+  |   .   |   |  |
+  |   |   |  |
+  |   |   |  |
+  +---+   +--+
+  |  |
+  | BTT Map  |
+  |  |
+  |  |
+  +--+
+  |  |
+  | BTT Flog |
+  |  |
+  +--+
+  | Info block copy  |
+  |   4K |
+  +--+
 
 
 3. Theory of Operation
---
+==
 
 
 

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

2019-06-28 Thread Brendan Higgins
On Thu, Jun 27, 2019 at 11:16 AM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-06-26 16:00:40)
> > On Tue, Jun 25, 2019 at 8:41 PM Stephen Boyd  wrote:
> >
> > > scenario like below, but where it is a problem. There could be three
> > > CPUs, or even one CPU and three threads if you want to describe the
> > > extra thread scenario.
> > >
> > > Here's my scenario where it isn't needed:
> > >
> > > CPU0  CPU1
> > >   
> > > kunit_run_test()
> > >   test_case_func()
> > > 
> > >   [mock hardirq]
> > > kunit_set_success()
> > >   [hardirq ends]
> > > ...
> > > complete(_done)
> > >   wait_for_completion(_done)
> > >   kunit_get_success()
> > >
> > > We don't need to care about having locking here because success or
> > > failure only happens in one place and it's synchronized with the
> > > completion.
> >
> > Here is the scenario I am concerned about:
> >
> > CPU0  CPU1   CPU2
> >      
> > kunit_run_test()
> >   test_case_func()
> > 
> > schedule_work(foo_func)
> >   [mock hardirq] foo_func()
> > ......
> > kunit_set_success(false)   
> > kunit_set_success(false)
> >   [hardirq ends]   ...
> > ...
> > complete(_done)
> >   wait_for_completion(...)
> >   kunit_get_success()
> >
> > In my scenario, since both CPU1 and CPU2 update the success status of
> > the test simultaneously, even though they are setting it to the same
> > value. If my understanding is correct, this could result in a
> > write-tear on some architectures in some circumstances. I suppose we
> > could just make it an atomic boolean, but I figured locking is also
> > fine, and generally preferred.
>
> This is what we have WRITE_ONCE() and READ_ONCE() for. Maybe you could
> just use that in the getter and setters and remove the lock if it isn't
> used for anything else.
>
> It may also be a good idea to have a kunit_fail_test() API that fails
> the test passed in with a WRITE_ONCE(false). Otherwise, the test is
> assumed successful and it isn't even possible for a test to change the
> state from failure to success due to a logical error because the API
> isn't available. Then we don't really need to have a generic
> kunit_set_success() function at all. We could have a kunit_test_failed()
> function too that replaces the kunit_get_success() function. That would
> read better in an if condition.

You know what, I think you are right.

Sorry, for not realizing this earlier, I think you mentioned something
along these lines a long time ago.

Thanks for your patience!

> >
> > Also, to be clear, I am onboard with dropping then IRQ stuff for now.
> > I am fine moving to a mutex for the time being.
> >
>
> Ok.

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


Re: [PATCH v5 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()

2019-06-28 Thread Brendan Higgins
On Wed, Jun 26, 2019 at 11:10 PM Luis Chamberlain  wrote:
>
> On Wed, Jun 26, 2019 at 09:07:43PM -0700, Iurii Zaikin wrote:
> > On Tue, Jun 25, 2019 at 7:17 PM Luis Chamberlain  wrote:
> > > > +static void sysctl_test_dointvec_table_maxlen_unset(struct kunit *test)
> > > > +{
> > > > + struct ctl_table table = {
> > > > + .procname = "foo",
> > > > + .data   = _data.int_0001,
> > > > + .maxlen = 0,
> > > > + .mode   = 0644,
> > > > + .proc_handler   = proc_dointvec,
> > > > + .extra1 = _zero,
> > > > + .extra2 = _one_hundred,
> > > > + };
> > > > + void  *buffer = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > + size_t len;
> > > > + loff_t pos;
> > > > +
> > > > + len = 1234;
> > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 0, buffer, , 
> > > > ));
> > > > + KUNIT_EXPECT_EQ(test, (size_t)0, len);
> > > > + len = 1234;
> > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 1, buffer, , 
> > > > ));
> > > > + KUNIT_EXPECT_EQ(test, (size_t)0, len);
> > > > +}
> > >
> > > In a way this is also testing for general kernel API changes. This is and 
> > > the
> > > last one were good examples. So this is not just testing functionality
> > > here. There is no wrong or write answer if 0 or -EINVAL was returned
> > > other than the fact that we have been doing this for years.
> > >
> > > Its a perhaps small but important difference for some of these tests.  I
> > > *do* think its worth clarifying through documentation which ones are
> > > testing for API consistency Vs proper correctness.
> >
> > You make a good point that the test codifies the existing behavior of
> > the function in lieu of formal documentation.  However, the test cases
> > were derived from examining the source code of the function under test
> > and attempting to cover all branches. The assertions were added only
> > for the values that appeared to be set deliberately in the
> > implementation. And it makes sense to me to test that the code does
> > exactly what the implementation author intended.
>
> I'm not arguing against adding them. I'm suggesting that it is different
> to test for API than for correctness of intended functionality, and
> it would be wise to make it clear which test cases are for API and which
> for correctness.

I see later on that some of the API stuff you are talking about is
public APIs from the standpoint of user (outside of LInux) visible. To
be clear, is that what you mean by public APIs throughout, or would
you distinguish between correctness tests, internal API tests, and
external API tests?

> This will come up later for other kunit tests and it would be great
> to set precendent so that other kunit tests can follow similar
> practices to ensure its clear what is API realted Vs correctness of
> intended functionality.
>
> In fact, I'm not yet sure if its possible to test public kernel API to
> userspace with kunit, but if it is possible... well, that could make
> linux-api folks happy as they could enable us to codify interpreation of
> what is expected into kunit test cases, and we'd ensure that the
> codified interpretation is not only documented in man pages but also
> through formal kunit test cases.
>
> A regression in linux-api then could be formalized through a proper
> kunit tests case. And if an API evolves, it would force developers to
> update the respective kunit which codifies that contract.

Yep, I think that is long term hope. Some of the file system interface
stuff that requires a filesystem to be mounted somewhere might get a
little weird/difficult, but I suspect we should be able to do it
eventually. I mean it's all just C code right? Should mostly boil down
to someone figuring out how to do it the first time.

> > > > +static void sysctl_test_dointvec_single_less_int_min(struct kunit 
> > > > *test)
> > > > +{
> > > > + struct ctl_table table = {
> > > > + .procname = "foo",
> > > > + .data   = _data.int_0001,
> > > > + .maxlen = sizeof(int),
> > > > + .mode   = 0644,
> > > > + .proc_handler   = proc_dointvec,
> > > > + .extra1 = _zero,
> > > > + .extra2 = _one_hundred,
> > > > + };
> > > > + char input[32];
> > > > + size_t len = sizeof(input) - 1;
> > > > + loff_t pos = 0;
> > > > + unsigned long abs_of_less_than_min = (unsigned long)INT_MAX
> > > > +  - (INT_MAX + INT_MIN) + 1;
> > > > +
> > > > + KUNIT_EXPECT_LT(test,
> > > > + (size_t)snprintf(input, sizeof(input), "-%lu",
> > > > +  abs_of_less_than_min),
> > > > + sizeof(input));
> > > > +
> > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > + KUNIT_EXPECT_EQ(test, -EINVAL,
> 

[ANNOUNCE] Alpine Linux Persistence and Storage Summit

2019-06-28 Thread Christoph Hellwig
We proudly announce the second Alpine Linux Persistence and Storage
Summit (ALPSS), which will be held October 1st to October 4th 2019 at
the Lizumerhuette [1][2] in Austria .

The goal of this conference is to discuss the hot topics in Linux storage
and file systems, such as persistent memory, NVMe, Zoned Block Device,
and PCIe peer to peer transfers in a cool and relaxed setting with
spectacular views in the Austrian alps.

We plan to have a small selection of short and to the point talks with
lots of room for discussion in small groups, as well as ample downtime
to enjoy the surrounding.

Attendance is free except for the accommodation and food at the lodge [3],
but the number of seats is strictly limited.  If you are interested in
attending please reserve a seat by mailing your favorite topic(s) to:

alpss...@lists.infradead.org

If you are interested in giving a short and crisp talk please also send
an abstract to the same address.

Note: The Lizumerhuette is an Alpine Society lodge in a high alpine
environment.  A hike of approximately 2 hours is required to the lodge,
and no other accommodations are available within walking distance.

More details will eventually be available on our website:

http://www.alpss.at/

Thank you on behalf of the program committee:

Stephen Bates
Sagi Grimberg
Christoph Hellwig
Johannes Thumshirn
Richard Weinberger

[1] http://www.tyrol.com/things-to-do/sports/hiking/refuge-huts/a-lizumer-hut
[2] https://www.glungezer.at/l-i-z-u-m-e-r-h-%C3%BC-t-t-e/
[3] approx. EUR 40-60 per person and night, depending on the room
category
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm