Re: [PATCH v3 2/2] dax/kmem: allow kmem to add memory with memmap_on_memory

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

On Tue, 01 Aug 2023 23:55:38 -0600
Vishal Verma <vishal.l.verma@intel.com> wrote:

> Large amounts of memory managed by the kmem driver may come in via CXL,
> and it is often desirable to have the memmap for this memory on the new
> memory itself.
> 
> Enroll kmem-managed memory for memmap_on_memory semantics as a default.
> Add a sysfs override under the dax device to opt out of this behavior.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Hi Vishal,

In general looks fine to me.  Just a question for potential discussion if didn't
miss it in earlier versions.

FWIW
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Also, any docs need updating?  Doesn't seem like the DAX ABI docs are present in
Documentation/ABI so not sure where it should be updated.

Jonathan


>  
> @@ -1400,6 +1435,13 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data)
>  	dev_dax->align = dax_region->align;
>  	ida_init(&dev_dax->ida);
>  
> +	/*
> +	 * If supported by memory_hotplug, allow memmap_on_memory behavior by
> +	 * default. This can be overridden via sysfs before handing the memory
> +	 * over to kmem if desired.
> +	 */
> +	dev_dax->memmap_on_memory = true;

If there are existing users, then this is a fairly significant change of defaults.
Maybe it should be false and opt in rather than out?

> +
>  	inode = dax_inode(dax_dev);
>  	dev->devt = inode->i_rdev;
>  	dev->bus = &dax_bus_type;