From: "Verma, Vishal L" <vishal.l.verma@intel.com> To: "Williams, Dan J" <dan.j.williams@intel.com>, "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "stable@vger.kernel.org" <stable@vger.kernel.org> Subject: Re: [PATCH 2/2] libnvdimm, pfn: Pad pfn namespaces relative to other regions Date: Mon, 3 Dec 2018 18:37:48 +0000 [thread overview] Message-ID: <59376b8cdfd0d0fff2a13e65ef30d0755b1457be.camel@intel.com> (raw) In-Reply-To: <154308522458.237140.16550557898744466932.stgit@dwillia2-desk3.amr.corp.intel.com> On Sat, 2018-11-24 at 10:47 -0800, Dan Williams wrote: > Commit cfe30b872058 "libnvdimm, pmem: adjust for section collisions with > 'System RAM'" enabled Linux to workaround occasions where platform > firmware arranges for "System RAM" and "Persistent Memory" to collide > within a single section boundary. Unfortunately, as reported in this > issue [1], platform firmware can inflict the same collision between > persistent memory regions. > > The approach of interrogating iomem_resource does not work in this > case because platform firmware may merge multiple regions into a single > iomem_resource range. Instead provide a method to interrogate regions > that share the same parent bus. > > This is a stop-gap until the core-MM can grow support for hotplug on > sub-section boundaries. > > [1]: https://github.com/pmem/ndctl/issues/76 > > Fixes: cfe30b872058 ("libnvdimm, pmem: adjust for section collisions with...") > Cc: <stable@vger.kernel.org> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/nvdimm/nd-core.h | 2 + > drivers/nvdimm/pfn_devs.c | 64 ++++++++++++++++++++++++------------------ > drivers/nvdimm/region_devs.c | 41 +++++++++++++++++++++++++++ > 3 files changed, 80 insertions(+), 27 deletions(-) Looks good to me, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > > diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h > index 182258f64417..d0c621b32f72 100644 > --- a/drivers/nvdimm/nd-core.h > +++ b/drivers/nvdimm/nd-core.h > @@ -111,6 +111,8 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region, > struct nd_mapping *nd_mapping, resource_size_t *overlap); > resource_size_t nd_blk_available_dpa(struct nd_region *nd_region); > resource_size_t nd_region_available_dpa(struct nd_region *nd_region); > +int nd_region_conflict(struct nd_region *nd_region, resource_size_t start, > + resource_size_t size); > resource_size_t nvdimm_allocated_dpa(struct nvdimm_drvdata *ndd, > struct nd_label_id *label_id); > int alias_dpa_busy(struct device *dev, void *data); > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > index 24c64090169e..6f22272e8d80 100644 > --- a/drivers/nvdimm/pfn_devs.c > +++ b/drivers/nvdimm/pfn_devs.c > @@ -649,14 +649,47 @@ static u64 phys_pmem_align_down(struct nd_pfn *nd_pfn, u64 phys) > ALIGN_DOWN(phys, nd_pfn->align)); > } > > +/* > + * Check if pmem collides with 'System RAM', or other regions when > + * section aligned. Trim it accordingly. > + */ > +static void trim_pfn_device(struct nd_pfn *nd_pfn, u32 *start_pad, u32 *end_trunc) > +{ > + struct nd_namespace_common *ndns = nd_pfn->ndns; > + struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev); > + struct nd_region *nd_region = to_nd_region(nd_pfn->dev.parent); > + const resource_size_t start = nsio->res.start; > + const resource_size_t end = start + resource_size(&nsio->res); > + resource_size_t adjust, size; > + > + *start_pad = 0; > + *end_trunc = 0; > + > + adjust = start - PHYS_SECTION_ALIGN_DOWN(start); > + size = resource_size(&nsio->res) + adjust; > + if (region_intersects(start - adjust, size, IORESOURCE_SYSTEM_RAM, > + IORES_DESC_NONE) == REGION_MIXED > + || nd_region_conflict(nd_region, start - adjust, size)) > + *start_pad = PHYS_SECTION_ALIGN_UP(start) - start; > + > + /* Now check that end of the range does not collide. */ > + adjust = PHYS_SECTION_ALIGN_UP(end) - end; > + size = resource_size(&nsio->res) + adjust; > + if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM, > + IORES_DESC_NONE) == REGION_MIXED > + || !IS_ALIGNED(end, nd_pfn->align) > + || nd_region_conflict(nd_region, start, size + adjust)) > + *end_trunc = end - phys_pmem_align_down(nd_pfn, end); > +} > + > static int nd_pfn_init(struct nd_pfn *nd_pfn) > { > u32 dax_label_reserve = is_nd_dax(&nd_pfn->dev) ? SZ_128K : 0; > struct nd_namespace_common *ndns = nd_pfn->ndns; > - u32 start_pad = 0, end_trunc = 0; > + struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev); > resource_size_t start, size; > - struct nd_namespace_io *nsio; > struct nd_region *nd_region; > + u32 start_pad, end_trunc; > struct nd_pfn_sb *pfn_sb; > unsigned long npfns; > phys_addr_t offset; > @@ -688,30 +721,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn) > > memset(pfn_sb, 0, sizeof(*pfn_sb)); > > - /* > - * 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 > - || !IS_ALIGNED(start + resource_size(&nsio->res), > - nd_pfn->align)) { > - size = resource_size(&nsio->res); > - end_trunc = start + size - phys_pmem_align_down(nd_pfn, > - start + size); > - } > - > + trim_pfn_device(nd_pfn, &start_pad, &end_trunc); > if (start_pad + end_trunc) > dev_info(&nd_pfn->dev, "%s alignment collision, truncate %d bytes\n", > dev_name(&ndns->dev), start_pad + end_trunc); > @@ -722,7 +732,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn) > * implementation will limit the pfns advertised through > * ->direct_access() to those that are included in the memmap. > */ > - start += start_pad; > + start = nsio->res.start + start_pad; > size = resource_size(&nsio->res); > npfns = PFN_SECTION_ALIGN_UP((size - start_pad - end_trunc - SZ_8K) > / PAGE_SIZE); > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c > index 174a418cb171..e7377f1028ef 100644 > --- a/drivers/nvdimm/region_devs.c > +++ b/drivers/nvdimm/region_devs.c > @@ -1184,6 +1184,47 @@ int nvdimm_has_cache(struct nd_region *nd_region) > } > EXPORT_SYMBOL_GPL(nvdimm_has_cache); > > +struct conflict_context { > + struct nd_region *nd_region; > + resource_size_t start, size; > +}; > + > +static int region_conflict(struct device *dev, void *data) > +{ > + struct nd_region *nd_region; > + struct conflict_context *ctx = data; > + resource_size_t res_end, region_end, region_start; > + > + if (!is_memory(dev)) > + return 0; > + > + nd_region = to_nd_region(dev); > + if (nd_region == ctx->nd_region) > + return 0; > + > + res_end = ctx->start + ctx->size; > + region_start = nd_region->ndr_start; > + region_end = region_start + nd_region->ndr_size; > + if (ctx->start >= region_start && ctx->start < region_end) > + return -EBUSY; > + if (res_end > region_start && res_end <= region_end) > + return -EBUSY; > + return 0; > +} > + > +int nd_region_conflict(struct nd_region *nd_region, resource_size_t start, > + resource_size_t size) > +{ > + struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(&nd_region->dev); > + struct conflict_context ctx = { > + .nd_region = nd_region, > + .start = start, > + .size = size, > + }; > + > + return device_for_each_child(&nvdimm_bus->dev, &ctx, region_conflict); > +} > + > void __exit nd_region_devs_exit(void) > { > ida_destroy(®ion_ida); > _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: "Verma, Vishal L" <vishal.l.verma@intel.com> To: "Williams, Dan J" <dan.j.williams@intel.com>, "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Jiang, Dave" <dave.jiang@intel.com>, "stable@vger.kernel.org" <stable@vger.kernel.org> Subject: Re: [PATCH 2/2] libnvdimm, pfn: Pad pfn namespaces relative to other regions Date: Mon, 3 Dec 2018 18:37:48 +0000 [thread overview] Message-ID: <59376b8cdfd0d0fff2a13e65ef30d0755b1457be.camel@intel.com> (raw) In-Reply-To: <154308522458.237140.16550557898744466932.stgit@dwillia2-desk3.amr.corp.intel.com> On Sat, 2018-11-24 at 10:47 -0800, Dan Williams wrote: > Commit cfe30b872058 "libnvdimm, pmem: adjust for section collisions with > 'System RAM'" enabled Linux to workaround occasions where platform > firmware arranges for "System RAM" and "Persistent Memory" to collide > within a single section boundary. Unfortunately, as reported in this > issue [1], platform firmware can inflict the same collision between > persistent memory regions. > > The approach of interrogating iomem_resource does not work in this > case because platform firmware may merge multiple regions into a single > iomem_resource range. Instead provide a method to interrogate regions > that share the same parent bus. > > This is a stop-gap until the core-MM can grow support for hotplug on > sub-section boundaries. > > [1]: https://github.com/pmem/ndctl/issues/76 > > Fixes: cfe30b872058 ("libnvdimm, pmem: adjust for section collisions with...") > Cc: <stable@vger.kernel.org> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/nvdimm/nd-core.h | 2 + > drivers/nvdimm/pfn_devs.c | 64 ++++++++++++++++++++++++------------------ > drivers/nvdimm/region_devs.c | 41 +++++++++++++++++++++++++++ > 3 files changed, 80 insertions(+), 27 deletions(-) Looks good to me, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > > diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h > index 182258f64417..d0c621b32f72 100644 > --- a/drivers/nvdimm/nd-core.h > +++ b/drivers/nvdimm/nd-core.h > @@ -111,6 +111,8 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region, > struct nd_mapping *nd_mapping, resource_size_t *overlap); > resource_size_t nd_blk_available_dpa(struct nd_region *nd_region); > resource_size_t nd_region_available_dpa(struct nd_region *nd_region); > +int nd_region_conflict(struct nd_region *nd_region, resource_size_t start, > + resource_size_t size); > resource_size_t nvdimm_allocated_dpa(struct nvdimm_drvdata *ndd, > struct nd_label_id *label_id); > int alias_dpa_busy(struct device *dev, void *data); > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > index 24c64090169e..6f22272e8d80 100644 > --- a/drivers/nvdimm/pfn_devs.c > +++ b/drivers/nvdimm/pfn_devs.c > @@ -649,14 +649,47 @@ static u64 phys_pmem_align_down(struct nd_pfn *nd_pfn, u64 phys) > ALIGN_DOWN(phys, nd_pfn->align)); > } > > +/* > + * Check if pmem collides with 'System RAM', or other regions when > + * section aligned. Trim it accordingly. > + */ > +static void trim_pfn_device(struct nd_pfn *nd_pfn, u32 *start_pad, u32 *end_trunc) > +{ > + struct nd_namespace_common *ndns = nd_pfn->ndns; > + struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev); > + struct nd_region *nd_region = to_nd_region(nd_pfn->dev.parent); > + const resource_size_t start = nsio->res.start; > + const resource_size_t end = start + resource_size(&nsio->res); > + resource_size_t adjust, size; > + > + *start_pad = 0; > + *end_trunc = 0; > + > + adjust = start - PHYS_SECTION_ALIGN_DOWN(start); > + size = resource_size(&nsio->res) + adjust; > + if (region_intersects(start - adjust, size, IORESOURCE_SYSTEM_RAM, > + IORES_DESC_NONE) == REGION_MIXED > + || nd_region_conflict(nd_region, start - adjust, size)) > + *start_pad = PHYS_SECTION_ALIGN_UP(start) - start; > + > + /* Now check that end of the range does not collide. */ > + adjust = PHYS_SECTION_ALIGN_UP(end) - end; > + size = resource_size(&nsio->res) + adjust; > + if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM, > + IORES_DESC_NONE) == REGION_MIXED > + || !IS_ALIGNED(end, nd_pfn->align) > + || nd_region_conflict(nd_region, start, size + adjust)) > + *end_trunc = end - phys_pmem_align_down(nd_pfn, end); > +} > + > static int nd_pfn_init(struct nd_pfn *nd_pfn) > { > u32 dax_label_reserve = is_nd_dax(&nd_pfn->dev) ? SZ_128K : 0; > struct nd_namespace_common *ndns = nd_pfn->ndns; > - u32 start_pad = 0, end_trunc = 0; > + struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev); > resource_size_t start, size; > - struct nd_namespace_io *nsio; > struct nd_region *nd_region; > + u32 start_pad, end_trunc; > struct nd_pfn_sb *pfn_sb; > unsigned long npfns; > phys_addr_t offset; > @@ -688,30 +721,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn) > > memset(pfn_sb, 0, sizeof(*pfn_sb)); > > - /* > - * 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 > - || !IS_ALIGNED(start + resource_size(&nsio->res), > - nd_pfn->align)) { > - size = resource_size(&nsio->res); > - end_trunc = start + size - phys_pmem_align_down(nd_pfn, > - start + size); > - } > - > + trim_pfn_device(nd_pfn, &start_pad, &end_trunc); > if (start_pad + end_trunc) > dev_info(&nd_pfn->dev, "%s alignment collision, truncate %d bytes\n", > dev_name(&ndns->dev), start_pad + end_trunc); > @@ -722,7 +732,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn) > * implementation will limit the pfns advertised through > * ->direct_access() to those that are included in the memmap. > */ > - start += start_pad; > + start = nsio->res.start + start_pad; > size = resource_size(&nsio->res); > npfns = PFN_SECTION_ALIGN_UP((size - start_pad - end_trunc - SZ_8K) > / PAGE_SIZE); > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c > index 174a418cb171..e7377f1028ef 100644 > --- a/drivers/nvdimm/region_devs.c > +++ b/drivers/nvdimm/region_devs.c > @@ -1184,6 +1184,47 @@ int nvdimm_has_cache(struct nd_region *nd_region) > } > EXPORT_SYMBOL_GPL(nvdimm_has_cache); > > +struct conflict_context { > + struct nd_region *nd_region; > + resource_size_t start, size; > +}; > + > +static int region_conflict(struct device *dev, void *data) > +{ > + struct nd_region *nd_region; > + struct conflict_context *ctx = data; > + resource_size_t res_end, region_end, region_start; > + > + if (!is_memory(dev)) > + return 0; > + > + nd_region = to_nd_region(dev); > + if (nd_region == ctx->nd_region) > + return 0; > + > + res_end = ctx->start + ctx->size; > + region_start = nd_region->ndr_start; > + region_end = region_start + nd_region->ndr_size; > + if (ctx->start >= region_start && ctx->start < region_end) > + return -EBUSY; > + if (res_end > region_start && res_end <= region_end) > + return -EBUSY; > + return 0; > +} > + > +int nd_region_conflict(struct nd_region *nd_region, resource_size_t start, > + resource_size_t size) > +{ > + struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(&nd_region->dev); > + struct conflict_context ctx = { > + .nd_region = nd_region, > + .start = start, > + .size = size, > + }; > + > + return device_for_each_child(&nvdimm_bus->dev, &ctx, region_conflict); > +} > + > void __exit nd_region_devs_exit(void) > { > ida_destroy(®ion_ida); >
next prev parent reply other threads:[~2018-12-03 18:37 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-24 18:46 [PATCH 1/2] tools/testing/nvdimm: Align test resources to 128M Dan Williams 2018-11-24 18:46 ` Dan Williams 2018-11-24 18:47 ` [PATCH 2/2] libnvdimm, pfn: Pad pfn namespaces relative to other regions Dan Williams 2018-11-24 18:47 ` Dan Williams [not found] ` <154308522458.237140.16550557898744466932.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2018-11-25 7:54 ` Sasha Levin 2018-11-25 7:54 ` Sasha Levin 2018-11-25 16:37 ` Dan Williams 2018-11-25 16:37 ` Dan Williams 2018-12-03 18:37 ` Verma, Vishal L [this message] 2018-12-03 18:37 ` Verma, Vishal L 2018-12-03 18:33 ` [PATCH 1/2] tools/testing/nvdimm: Align test resources to 128M Verma, Vishal L 2018-12-03 18:33 ` Verma, Vishal L 2018-12-05 22:15 ` [PATCH v2] " Dan Williams 2018-12-05 22:15 ` Dan Williams
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=59376b8cdfd0d0fff2a13e65ef30d0755b1457be.camel@intel.com \ --to=vishal.l.verma@intel.com \ --cc=dan.j.williams@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.org \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.