Re: [PATCH v2] DAX: warn when kmem regions are truncated for memory block alignment.

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Tue,  1 Apr 2025 21:59:20 -0400
Gregory Price <gourry@gourry.net> wrote:

> Device capacity intended for use as system ram should be aligned to the
> archite-defined memory block size or that capacity will be silently

archite?

> truncated and capacity stranded.
> 
> As hotplug dax memory becomes more prevelant, the memory block size
> alignment becomes more important for platform and device vendors to
> pay attention to - so this truncation should not be silent.
> 
> This issue is particularly relevant for CXL Dynamic Capacity devices,
> whose capacity may arrive in spec-aligned but block-misaligned chunks.
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Gregory Price <gourry@gourry.net>

One trivial comment inline otherwise seems reasonable to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> ---
>  drivers/dax/kmem.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index e97d47f42ee2..32fe3215e11e 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c
> @@ -13,6 +13,7 @@
>  #include <linux/mman.h>
>  #include <linux/memory-tiers.h>
>  #include <linux/memory_hotplug.h>
> +#include <linux/string_helpers.h>
>  #include "dax-private.h"
>  #include "bus.h"
>  
> @@ -68,7 +69,7 @@ static void kmem_put_memory_types(void)
>  static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
>  {
>  	struct device *dev = &dev_dax->dev;
> -	unsigned long total_len = 0;
> +	unsigned long total_len = 0, orig_len = 0;
>  	struct dax_kmem_data *data;
>  	struct memory_dev_type *mtype;
>  	int i, rc, mapped = 0;
> @@ -97,6 +98,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
>  	for (i = 0; i < dev_dax->nr_range; i++) {
>  		struct range range;
>  
> +		orig_len += range_len(&dev_dax->ranges[i].range);
>  		rc = dax_kmem_range(dev_dax, i, &range);
>  		if (rc) {
>  			dev_info(dev, "mapping%d: %#llx-%#llx too small after alignment\n",
> @@ -109,6 +111,12 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
>  	if (!total_len) {
>  		dev_warn(dev, "rejecting DAX region without any memory after alignment\n");
>  		return -EINVAL;
> +	} else if (total_len != orig_len) {
> +		char buf[16];
> +
> +		string_get_size((orig_len - total_len), 1, STRING_UNITS_2,

Trivial but do those inner brackets really add anything?

> +				buf, sizeof(buf));
> +		dev_warn(dev, "DAX region truncated by %s due to alignment\n", buf);
>  	}
>  
>  	init_node_memory_type(numa_node, mtype);