Re: [PATCH RFC 7/9] mm/gup: Decrement head page once for group of
subpages
by John Hubbard
On 12/17/20 12:05 PM, Jason Gunthorpe wrote:
> On Thu, Dec 17, 2020 at 07:05:37PM +0000, Joao Martins wrote:
>>> No reason not to fix set_page_dirty_lock() too while you are here.
>>
>> The wack of atomics you mentioned earlier you referred to, I suppose it
>> ends being account_page_dirtied(). See partial diff at the end.
>
> Well, even just eliminating the lock_page, page_mapping, PageDirty,
> etc is already a big win.
>
> If mapping->a_ops->set_page_dirty() needs to be called multiple times
> on the head page I'd probably just suggest:
>
> while (ntails--)
> rc |= (*spd)(head);
I think once should be enough. There is no counter for page dirtiness,
and this kind of accounting is always tracked in the head page, so there
is no reason to repeatedly call set_page_dirty() from the same
spot.
thanks,
--
John Hubbard
NVIDIA
5 months, 3 weeks
[ANNOUNCE] ndctl v71
by Verma, Vishal L
A new ndctl release is available[1].
Highlights include support for the new device-dax subdivision
functionality added in Linux in v5.10, including ways to create smaller
devdax devices using daxctl/libdaxctl, as well as creating, listing, and
restoring from a config dump, 'mappings' on these devices. Other updates
include several static analysis fixups, reworking the license
identification scheme for different sub-components, and a fix for the
reconfigure-in-place workflow which tries to retain device names.
A shortlog is appended below.
[1]: https://github.com/pmem/ndctl/releases/tag/v71
Aneesh Kumar K.V (1):
daxctl: phys_index value 0 is valid
Dan Williams (6):
build: Use asciidoc instead of asciidoctor on RHEL
ndctl/namespace: Catch attempts to sub-divide legacy / label-less capacity
Clarify COPYING
daxctl: Cleanup whitespace
Rework license identification
ndctl/namespace: Reconfigure in-place
Joao Martins (20):
libdaxctl: add daxctl_dev_set_size()
daxctl: add resize support in reconfigure-device
daxctl: add command to disable devdax device
daxctl: add command to enable devdax device
libdaxctl: add daxctl_region_create_dev()
daxctl: add command to create device
libdaxctl: add daxctl_region_destroy_dev()
daxctl: add command to destroy device
daxctl/test: Add tests for dynamic dax regions
test/daxctl-create.sh: Validate @size versus mappingX sizes
daxctl: add daxctl_dev_{get,set}_align()
util/json: Print device align
daxctl: add align support in reconfigure-device
daxctl: add align support in create-device
daxctl/test: Add a test for daxctl-create with align
libdaxctl: add mapping iterator APIs
daxctl: include mappings when listing
libdaxctl: add daxctl_dev_set_mapping()
daxctl: allow creating devices from input json
daxctl/test: add a test for daxctl-create with input file
Vishal Verma (3):
Documentation/daxctl: use option includes in reconfigure-device
daxctl/device: fix a memory leak in create-device
ndctl.spec.in: update for license reworks
Zhiqiang Liu (8):
namespace: check whether pfn|dax|btt is NULL in setup_namespace
lib/libndctl: fix memory leakage problem in add_bus
libdaxctl: fix memory leakage in add_dax_region()
dimm: fix potential fd leakage in dimm_action()
util/help: check whether strdup returns NULL in exec_man_konqueror
lib/inject: check whether cmd is created successfully
Check whether ndctl_btt_get_namespace returns NULL in callers
Check whether seed is NULL in validate_namespace_options
5 months, 3 weeks
[ndctl PATCH] ndctl.spec.in: update for license reworks
by Vishal Verma
Commit 14eacf0d694a ("Rework license identification") reworked license
identification, making the files listed in ndctl.spec.in for %license
obsolete. This updates those sections in the RPM spec to match the
current layout of license files.
Fixes: 14eacf0d694a ("Rework license identification")
Cc: Dan Williams <dan.j.williams(a)intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma(a)intel.com>
---
ndctl.spec.in | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/ndctl.spec.in b/ndctl.spec.in
index 056c530..0563b2d 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -109,7 +109,7 @@ make check
%files
%defattr(-,root,root)
-%license util/COPYING licenses/BSD-MIT licenses/CC0
+%license LICENSES/preferred/GPL-2.0 LICENSES/other/MIT LICENSES/other/CC0-1.0
%{_bindir}/ndctl
%{_mandir}/man1/ndctl*
%{bashcompdir}/
@@ -121,7 +121,7 @@ make check
%files -n daxctl
%defattr(-,root,root)
-%license util/COPYING licenses/BSD-MIT licenses/CC0
+%license LICENSES/preferred/GPL-2.0 LICENSES/other/MIT LICENSES/other/CC0-1.0
%{_bindir}/daxctl
%{_mandir}/man1/daxctl*
%{_datadir}/daxctl/daxctl.conf
@@ -129,25 +129,25 @@ make check
%files -n LNAME
%defattr(-,root,root)
%doc README.md
-%license COPYING licenses/BSD-MIT licenses/CC0
+%license LICENSES/preferred/LGPL-2.1 LICENSES/other/MIT LICENSES/other/CC0-1.0
%{_libdir}/libndctl.so.*
%files -n DAX_LNAME
%defattr(-,root,root)
%doc README.md
-%license COPYING licenses/BSD-MIT licenses/CC0
+%license LICENSES/preferred/LGPL-2.1 LICENSES/other/MIT LICENSES/other/CC0-1.0
%{_libdir}/libdaxctl.so.*
%files -n DNAME
%defattr(-,root,root)
-%license COPYING
+%license LICENSES/preferred/LGPL-2.1
%{_includedir}/ndctl/
%{_libdir}/libndctl.so
%{_libdir}/pkgconfig/libndctl.pc
%files -n DAX_DNAME
%defattr(-,root,root)
-%license COPYING
+%license LICENSES/preferred/LGPL-2.1
%{_includedir}/daxctl/
%{_libdir}/libdaxctl.so
%{_libdir}/pkgconfig/libdaxctl.pc
--
2.26.2
5 months, 3 weeks
[PATCH daxctl v2 0/5] daxctl: range mapping allocation
by Joao Martins
Hey,
This series adds support for:
1) Listing mappings when passing -M to ´daxctl list´. These are ommited
by default.
2) Iteration APIs for the mappings.
3) Allow passing an input JSON file with the manually selected ranges
to be used when creating the device-dax instance.
This applies on top of 'jm/devdax_subdiv' branch in github.com:pmem/ndctl.git
Testing requires a 5.10+ kernel.
v1 -> v2:
* List mappings only with -M|--mappings option
* Adds a unit test for --input file (while testing with -M listing too)
* Rename --restore to --input
* Add Documentation for -M and for --input
Joao Martins (5):
libdaxctl: add mapping iterator APIs
daxctl: include mappings when listing
libdaxctl: add daxctl_dev_set_mapping()
daxctl: allow creating devices from input json
daxctl/test: add a test for daxctl-create with input file
Documentation/daxctl/daxctl-create-device.txt | 13 +++
Documentation/daxctl/daxctl-list.txt | 4 +
daxctl/device.c | 128 +++++++++++++++++++++++++-
daxctl/lib/libdaxctl-private.h | 8 ++
daxctl/lib/libdaxctl.c | 118 +++++++++++++++++++++++-
daxctl/lib/libdaxctl.sym | 7 ++
daxctl/libdaxctl.h | 14 +++
daxctl/list.c | 4 +
test/daxctl-create.sh | 31 ++++++-
util/json.c | 57 +++++++++++-
util/json.h | 4 +
11 files changed, 380 insertions(+), 8 deletions(-)
--
1.8.3.1
5 months, 3 weeks
[PATCH v2 0/1] device-dax: avoid an unnecessary check in alloc_dev_dax_range()
by Zhen Lei
v1 --> v2:
In v1, I use the "goto" statement to merge two identical __release_region() calls.
However, the new patch https://lkml.org/lkml/2020/12/18/735 deletes one of them, the
"goto" becomes worthless. So when krealloc() failed, directly call __release_region()
and return error code.
Zhen Lei (1):
device-dax: avoid an unnecessary check in alloc_dev_dax_range()
drivers/dax/bus.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
--
2.26.0.106.g9fadedd
5 months, 3 weeks
[ndctl PATCH] daxctl/device: fix a memory leak in create-device
by Vishal Verma
Static analysis reports that we neglected to free 'maps' which is used
to hold mappings parsed out of an input file. Free this at the end of
do_xaction_region so we don't leak any memory. Additionally, using local
variables for the calloc cause a scope warning. Fix this by just
allocating into the 'maps' variable directly. Give 'len' / 'nmaps' the
same treatment.
Fixes: 5bedb2a8197a ("daxctl: allow creating devices from input json")
Cc: Joao Martins <joao.m.martins(a)oracle.com>
Signed-off-by: Vishal Verma <vishal.l.verma(a)intel.com>
---
daxctl/device.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/daxctl/device.c b/daxctl/device.c
index 161025e..0721a57 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -157,10 +157,9 @@ static int sort_mappings(const void *a, const void *b)
static int parse_device_file(const char *filename)
{
struct json_object *jobj, *jval = NULL, *jmappings = NULL;
- int i, len, rc = -EINVAL, region_id, id;
+ int i, rc = -EINVAL, region_id, id;
const char *chardev;
char *region = NULL;
- struct mapping *m;
jobj = json_object_from_file(filename);
if (!jobj)
@@ -187,12 +186,12 @@ static int parse_device_file(const char *filename)
return rc;
json_object_array_sort(jmappings, sort_mappings);
- len = json_object_array_length(jmappings);
- m = calloc(len, sizeof(*m));
- if (!m)
+ nmaps = json_object_array_length(jmappings);
+ maps = calloc(nmaps, sizeof(*maps));
+ if (!maps)
return -ENOMEM;
- for (i = 0; i < len; i++) {
+ for (i = 0; i < nmaps; i++) {
struct json_object *j, *val;
j = json_object_array_get_idx(jmappings, i);
@@ -201,23 +200,21 @@ static int parse_device_file(const char *filename)
if (!json_object_object_get_ex(j, "start", &val))
goto err;
- m[i].start = json_object_get_int64(val);
+ maps[i].start = json_object_get_int64(val);
if (!json_object_object_get_ex(j, "end", &val))
goto err;
- m[i].end = json_object_get_int64(val);
+ maps[i].end = json_object_get_int64(val);
if (!json_object_object_get_ex(j, "page_offset", &val))
goto err;
- m[i].pgoff = json_object_get_int64(val);
+ maps[i].pgoff = json_object_get_int64(val);
}
- maps = m;
- nmaps = len;
- rc = 0;
+
+ return 0;
err:
- if (!maps)
- free(m);
+ free(maps);
return rc;
}
@@ -817,6 +814,7 @@ static int do_xaction_region(enum device_action action,
break;
}
}
+ free(maps);
/*
* jdevs is the containing json array for all devices we are reporting
--
2.26.2
5 months, 3 weeks
[PATCH 1/1] device-dax: avoid an unnecessary check in alloc_dev_dax_range()
by Zhen Lei
Swap the calling sequence of krealloc() and __request_region(), call the
latter first. In this way, the value of dev_dax->nr_range does not need to
be considered when __request_region() failed.
Signed-off-by: Zhen Lei <thunder.leizhen(a)huawei.com>
---
drivers/dax/bus.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index 27513d311242..1efae11d947a 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -763,23 +763,15 @@ static int alloc_dev_dax_range(struct dev_dax *dev_dax, u64 start,
return 0;
}
- ranges = krealloc(dev_dax->ranges, sizeof(*ranges)
- * (dev_dax->nr_range + 1), GFP_KERNEL);
- if (!ranges)
- return -ENOMEM;
-
alloc = __request_region(res, start, size, dev_name(dev), 0);
- if (!alloc) {
- /*
- * If this was an empty set of ranges nothing else
- * will release @ranges, so do it now.
- */
- if (!dev_dax->nr_range) {
- kfree(ranges);
- ranges = NULL;
- }
- dev_dax->ranges = ranges;
+ if (!alloc)
return -ENOMEM;
+
+ ranges = krealloc(dev_dax->ranges, sizeof(*ranges)
+ * (dev_dax->nr_range + 1), GFP_KERNEL);
+ if (!ranges) {
+ rc = -ENOMEM;
+ goto err;
}
for (i = 0; i < dev_dax->nr_range; i++)
@@ -808,11 +800,14 @@ static int alloc_dev_dax_range(struct dev_dax *dev_dax, u64 start,
dev_dbg(dev, "delete range[%d]: %pa:%pa\n", dev_dax->nr_range - 1,
&alloc->start, &alloc->end);
dev_dax->nr_range--;
- __release_region(res, alloc->start, resource_size(alloc));
- return rc;
+ goto err;
}
return 0;
+
+err:
+ __release_region(res, alloc->start, resource_size(alloc));
+ return rc;
}
static int adjust_dev_dax_range(struct dev_dax *dev_dax, struct resource *res, resource_size_t size)
--
2.26.0.106.g9fadedd
5 months, 4 weeks
Re: [PATCH RFC 7/9] mm/gup: Decrement head page once for group of
subpages
by Joao Martins
On 12/17/20 8:05 PM, Jason Gunthorpe wrote:
> On Thu, Dec 17, 2020 at 07:05:37PM +0000, Joao Martins wrote:
>>> No reason not to fix set_page_dirty_lock() too while you are here.
>>
>> The wack of atomics you mentioned earlier you referred to, I suppose it
>> ends being account_page_dirtied(). See partial diff at the end.
>
> Well, even just eliminating the lock_page, page_mapping, PageDirty,
> etc is already a big win.
>
> If mapping->a_ops->set_page_dirty() needs to be called multiple times
> on the head page I'd probably just suggest:
>
> while (ntails--)
> rc |= (*spd)(head);
>
> At least as a start.
>
/me nods
> If you have workloads that have page_mapping != NULL then look at
> another series to optimze that. Looks a bit large though given the
> number of places implementing set_page_dirty
>
Yes. I don't have a particular workload, was just wondering what you had in
mind, as at a glance, changing all the places without messing filesystems looks like
the subject of a separate series.
> I think the current reality is calling set_page_dirty on an actual
> file system is busted anyhow, so I think mapping is generally going to
> be NULL here?
Perhaps -- I'll have to check.
Joao
5 months, 4 weeks
[PATCH 1/1] device-dax: delete a redundancy check in dev_dax_validate_align()
by Zhen Lei
After we have done the alignment check for the length of each range, the
alignment check for dev_dax_size(dev_dax) is no longer needed, because it
get the sum of the length of each range.
Signed-off-by: Zhen Lei <thunder.leizhen(a)huawei.com>
---
drivers/dax/bus.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index 1efae11d947a..35f9238c0139 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -1109,16 +1109,9 @@ static ssize_t align_show(struct device *dev,
static ssize_t dev_dax_validate_align(struct dev_dax *dev_dax)
{
- resource_size_t dev_size = dev_dax_size(dev_dax);
struct device *dev = &dev_dax->dev;
int i;
- if (dev_size > 0 && !alloc_is_aligned(dev_dax, dev_size)) {
- dev_dbg(dev, "%s: align %u invalid for size %pa\n",
- __func__, dev_dax->align, &dev_size);
- return -EINVAL;
- }
-
for (i = 0; i < dev_dax->nr_range; i++) {
size_t len = range_len(&dev_dax->ranges[i].range);
--
2.26.0.106.g9fadedd
5 months, 4 weeks
[PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation
by Joao Martins
Hey,
This series builds on top of this one[0] and does the following improvements
to the Soft-Reserved subdivision:
1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
2) Listing improvements for device alignment and mappings;
Note: Perhaps it is better to hide the mappings by default, and only
print with -v|--verbose. This would align with ndctl, as the mappings
info can be quite large.
3) Allow creating devices from selecting ranges. This allows to keep the
same GPA->HPA mapping as before we kexec the hypervisor with running guests:
daxctl list -d dax0.1 > /var/log/dax0.1.json
kexec -d -l bzImage
systemctl kexec
daxctl create -u --restore /var/log/dax0.1.json
The JSON was what I though it would be easier for an user, given that it is
the data format daxctl outputs. Alternatives could be adding multiple:
--mapping <pgoff>:<start>-<end>
But that could end up in a gigantic line and a little more
unmanageable I think.
This series requires this series[0] on top of Dan's patches[1]:
[0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martin...
[1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147...
The only TODO here is docs and improving tests to validate mappings, and test
the restore path.
Suggestions/comments are welcome.
Joao
Joao Martins (8):
daxctl: add daxctl_dev_{get,set}_align()
util/json: Print device align
daxctl: add align support in reconfigure-device
daxctl: add align support in create-device
libdaxctl: add mapping iterator APIs
daxctl: include mappings when listing
libdaxctl: add daxctl_dev_set_mapping()
daxctl: Allow restore devices from JSON metadata
daxctl/device.c | 154 +++++++++++++++++++++++++++++++++++++++--
daxctl/lib/libdaxctl-private.h | 9 +++
daxctl/lib/libdaxctl.c | 152 +++++++++++++++++++++++++++++++++++++++-
daxctl/lib/libdaxctl.sym | 9 +++
daxctl/libdaxctl.h | 16 +++++
util/json.c | 63 ++++++++++++++++-
util/json.h | 3 +
7 files changed, 396 insertions(+), 10 deletions(-)
--
1.8.3.1
5 months, 4 weeks
