Re: [PATCH v2 2/3] libnvdimm, pmem: adjust for section collisions with 'System RAM'

[Toshi Kani]

On Thu, 2016-03-03 at 13:53 -0800, Dan Williams wrote:
> On a platform where 'Persistent Memory' and 'System RAM' are mixed
> within a given sparsemem section, trim the namespace and notify about the
> sub-optimal alignment.
> 
> Cc: Toshi Kani <toshi.kani@xxxxxxx>
> Cc: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
> Âdrivers/nvdimm/namespace_devs.c |ÂÂÂÂ7 ++
> Âdrivers/nvdimm/pfn.hÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂÂ10 ++-
> Âdrivers/nvdimm/pfn_devs.cÂÂÂÂÂÂÂ|ÂÂÂÂ5 ++
> Âdrivers/nvdimm/pmem.cÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ125 ++++++++++++++++++++++++++++-----
> ------
> Â4 files changed, 111 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/nvdimm/namespace_devs.c
> b/drivers/nvdimm/namespace_devs.c
> index 8ebfcaae3f5a..463756ca2d4b 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -133,6 +133,7 @@ bool nd_is_uuid_unique(struct device *dev, u8 *uuid)
> Âbool pmem_should_map_pages(struct device *dev)
> Â{
> Â	struct nd_region *nd_region = to_nd_region(dev->parent);
> +	struct nd_namespace_io *nsio;
> Â
> Â	if (!IS_ENABLED(CONFIG_ZONE_DEVICE))
> Â		return false;
> @@ -143,6 +144,12 @@ bool pmem_should_map_pages(struct device *dev)
> Â	if (is_nd_pfn(dev) || is_nd_btt(dev))
> Â		return false;
> Â
> +	nsio = to_nd_namespace_io(dev);
> +	if (region_intersects(nsio->res.start, resource_size(&nsio-
> >res),
> +				IORESOURCE_SYSTEM_RAM,
> +				IORES_DESC_NONE) == REGION_MIXED)

Should this be !=ÂREGION_DISJOINT for safe?

> +		return false;
> +

Â:

> @@ -304,21 +311,56 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
> Â	}
> Â
> Â	memset(pfn_sb, 0, sizeof(*pfn_sb));
> -	npfns = (pmem->size - SZ_8K) / SZ_4K;
> +
> +	/*
> +	Â* Check if pmem collides with 'System RAM' when section aligned
> and
> +	Â* trim it accordingly
> +	Â*/
> +	nsio = to_nd_namespace_io(&ndns->dev);
> +	start = PHYS_SECTION_ALIGN_DOWN(nsio->res.start);
> +	size = resource_size(&nsio->res);
> +	if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM,
> +				IORES_DESC_NONE) == REGION_MIXED) {
> +
> +		start = nsio->res.start;
> +		start_pad = PHYS_SECTION_ALIGN_UP(start) - start;
> +	}
> +
> +	start = nsio->res.start;
> +	size = PHYS_SECTION_ALIGN_UP(start + size) - start;
> +	if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM,
> +				IORES_DESC_NONE) == REGION_MIXED) {
> +		size = resource_size(&nsio->res);
> +		end_trunc = start + size - PHYS_SECTION_ALIGN_DOWN(start
> + size);
> +	}

This check seems to assume that guest's regular memory layout does not
change.ÂÂThat is, if there is no collision at first, there won't be any
later.ÂÂIs this a valid assumption?

Thanks,
-Toshi