| Message ID | 159643094279.4062302.17779410714418721328.stgit@dwillia2-desk3.amr.corp.intel.com |
|---|---|
| Headers | show |
| Series | device-dax: Support sub-dividing soft-reserved ranges | expand |
On 22.08.20 01:21, Andrew Morton wrote: > On Wed, 19 Aug 2020 18:53:57 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > >>> I think I am missing some important pieces. Bear with me. >> >> No worries, also bear with me, I'm going to be offline intermittently >> until at least mid-September. Hopefully Joao and/or Vishal can jump in >> on this discussion. > > Ordinarily I'd prefer a refresh&resend for 2+ week-old series such as > this. > > But given that v4 all applies OK and that Dan has pending outages, I'll > scoop up this version, even though at least one change has been suggested. > Should I try to fix patch #11 while Dan is away? Because I think at least two things in there are wrong (and it would have been better to split that patch into reviewable pieces).
[Sorry for the late response] On 8/21/20 11:06 AM, David Hildenbrand wrote: > On 03.08.20 07:03, Dan Williams wrote: >> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev) >> * could be mixed in a node with faster memory, causing >> * unavoidable performance issues. >> */ >> - numa_node = dev_dax->target_node; >> if (numa_node < 0) { >> dev_warn(dev, "rejecting DAX region with invalid node: %d\n", >> numa_node); >> return -EINVAL; >> } >> >> - /* Hotplug starting at the beginning of the next block: */ >> - kmem_start = ALIGN(range->start, memory_block_size_bytes()); >> - >> - kmem_size = range_len(range); >> - /* Adjust the size down to compensate for moving up kmem_start: */ >> - kmem_size -= kmem_start - range->start; >> - /* Align the size down to cover only complete blocks: */ >> - kmem_size &= ~(memory_block_size_bytes() - 1); >> - kmem_end = kmem_start + kmem_size; >> - >> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL); >> - if (!new_res_name) >> + res_name = kstrdup(dev_name(dev), GFP_KERNEL); >> + if (!res_name) >> return -ENOMEM; >> >> - /* Region is permanently reserved if hotremove fails. */ >> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name); >> - if (!new_res) { >> - dev_warn(dev, "could not reserve region [%pa-%pa]\n", >> - &kmem_start, &kmem_end); >> - kfree(new_res_name); >> + res = request_mem_region(range.start, range_len(&range), res_name); > > I think our range could be empty after aligning. I assume > request_mem_region() would check that, but maybe we could report a > better error/warning in that case. > dax_kmem_range() already returns a memory-block-aligned @range but IIUC request_mem_region() isn't checking for that. Having said that the returned @res wouldn't be different from the passed range.start. >> /* >> * Ensure that future kexec'd kernels will not treat this as RAM >> * automatically. >> */ >> - rc = add_memory_driver_managed(numa_node, new_res->start, >> - resource_size(new_res), kmem_name); >> + rc = add_memory_driver_managed(numa_node, res->start, >> + resource_size(res), kmem_name); >> + >> + res->flags |= IORESOURCE_BUSY; > > Hm, I don't think that's correct. Any specific reason why to mark the > not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could > suddenly stumble over it - and e.g., similarly kexec code when trying to > find memory for placing kexec images. I think we should leave this > !BUSY, just as it is right now. > Agreed. >> if (rc) { >> - release_resource(new_res); >> - kfree(new_res); >> - kfree(new_res_name); >> + release_mem_region(range.start, range_len(&range)); >> + kfree(res_name); >> return rc; >> } >> - dev_dax->dax_kmem_res = new_res; >> + >> + dev_set_drvdata(dev, res_name); >> >> return 0; >> } >> >> #ifdef CONFIG_MEMORY_HOTREMOVE >> -static int dev_dax_kmem_remove(struct device *dev) >> +static void dax_kmem_release(struct dev_dax *dev_dax) >> { >> - struct dev_dax *dev_dax = to_dev_dax(dev); >> - struct resource *res = dev_dax->dax_kmem_res; >> - resource_size_t kmem_start = res->start; >> - resource_size_t kmem_size = resource_size(res); >> - const char *res_name = res->name; >> int rc; >> + struct device *dev = &dev_dax->dev; >> + const char *res_name = dev_get_drvdata(dev); >> + struct range range = dax_kmem_range(dev_dax); >> >> /* >> * We have one shot for removing memory, if some memory blocks were not >> * offline prior to calling this function remove_memory() will fail, and >> * there is no way to hotremove this memory until reboot because device >> - * unbind will succeed even if we return failure. >> + * unbind will proceed regardless of the remove_memory result. >> */ >> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size); >> - if (rc) { >> - any_hotremove_failed = true; >> - dev_err(dev, >> - "DAX region %pR cannot be hotremoved until the next reboot\n", >> - res); >> - return rc; >> + rc = remove_memory(dev_dax->target_node, range.start, range_len(&range)); >> + if (rc == 0) { > > if (!rc) ? > Better off would be to keep the old order: if (rc) { any_hotremove_failed = true; dev_err(dev, "%#llx-%#llx cannot be hotremoved until the next reboot\n", range.start, range.end); return; } release_mem_region(range.start, range_len(&range)); dev_set_drvdata(dev, NULL); kfree(res_name); return; >> + release_mem_region(range.start, range_len(&range)); > > remove_memory() does a release_mem_region_adjustable(). Don't you > actually want to release the *unaligned* region you requested? > Isn't it what we're doing here? (The release_mem_region_adjustable() is using the same dax_kmem-aligned range and there's no split/adjust) Meaning right now (+ parent marked as !BUSY), and if I am understanding this correctly: request_mem_region(range.start, range_len) __request_region(iomem_res, range.start, range_len) -> alloc @parent add_memory_driver_managed(parent.start, resource_size(parent)) __request_region(parent.start, resource_size(parent)) -> alloc @child [...] remove_memory(range.start, range_len) request_mem_region_adjustable(range.start, range_len) __release_region(range.start, range_len) -> remove @child release_mem_region(range.start, range_len) __release_region(range.start, range_len) -> doesn't remove @parent because !BUSY? The add/removal of this relies on !BUSY. But now I am wondering if the parent remaining unreleased is deliberate even on CONFIG_MEMORY_HOTREMOVE=y. Joao
>>> + release_mem_region(range.start, range_len(&range)); >> >> remove_memory() does a release_mem_region_adjustable(). Don't you >> actually want to release the *unaligned* region you requested? >> > Isn't it what we're doing here? > (The release_mem_region_adjustable() is using the same > dax_kmem-aligned range and there's no split/adjust) Oh, I think I was messing up things (there is just too much going on in this patch). Right, request_mem_region() and add_memory_driver_managed() are - and were - called with the exact same range. That would have been clearer if the patch would simply use range.start and range_len(&range) for both calls (similar in the original code). So, also the release calls have to use the same range. Agreed. > > Meaning right now (+ parent marked as !BUSY), and if I am understanding > this correctly: > > request_mem_region(range.start, range_len) > __request_region(iomem_res, range.start, range_len) -> alloc @parent > add_memory_driver_managed(parent.start, resource_size(parent)) > __request_region(parent.start, resource_size(parent)) -> alloc @child > > [...] > > remove_memory(range.start, range_len) > request_mem_region_adjustable(range.start, range_len) > __release_region(range.start, range_len) -> remove @child > > release_mem_region(range.start, range_len) > __release_region(range.start, range_len) -> doesn't remove @parent because !BUSY? > > The add/removal of this relies on !BUSY. But now I am wondering if the parent remaining > unreleased is deliberate even on CONFIG_MEMORY_HOTREMOVE=y. Interesting, I can only tell that virtio-mem expects that remove_memory() won't remove the parent resource (which is !BUSY). So it relies on the existing functionality. I do wonder how walk_system_ram_range() behaves if both the parent and the child are BUSY. Looking at it, I think it will detect the parent and skip to the next range (without visiting the child) - which is not what we want. We could set the parent to BUSY just before doing the release_mem_region() call, but that feels like a hack. Maybe it's just easier to keep dax_kmem_res around ...
On Tue, Sep 8, 2020 at 3:46 AM David Hildenbrand <david@redhat.com> wrote: > > On 22.08.20 01:21, Andrew Morton wrote: > > On Wed, 19 Aug 2020 18:53:57 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > > > >>> I think I am missing some important pieces. Bear with me. > >> > >> No worries, also bear with me, I'm going to be offline intermittently > >> until at least mid-September. Hopefully Joao and/or Vishal can jump in > >> on this discussion. > > > > Ordinarily I'd prefer a refresh&resend for 2+ week-old series such as > > this. > > > > But given that v4 all applies OK and that Dan has pending outages, I'll > > scoop up this version, even though at least one change has been suggested. > > > > Should I try to fix patch #11 while Dan is away? Because I think at > least two things in there are wrong (and it would have been better to > split that patch into reviewable pieces). Hey David, Back now, I'll take a look. I didn't immediately see a way to meaningfully break up that patch without some dead-code steps in the conversion, but I'll take another run at it.
On 08.09.20 17:33, Joao Martins wrote: > [Sorry for the late response] > > On 8/21/20 11:06 AM, David Hildenbrand wrote: >> On 03.08.20 07:03, Dan Williams wrote: >>> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev) >>> * could be mixed in a node with faster memory, causing >>> * unavoidable performance issues. >>> */ >>> - numa_node = dev_dax->target_node; >>> if (numa_node < 0) { >>> dev_warn(dev, "rejecting DAX region with invalid node: %d\n", >>> numa_node); >>> return -EINVAL; >>> } >>> >>> - /* Hotplug starting at the beginning of the next block: */ >>> - kmem_start = ALIGN(range->start, memory_block_size_bytes()); >>> - >>> - kmem_size = range_len(range); >>> - /* Adjust the size down to compensate for moving up kmem_start: */ >>> - kmem_size -= kmem_start - range->start; >>> - /* Align the size down to cover only complete blocks: */ >>> - kmem_size &= ~(memory_block_size_bytes() - 1); >>> - kmem_end = kmem_start + kmem_size; >>> - >>> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL); >>> - if (!new_res_name) >>> + res_name = kstrdup(dev_name(dev), GFP_KERNEL); >>> + if (!res_name) >>> return -ENOMEM; >>> >>> - /* Region is permanently reserved if hotremove fails. */ >>> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name); >>> - if (!new_res) { >>> - dev_warn(dev, "could not reserve region [%pa-%pa]\n", >>> - &kmem_start, &kmem_end); >>> - kfree(new_res_name); >>> + res = request_mem_region(range.start, range_len(&range), res_name); >> >> I think our range could be empty after aligning. I assume >> request_mem_region() would check that, but maybe we could report a >> better error/warning in that case. >> > dax_kmem_range() already returns a memory-block-aligned @range but > IIUC request_mem_region() isn't checking for that. Having said that > the returned @res wouldn't be different from the passed range.start. > >>> /* >>> * Ensure that future kexec'd kernels will not treat this as RAM >>> * automatically. >>> */ >>> - rc = add_memory_driver_managed(numa_node, new_res->start, >>> - resource_size(new_res), kmem_name); >>> + rc = add_memory_driver_managed(numa_node, res->start, >>> + resource_size(res), kmem_name); >>> + >>> + res->flags |= IORESOURCE_BUSY; >> >> Hm, I don't think that's correct. Any specific reason why to mark the >> not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could >> suddenly stumble over it - and e.g., similarly kexec code when trying to >> find memory for placing kexec images. I think we should leave this >> !BUSY, just as it is right now. >> > Agreed. > >>> if (rc) { >>> - release_resource(new_res); >>> - kfree(new_res); >>> - kfree(new_res_name); >>> + release_mem_region(range.start, range_len(&range)); >>> + kfree(res_name); >>> return rc; >>> } >>> - dev_dax->dax_kmem_res = new_res; >>> + >>> + dev_set_drvdata(dev, res_name); >>> >>> return 0; >>> } >>> >>> #ifdef CONFIG_MEMORY_HOTREMOVE >>> -static int dev_dax_kmem_remove(struct device *dev) >>> +static void dax_kmem_release(struct dev_dax *dev_dax) >>> { >>> - struct dev_dax *dev_dax = to_dev_dax(dev); >>> - struct resource *res = dev_dax->dax_kmem_res; >>> - resource_size_t kmem_start = res->start; >>> - resource_size_t kmem_size = resource_size(res); >>> - const char *res_name = res->name; >>> int rc; >>> + struct device *dev = &dev_dax->dev; >>> + const char *res_name = dev_get_drvdata(dev); >>> + struct range range = dax_kmem_range(dev_dax); >>> >>> /* >>> * We have one shot for removing memory, if some memory blocks were not >>> * offline prior to calling this function remove_memory() will fail, and >>> * there is no way to hotremove this memory until reboot because device >>> - * unbind will succeed even if we return failure. >>> + * unbind will proceed regardless of the remove_memory result. >>> */ >>> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size); >>> - if (rc) { >>> - any_hotremove_failed = true; >>> - dev_err(dev, >>> - "DAX region %pR cannot be hotremoved until the next reboot\n", >>> - res); >>> - return rc; >>> + rc = remove_memory(dev_dax->target_node, range.start, range_len(&range)); >>> + if (rc == 0) { >> >> if (!rc) ? >> > Better off would be to keep the old order: > > if (rc) { > any_hotremove_failed = true; > dev_err(dev, "%#llx-%#llx cannot be hotremoved until the next reboot\n", > range.start, range.end); > return; > } > > release_mem_region(range.start, range_len(&range)); > dev_set_drvdata(dev, NULL); > kfree(res_name); > return; > > >>> + release_mem_region(range.start, range_len(&range)); >> >> remove_memory() does a release_mem_region_adjustable(). Don't you >> actually want to release the *unaligned* region you requested? >> > Isn't it what we're doing here? > (The release_mem_region_adjustable() is using the same > dax_kmem-aligned range and there's no split/adjust) > > Meaning right now (+ parent marked as !BUSY), and if I am understanding > this correctly: > > request_mem_region(range.start, range_len) > __request_region(iomem_res, range.start, range_len) -> alloc @parent > add_memory_driver_managed(parent.start, resource_size(parent)) > __request_region(parent.start, resource_size(parent)) -> alloc @child > > [...] > > remove_memory(range.start, range_len) > request_mem_region_adjustable(range.start, range_len) > __release_region(range.start, range_len) -> remove @child > > release_mem_region(range.start, range_len) > __release_region(range.start, range_len) -> doesn't remove @parent because !BUSY? > > The add/removal of this relies on !BUSY. But now I am wondering if the parent remaining > unreleased is deliberate even on CONFIG_MEMORY_HOTREMOVE=y. > > Joao > Thinking about it, if we don't set the parent resource BUSY (which is what I think is the right way of doing things), and don't want to store the parent resource pointer, we could add something like lookup_resource() - e.g., lookup_mem_resource() - , however, searching properly in the whole hierarchy (instead of only the first level), and traversing down to the last hierarchy. Then it would be as simple as remove_memory(range.start, range_len) res = lookup_mem_resource(range.start); release_resource(res);
On Wed, Sep 23, 2020 at 1:04 AM David Hildenbrand <david@redhat.com> wrote: > > On 08.09.20 17:33, Joao Martins wrote: > > [Sorry for the late response] > > > > On 8/21/20 11:06 AM, David Hildenbrand wrote: > >> On 03.08.20 07:03, Dan Williams wrote: > >>> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev) > >>> * could be mixed in a node with faster memory, causing > >>> * unavoidable performance issues. > >>> */ > >>> - numa_node = dev_dax->target_node; > >>> if (numa_node < 0) { > >>> dev_warn(dev, "rejecting DAX region with invalid node: %d\n", > >>> numa_node); > >>> return -EINVAL; > >>> } > >>> > >>> - /* Hotplug starting at the beginning of the next block: */ > >>> - kmem_start = ALIGN(range->start, memory_block_size_bytes()); > >>> - > >>> - kmem_size = range_len(range); > >>> - /* Adjust the size down to compensate for moving up kmem_start: */ > >>> - kmem_size -= kmem_start - range->start; > >>> - /* Align the size down to cover only complete blocks: */ > >>> - kmem_size &= ~(memory_block_size_bytes() - 1); > >>> - kmem_end = kmem_start + kmem_size; > >>> - > >>> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL); > >>> - if (!new_res_name) > >>> + res_name = kstrdup(dev_name(dev), GFP_KERNEL); > >>> + if (!res_name) > >>> return -ENOMEM; > >>> > >>> - /* Region is permanently reserved if hotremove fails. */ > >>> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name); > >>> - if (!new_res) { > >>> - dev_warn(dev, "could not reserve region [%pa-%pa]\n", > >>> - &kmem_start, &kmem_end); > >>> - kfree(new_res_name); > >>> + res = request_mem_region(range.start, range_len(&range), res_name); > >> > >> I think our range could be empty after aligning. I assume > >> request_mem_region() would check that, but maybe we could report a > >> better error/warning in that case. > >> > > dax_kmem_range() already returns a memory-block-aligned @range but > > IIUC request_mem_region() isn't checking for that. Having said that > > the returned @res wouldn't be different from the passed range.start. > > > >>> /* > >>> * Ensure that future kexec'd kernels will not treat this as RAM > >>> * automatically. > >>> */ > >>> - rc = add_memory_driver_managed(numa_node, new_res->start, > >>> - resource_size(new_res), kmem_name); > >>> + rc = add_memory_driver_managed(numa_node, res->start, > >>> + resource_size(res), kmem_name); > >>> + > >>> + res->flags |= IORESOURCE_BUSY; > >> > >> Hm, I don't think that's correct. Any specific reason why to mark the > >> not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could > >> suddenly stumble over it - and e.g., similarly kexec code when trying to > >> find memory for placing kexec images. I think we should leave this > >> !BUSY, just as it is right now. > >> > > Agreed. > > > >>> if (rc) { > >>> - release_resource(new_res); > >>> - kfree(new_res); > >>> - kfree(new_res_name); > >>> + release_mem_region(range.start, range_len(&range)); > >>> + kfree(res_name); > >>> return rc; > >>> } > >>> - dev_dax->dax_kmem_res = new_res; > >>> + > >>> + dev_set_drvdata(dev, res_name); > >>> > >>> return 0; > >>> } > >>> > >>> #ifdef CONFIG_MEMORY_HOTREMOVE > >>> -static int dev_dax_kmem_remove(struct device *dev) > >>> +static void dax_kmem_release(struct dev_dax *dev_dax) > >>> { > >>> - struct dev_dax *dev_dax = to_dev_dax(dev); > >>> - struct resource *res = dev_dax->dax_kmem_res; > >>> - resource_size_t kmem_start = res->start; > >>> - resource_size_t kmem_size = resource_size(res); > >>> - const char *res_name = res->name; > >>> int rc; > >>> + struct device *dev = &dev_dax->dev; > >>> + const char *res_name = dev_get_drvdata(dev); > >>> + struct range range = dax_kmem_range(dev_dax); > >>> > >>> /* > >>> * We have one shot for removing memory, if some memory blocks were not > >>> * offline prior to calling this function remove_memory() will fail, and > >>> * there is no way to hotremove this memory until reboot because device > >>> - * unbind will succeed even if we return failure. > >>> + * unbind will proceed regardless of the remove_memory result. > >>> */ > >>> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size); > >>> - if (rc) { > >>> - any_hotremove_failed = true; > >>> - dev_err(dev, > >>> - "DAX region %pR cannot be hotremoved until the next reboot\n", > >>> - res); > >>> - return rc; > >>> + rc = remove_memory(dev_dax->target_node, range.start, range_len(&range)); > >>> + if (rc == 0) { > >> > >> if (!rc) ? > >> > > Better off would be to keep the old order: > > > > if (rc) { > > any_hotremove_failed = true; > > dev_err(dev, "%#llx-%#llx cannot be hotremoved until the next reboot\n", > > range.start, range.end); > > return; > > } > > > > release_mem_region(range.start, range_len(&range)); > > dev_set_drvdata(dev, NULL); > > kfree(res_name); > > return; > > > > > >>> + release_mem_region(range.start, range_len(&range)); > >> > >> remove_memory() does a release_mem_region_adjustable(). Don't you > >> actually want to release the *unaligned* region you requested? > >> > > Isn't it what we're doing here? > > (The release_mem_region_adjustable() is using the same > > dax_kmem-aligned range and there's no split/adjust) > > > > Meaning right now (+ parent marked as !BUSY), and if I am understanding > > this correctly: > > > > request_mem_region(range.start, range_len) > > __request_region(iomem_res, range.start, range_len) -> alloc @parent > > add_memory_driver_managed(parent.start, resource_size(parent)) > > __request_region(parent.start, resource_size(parent)) -> alloc @child > > > > [...] > > > > remove_memory(range.start, range_len) > > request_mem_region_adjustable(range.start, range_len) > > __release_region(range.start, range_len) -> remove @child > > > > release_mem_region(range.start, range_len) > > __release_region(range.start, range_len) -> doesn't remove @parent because !BUSY? > > > > The add/removal of this relies on !BUSY. But now I am wondering if the parent remaining > > unreleased is deliberate even on CONFIG_MEMORY_HOTREMOVE=y. > > > > Joao > > > > Thinking about it, if we don't set the parent resource BUSY (which is > what I think is the right way of doing things), and don't want to store > the parent resource pointer, we could add something like > lookup_resource() - e.g., lookup_mem_resource() - , however, searching > properly in the whole hierarchy (instead of only the first level), and > traversing down to the last hierarchy. Then it would be as simple as > > remove_memory(range.start, range_len) > res = lookup_mem_resource(range.start); > release_resource(res); Another thought... I notice that you've taught register_memory_resource() a IORESOURCE_MEM_DRIVER_MANAGED special case. Lets just make the assumption of add_memory_driver_managed() that it is the driver's responsibility to mark the range busy before calling, and the driver's responsibility to release the region. I.e. validate (rather than request) that the range is busy in register_memory_resource(), and teach release_memory_resource() to skip releasing the region when the memory is marked driver managed. That would let dax_kmem drop its manipulation of the 'busy' flag which is a layering violation no matter how many comments we put around it.
On Thu, Sep 24, 2020 at 12:26 AM David Hildenbrand <david@redhat.com> wrote: > > On 23.09.20 23:41, Dan Williams wrote: > > On Wed, Sep 23, 2020 at 1:04 AM David Hildenbrand <david@redhat.com> wrote: > >> > >> On 08.09.20 17:33, Joao Martins wrote: > >>> [Sorry for the late response] > >>> > >>> On 8/21/20 11:06 AM, David Hildenbrand wrote: > >>>> On 03.08.20 07:03, Dan Williams wrote: > >>>>> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev) > >>>>> * could be mixed in a node with faster memory, causing > >>>>> * unavoidable performance issues. > >>>>> */ > >>>>> - numa_node = dev_dax->target_node; > >>>>> if (numa_node < 0) { > >>>>> dev_warn(dev, "rejecting DAX region with invalid node: %d\n", > >>>>> numa_node); > >>>>> return -EINVAL; > >>>>> } > >>>>> > >>>>> - /* Hotplug starting at the beginning of the next block: */ > >>>>> - kmem_start = ALIGN(range->start, memory_block_size_bytes()); > >>>>> - > >>>>> - kmem_size = range_len(range); > >>>>> - /* Adjust the size down to compensate for moving up kmem_start: */ > >>>>> - kmem_size -= kmem_start - range->start; > >>>>> - /* Align the size down to cover only complete blocks: */ > >>>>> - kmem_size &= ~(memory_block_size_bytes() - 1); > >>>>> - kmem_end = kmem_start + kmem_size; > >>>>> - > >>>>> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL); > >>>>> - if (!new_res_name) > >>>>> + res_name = kstrdup(dev_name(dev), GFP_KERNEL); > >>>>> + if (!res_name) > >>>>> return -ENOMEM; > >>>>> > >>>>> - /* Region is permanently reserved if hotremove fails. */ > >>>>> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name); > >>>>> - if (!new_res) { > >>>>> - dev_warn(dev, "could not reserve region [%pa-%pa]\n", > >>>>> - &kmem_start, &kmem_end); > >>>>> - kfree(new_res_name); > >>>>> + res = request_mem_region(range.start, range_len(&range), res_name); > >>>> > >>>> I think our range could be empty after aligning. I assume > >>>> request_mem_region() would check that, but maybe we could report a > >>>> better error/warning in that case. > >>>> > >>> dax_kmem_range() already returns a memory-block-aligned @range but > >>> IIUC request_mem_region() isn't checking for that. Having said that > >>> the returned @res wouldn't be different from the passed range.start. > >>> > >>>>> /* > >>>>> * Ensure that future kexec'd kernels will not treat this as RAM > >>>>> * automatically. > >>>>> */ > >>>>> - rc = add_memory_driver_managed(numa_node, new_res->start, > >>>>> - resource_size(new_res), kmem_name); > >>>>> + rc = add_memory_driver_managed(numa_node, res->start, > >>>>> + resource_size(res), kmem_name); > >>>>> + > >>>>> + res->flags |= IORESOURCE_BUSY; > >>>> > >>>> Hm, I don't think that's correct. Any specific reason why to mark the > >>>> not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could > >>>> suddenly stumble over it - and e.g., similarly kexec code when trying to > >>>> find memory for placing kexec images. I think we should leave this > >>>> !BUSY, just as it is right now. > >>>> > >>> Agreed. > >>> > >>>>> if (rc) { > >>>>> - release_resource(new_res); > >>>>> - kfree(new_res); > >>>>> - kfree(new_res_name); > >>>>> + release_mem_region(range.start, range_len(&range)); > >>>>> + kfree(res_name); > >>>>> return rc; > >>>>> } > >>>>> - dev_dax->dax_kmem_res = new_res; > >>>>> + > >>>>> + dev_set_drvdata(dev, res_name); > >>>>> > >>>>> return 0; > >>>>> } > >>>>> > >>>>> #ifdef CONFIG_MEMORY_HOTREMOVE > >>>>> -static int dev_dax_kmem_remove(struct device *dev) > >>>>> +static void dax_kmem_release(struct dev_dax *dev_dax) > >>>>> { > >>>>> - struct dev_dax *dev_dax = to_dev_dax(dev); > >>>>> - struct resource *res = dev_dax->dax_kmem_res; > >>>>> - resource_size_t kmem_start = res->start; > >>>>> - resource_size_t kmem_size = resource_size(res); > >>>>> - const char *res_name = res->name; > >>>>> int rc; > >>>>> + struct device *dev = &dev_dax->dev; > >>>>> + const char *res_name = dev_get_drvdata(dev); > >>>>> + struct range range = dax_kmem_range(dev_dax); > >>>>> > >>>>> /* > >>>>> * We have one shot for removing memory, if some memory blocks were not > >>>>> * offline prior to calling this function remove_memory() will fail, and > >>>>> * there is no way to hotremove this memory until reboot because device > >>>>> - * unbind will succeed even if we return failure. > >>>>> + * unbind will proceed regardless of the remove_memory result. > >>>>> */ > >>>>> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size); > >>>>> - if (rc) { > >>>>> - any_hotremove_failed = true; > >>>>> - dev_err(dev, > >>>>> - "DAX region %pR cannot be hotremoved until the next reboot\n", > >>>>> - res); > >>>>> - return rc; > >>>>> + rc = remove_memory(dev_dax->target_node, range.start, range_len(&range)); > >>>>> + if (rc == 0) { > >>>> > >>>> if (!rc) ? > >>>> > >>> Better off would be to keep the old order: > >>> > >>> if (rc) { > >>> any_hotremove_failed = true; > >>> dev_err(dev, "%#llx-%#llx cannot be hotremoved until the next reboot\n", > >>> range.start, range.end); > >>> return; > >>> } > >>> > >>> release_mem_region(range.start, range_len(&range)); > >>> dev_set_drvdata(dev, NULL); > >>> kfree(res_name); > >>> return; > >>> > >>> > >>>>> + release_mem_region(range.start, range_len(&range)); > >>>> > >>>> remove_memory() does a release_mem_region_adjustable(). Don't you > >>>> actually want to release the *unaligned* region you requested? > >>>> > >>> Isn't it what we're doing here? > >>> (The release_mem_region_adjustable() is using the same > >>> dax_kmem-aligned range and there's no split/adjust) > >>> > >>> Meaning right now (+ parent marked as !BUSY), and if I am understanding > >>> this correctly: > >>> > >>> request_mem_region(range.start, range_len) > >>> __request_region(iomem_res, range.start, range_len) -> alloc @parent > >>> add_memory_driver_managed(parent.start, resource_size(parent)) > >>> __request_region(parent.start, resource_size(parent)) -> alloc @child > >>> > >>> [...] > >>> > >>> remove_memory(range.start, range_len) > >>> request_mem_region_adjustable(range.start, range_len) > >>> __release_region(range.start, range_len) -> remove @child > >>> > >>> release_mem_region(range.start, range_len) > >>> __release_region(range.start, range_len) -> doesn't remove @parent because !BUSY? > >>> > >>> The add/removal of this relies on !BUSY. But now I am wondering if the parent remaining > >>> unreleased is deliberate even on CONFIG_MEMORY_HOTREMOVE=y. > >>> > >>> Joao > >>> > >> > >> Thinking about it, if we don't set the parent resource BUSY (which is > >> what I think is the right way of doing things), and don't want to store > >> the parent resource pointer, we could add something like > >> lookup_resource() - e.g., lookup_mem_resource() - , however, searching > >> properly in the whole hierarchy (instead of only the first level), and > >> traversing down to the last hierarchy. Then it would be as simple as > >> > >> remove_memory(range.start, range_len) > >> res = lookup_mem_resource(range.start); > >> release_resource(res); > > > > Another thought... I notice that you've taught > > register_memory_resource() a IORESOURCE_MEM_DRIVER_MANAGED special > > case. Lets just make the assumption of add_memory_driver_managed() > > that it is the driver's responsibility to mark the range busy before > > calling, and the driver's responsibility to release the region. I.e. > > validate (rather than request) that the range is busy in > > register_memory_resource(), and teach release_memory_resource() to > > skip releasing the region when the memory is marked driver managed. > > That would let dax_kmem drop its manipulation of the 'busy' flag which > > is a layering violation no matter how many comments we put around it. > > IIUC, that won't work for virtio-mem, whereby the parent resource spans > multiple possible (future) add_memory_driver_managed() calls and is > (just like for kmem) a pure indication to which device memory ranges belong. > > For example, when exposing 2GB via a virtio-mem device with max 4GB: > > (/proc/iomem) > 240000000-33fffffff : virtio0 > 240000000-2bfffffff : System RAM (virtio_mem) > > And after hotplugging additional 2GB: > > 240000000-33fffffff : virtio0 > 240000000-33fffffff : System RAM (virtio_mem) > > So marking "virtio0" always BUSY (especially right from the start) would > be wrong. I'm not suggesting to busy the whole "virtio" range, just the portion that's about to be passed to add_memory_driver_managed(). > The assumption is that anything that's IORESOURCE_SYSTEM_RAM > and IORESOUCE_BUSY is currently added to the system as system RAM (e.g., > after add_memory() and friends, or during boot). > > I do agree that manually clearing the busy flag is ugly. What we most > probably want is request_mem_region() that performs similar checks (no > overlaps with existing BUSY resources), but doesn't set the region busy. > I can't see that working without some way to export and hold the resource lock until some agent can atomically claim the range.
On 24.09.20 15:54, Dan Williams wrote: > On Thu, Sep 24, 2020 at 12:26 AM David Hildenbrand <david@redhat.com> wrote: >> >> On 23.09.20 23:41, Dan Williams wrote: >>> On Wed, Sep 23, 2020 at 1:04 AM David Hildenbrand <david@redhat.com> wrote: >>>> >>>> On 08.09.20 17:33, Joao Martins wrote: >>>>> [Sorry for the late response] >>>>> >>>>> On 8/21/20 11:06 AM, David Hildenbrand wrote: >>>>>> On 03.08.20 07:03, Dan Williams wrote: >>>>>>> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev) >>>>>>> * could be mixed in a node with faster memory, causing >>>>>>> * unavoidable performance issues. >>>>>>> */ >>>>>>> - numa_node = dev_dax->target_node; >>>>>>> if (numa_node < 0) { >>>>>>> dev_warn(dev, "rejecting DAX region with invalid node: %d\n", >>>>>>> numa_node); >>>>>>> return -EINVAL; >>>>>>> } >>>>>>> >>>>>>> - /* Hotplug starting at the beginning of the next block: */ >>>>>>> - kmem_start = ALIGN(range->start, memory_block_size_bytes()); >>>>>>> - >>>>>>> - kmem_size = range_len(range); >>>>>>> - /* Adjust the size down to compensate for moving up kmem_start: */ >>>>>>> - kmem_size -= kmem_start - range->start; >>>>>>> - /* Align the size down to cover only complete blocks: */ >>>>>>> - kmem_size &= ~(memory_block_size_bytes() - 1); >>>>>>> - kmem_end = kmem_start + kmem_size; >>>>>>> - >>>>>>> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL); >>>>>>> - if (!new_res_name) >>>>>>> + res_name = kstrdup(dev_name(dev), GFP_KERNEL); >>>>>>> + if (!res_name) >>>>>>> return -ENOMEM; >>>>>>> >>>>>>> - /* Region is permanently reserved if hotremove fails. */ >>>>>>> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name); >>>>>>> - if (!new_res) { >>>>>>> - dev_warn(dev, "could not reserve region [%pa-%pa]\n", >>>>>>> - &kmem_start, &kmem_end); >>>>>>> - kfree(new_res_name); >>>>>>> + res = request_mem_region(range.start, range_len(&range), res_name); >>>>>> >>>>>> I think our range could be empty after aligning. I assume >>>>>> request_mem_region() would check that, but maybe we could report a >>>>>> better error/warning in that case. >>>>>> >>>>> dax_kmem_range() already returns a memory-block-aligned @range but >>>>> IIUC request_mem_region() isn't checking for that. Having said that >>>>> the returned @res wouldn't be different from the passed range.start. >>>>> >>>>>>> /* >>>>>>> * Ensure that future kexec'd kernels will not treat this as RAM >>>>>>> * automatically. >>>>>>> */ >>>>>>> - rc = add_memory_driver_managed(numa_node, new_res->start, >>>>>>> - resource_size(new_res), kmem_name); >>>>>>> + rc = add_memory_driver_managed(numa_node, res->start, >>>>>>> + resource_size(res), kmem_name); >>>>>>> + >>>>>>> + res->flags |= IORESOURCE_BUSY; >>>>>> >>>>>> Hm, I don't think that's correct. Any specific reason why to mark the >>>>>> not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could >>>>>> suddenly stumble over it - and e.g., similarly kexec code when trying to >>>>>> find memory for placing kexec images. I think we should leave this >>>>>> !BUSY, just as it is right now. >>>>>> >>>>> Agreed. >>>>> >>>>>>> if (rc) { >>>>>>> - release_resource(new_res); >>>>>>> - kfree(new_res); >>>>>>> - kfree(new_res_name); >>>>>>> + release_mem_region(range.start, range_len(&range)); >>>>>>> + kfree(res_name); >>>>>>> return rc; >>>>>>> } >>>>>>> - dev_dax->dax_kmem_res = new_res; >>>>>>> + >>>>>>> + dev_set_drvdata(dev, res_name); >>>>>>> >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> #ifdef CONFIG_MEMORY_HOTREMOVE >>>>>>> -static int dev_dax_kmem_remove(struct device *dev) >>>>>>> +static void dax_kmem_release(struct dev_dax *dev_dax) >>>>>>> { >>>>>>> - struct dev_dax *dev_dax = to_dev_dax(dev); >>>>>>> - struct resource *res = dev_dax->dax_kmem_res; >>>>>>> - resource_size_t kmem_start = res->start; >>>>>>> - resource_size_t kmem_size = resource_size(res); >>>>>>> - const char *res_name = res->name; >>>>>>> int rc; >>>>>>> + struct device *dev = &dev_dax->dev; >>>>>>> + const char *res_name = dev_get_drvdata(dev); >>>>>>> + struct range range = dax_kmem_range(dev_dax); >>>>>>> >>>>>>> /* >>>>>>> * We have one shot for removing memory, if some memory blocks were not >>>>>>> * offline prior to calling this function remove_memory() will fail, and >>>>>>> * there is no way to hotremove this memory until reboot because device >>>>>>> - * unbind will succeed even if we return failure. >>>>>>> + * unbind will proceed regardless of the remove_memory result. >>>>>>> */ >>>>>>> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size); >>>>>>> - if (rc) { >>>>>>> - any_hotremove_failed = true; >>>>>>> - dev_err(dev, >>>>>>> - "DAX region %pR cannot be hotremoved until the next reboot\n", >>>>>>> - res); >>>>>>> - return rc; >>>>>>> + rc = remove_memory(dev_dax->target_node, range.start, range_len(&range)); >>>>>>> + if (rc == 0) { >>>>>> >>>>>> if (!rc) ? >>>>>> >>>>> Better off would be to keep the old order: >>>>> >>>>> if (rc) { >>>>> any_hotremove_failed = true; >>>>> dev_err(dev, "%#llx-%#llx cannot be hotremoved until the next reboot\n", >>>>> range.start, range.end); >>>>> return; >>>>> } >>>>> >>>>> release_mem_region(range.start, range_len(&range)); >>>>> dev_set_drvdata(dev, NULL); >>>>> kfree(res_name); >>>>> return; >>>>> >>>>> >>>>>>> + release_mem_region(range.start, range_len(&range)); >>>>>> >>>>>> remove_memory() does a release_mem_region_adjustable(). Don't you >>>>>> actually want to release the *unaligned* region you requested? >>>>>> >>>>> Isn't it what we're doing here? >>>>> (The release_mem_region_adjustable() is using the same >>>>> dax_kmem-aligned range and there's no split/adjust) >>>>> >>>>> Meaning right now (+ parent marked as !BUSY), and if I am understanding >>>>> this correctly: >>>>> >>>>> request_mem_region(range.start, range_len) >>>>> __request_region(iomem_res, range.start, range_len) -> alloc @parent >>>>> add_memory_driver_managed(parent.start, resource_size(parent)) >>>>> __request_region(parent.start, resource_size(parent)) -> alloc @child >>>>> >>>>> [...] >>>>> >>>>> remove_memory(range.start, range_len) >>>>> request_mem_region_adjustable(range.start, range_len) >>>>> __release_region(range.start, range_len) -> remove @child >>>>> >>>>> release_mem_region(range.start, range_len) >>>>> __release_region(range.start, range_len) -> doesn't remove @parent because !BUSY? >>>>> >>>>> The add/removal of this relies on !BUSY. But now I am wondering if the parent remaining >>>>> unreleased is deliberate even on CONFIG_MEMORY_HOTREMOVE=y. >>>>> >>>>> Joao >>>>> >>>> >>>> Thinking about it, if we don't set the parent resource BUSY (which is >>>> what I think is the right way of doing things), and don't want to store >>>> the parent resource pointer, we could add something like >>>> lookup_resource() - e.g., lookup_mem_resource() - , however, searching >>>> properly in the whole hierarchy (instead of only the first level), and >>>> traversing down to the last hierarchy. Then it would be as simple as >>>> >>>> remove_memory(range.start, range_len) >>>> res = lookup_mem_resource(range.start); >>>> release_resource(res); >>> >>> Another thought... I notice that you've taught >>> register_memory_resource() a IORESOURCE_MEM_DRIVER_MANAGED special >>> case. Lets just make the assumption of add_memory_driver_managed() >>> that it is the driver's responsibility to mark the range busy before >>> calling, and the driver's responsibility to release the region. I.e. >>> validate (rather than request) that the range is busy in >>> register_memory_resource(), and teach release_memory_resource() to >>> skip releasing the region when the memory is marked driver managed. >>> That would let dax_kmem drop its manipulation of the 'busy' flag which >>> is a layering violation no matter how many comments we put around it. >> >> IIUC, that won't work for virtio-mem, whereby the parent resource spans >> multiple possible (future) add_memory_driver_managed() calls and is >> (just like for kmem) a pure indication to which device memory ranges belong. >> >> For example, when exposing 2GB via a virtio-mem device with max 4GB: >> >> (/proc/iomem) >> 240000000-33fffffff : virtio0 >> 240000000-2bfffffff : System RAM (virtio_mem) >> >> And after hotplugging additional 2GB: >> >> 240000000-33fffffff : virtio0 >> 240000000-33fffffff : System RAM (virtio_mem) >> >> So marking "virtio0" always BUSY (especially right from the start) would >> be wrong. > > I'm not suggesting to busy the whole "virtio" range, just the portion > that's about to be passed to add_memory_driver_managed(). I'm afraid I don't get your point. For virtio-mem: Before: 1. Create virtio0 container resource 2. (somewhen in the future) add_memory_driver_managed() - Create resource (System RAM (virtio_mem)), marking it busy/driver managed After: 1. Create virtio0 container resource 2. (somewhen in the future) Create resource (System RAM (virtio_mem)), marking it busy/driver managed 3. add_memory_driver_managed() Not helpful or simpler IMHO. > >> The assumption is that anything that's IORESOURCE_SYSTEM_RAM >> and IORESOUCE_BUSY is currently added to the system as system RAM (e.g., >> after add_memory() and friends, or during boot). >> >> I do agree that manually clearing the busy flag is ugly. What we most >> probably want is request_mem_region() that performs similar checks (no >> overlaps with existing BUSY resources), but doesn't set the region busy. >> > > I can't see that working without some way to export and hold the > resource lock until some agent can atomically claim the range. I don't think we have to care about races here. The "BUSY" checks is really just a check for leftovers, e.g., after kexec or after driver reloading. If somebody else would try to concurrently add System RAM /something else within the range of your device, something else, very weird, would be going on (let's call it a BUG, just like if somebody would be removing system RAM in your device range ...). For example, in case of virtio-mem, when you unload the driver, it cannot remove the "virtio0" resource in case some system ram in the range is still plugged (busy). So when reloading the driver, it would try to re-create the virtio0 resource, detect that some system ram in the range is still BUSY, and fail gracefully. This is how it works and how it's expected to work - at least for virtio-mem. I assume something similar can be observed with kmem, when trying to reload the driver or similar - but races shouldn't be relevant here.
Changes since v3 [1]: - Update x86 boot options documentation for 'nohmat' (Randy) - Fixup a handful of kbuild robot reports, the most significant being moving usage of PUD_SIZE and PMD_SIZE under #ifdef CONFIG_TRANSPARENT_HUGEPAGE protection. [1]: http://lore.kernel.org/r/159625229779.3040297.11363509688097221416.stgit@dwillia2-desk3.amr.corp.intel.com --- Merge notes: Well, no v5.8-rc8 to line this up for v5.9, so next best is early integration into -mm before other collisions develop. Chatted with Justin offline and it currently appears that the missing numa information is the fault of the platform firmware to populate all the necessary NUMA data in the NFIT. --- Cover: The device-dax facility allows an address range to be directly mapped through a chardev, or optionally hotplugged to the core kernel page allocator as System-RAM. It is the mechanism for converting persistent memory (pmem) to be used as another volatile memory pool i.e. the current Memory Tiering hot topic on linux-mm. In the case of pmem the nvdimm-namespace-label mechanism can sub-divide it, but that labeling mechanism is not available / applicable to soft-reserved ("EFI specific purpose") memory [3]. This series provides a sysfs-mechanism for the daxctl utility to enable provisioning of volatile-soft-reserved memory ranges. The motivations for this facility are: 1/ Allow performance differentiated memory ranges to be split between kernel-managed and directly-accessed use cases. 2/ Allow physical memory to be provisioned along performance relevant address boundaries. For example, divide a memory-side cache [4] along cache-color boundaries. 3/ Parcel out soft-reserved memory to VMs using device-dax as a security / permissions boundary [5]. Specifically I have seen people (ab)using memmap=nn!ss (mark System-RAM as Persistent Memory) just to get the device-dax interface on custom address ranges. A follow-on for the VM use case is to teach device-dax to dynamically allocate 'struct page' at runtime to reduce the duplication of 'struct page' space in both the guest and the host kernel for the same physical pages. [2]: http://lore.kernel.org/r/20200713160837.13774-11-joao.m.martins@oracle.com [3]: http://lore.kernel.org/r/157309097008.1579826.12818463304589384434.stgit@dwillia2-desk3.amr.corp.intel.com [4]: http://lore.kernel.org/r/154899811738.3165233.12325692939590944259.stgit@dwillia2-desk3.amr.corp.intel.com [5]: http://lore.kernel.org/r/20200110190313.17144-1-joao.m.martins@oracle.com --- Dan Williams (19): x86/numa: Cleanup configuration dependent command-line options x86/numa: Add 'nohmat' option efi/fake_mem: Arrange for a resource entry per efi_fake_mem instance ACPI: HMAT: Refactor hmat_register_target_device to hmem_register_device resource: Report parent to walk_iomem_res_desc() callback mm/memory_hotplug: Introduce default phys_to_target_node() implementation ACPI: HMAT: Attach a device for each soft-reserved range device-dax: Drop the dax_region.pfn_flags attribute device-dax: Move instance creation parameters to 'struct dev_dax_data' device-dax: Make pgmap optional for instance creation device-dax: Kill dax_kmem_res device-dax: Add an allocation interface for device-dax instances device-dax: Introduce 'seed' devices drivers/base: Make device_find_child_by_name() compatible with sysfs inputs device-dax: Add resize support mm/memremap_pages: Convert to 'struct range' mm/memremap_pages: Support multiple ranges per invocation device-dax: Add dis-contiguous resource support device-dax: Introduce 'mapping' devices Joao Martins (4): device-dax: Make align a per-device property device-dax: Add an 'align' attribute dax/hmem: Introduce dax_hmem.region_idle parameter device-dax: Add a range mapping allocation attribute Documentation/x86/x86_64/boot-options.rst | 4 arch/powerpc/kvm/book3s_hv_uvmem.c | 14 arch/x86/include/asm/numa.h | 8 arch/x86/kernel/e820.c | 16 arch/x86/mm/numa.c | 11 arch/x86/mm/numa_emulation.c | 3 arch/x86/xen/enlighten_pv.c | 2 drivers/acpi/numa/hmat.c | 76 -- drivers/acpi/numa/srat.c | 9 drivers/base/core.c | 2 drivers/dax/Kconfig | 4 drivers/dax/Makefile | 3 drivers/dax/bus.c | 1046 +++++++++++++++++++++++++++-- drivers/dax/bus.h | 28 - drivers/dax/dax-private.h | 60 +- drivers/dax/device.c | 134 ++-- drivers/dax/hmem.c | 56 -- drivers/dax/hmem/Makefile | 6 drivers/dax/hmem/device.c | 100 +++ drivers/dax/hmem/hmem.c | 65 ++ drivers/dax/kmem.c | 199 +++--- drivers/dax/pmem/compat.c | 2 drivers/dax/pmem/core.c | 22 - drivers/firmware/efi/x86_fake_mem.c | 12 drivers/gpu/drm/nouveau/nouveau_dmem.c | 15 drivers/nvdimm/badrange.c | 26 - drivers/nvdimm/claim.c | 13 drivers/nvdimm/nd.h | 3 drivers/nvdimm/pfn_devs.c | 13 drivers/nvdimm/pmem.c | 27 - drivers/nvdimm/region.c | 21 - drivers/pci/p2pdma.c | 12 include/acpi/acpi_numa.h | 14 include/linux/dax.h | 8 include/linux/memory_hotplug.h | 5 include/linux/memremap.h | 11 include/linux/numa.h | 11 include/linux/range.h | 6 kernel/resource.c | 11 lib/test_hmm.c | 15 mm/memory_hotplug.c | 10 mm/memremap.c | 299 +++++--- tools/testing/nvdimm/dax-dev.c | 22 - tools/testing/nvdimm/test/iomap.c | 2 44 files changed, 1825 insertions(+), 601 deletions(-) delete mode 100644 drivers/dax/hmem.c create mode 100644 drivers/dax/hmem/Makefile create mode 100644 drivers/dax/hmem/device.c create mode 100644 drivers/dax/hmem/hmem.c base-commit: 01830e6c042e8eb6eb202e05d7df8057135b4c26