Hi all,
Today's linux-next merge of the akpm-current tree got a conflict in:
mm/memory_hotplug.c
between commit:
514caf23a70f ("memremap: replace the altmap_valid field with a PGMAP_ALTMAP_VALID flag")
from the hmm tree and commit:
db30f881e2d7 ("mm/hotplug: kill is_dev_zone() usage in __remove_pages()")
from the akpm-current tree.
I fixed it up (I think - see below) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc mm/memory_hotplug.c
index 6166ba5a15f3,dfab21dc33dc..000000000000
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@@ -549,16 -537,14 +537,13 @@@ static void __remove_section(struct zon
* sure that pages are marked reserved and zones are adjust properly by
* calling offline_pages().
*/
- void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
+ void __remove_pages(struct zone *zone, unsigned long pfn,
unsigned long nr_pages, struct vmem_altmap *altmap)
{
- unsigned long i;
-- unsigned long map_offset = 0;
- int sections_to_remove;
++ unsigned long map_offset;
+ unsigned long nr, start_sec, end_sec;
- /* In the ZONE_DEVICE case device driver owns the memory region */
- if (is_dev_zone(zone))
- if (altmap)
-- map_offset = vmem_altmap_offset(altmap);
++ map_offset = vmem_altmap_offset(altmap);
clear_zone_contiguous(zone);
On Thu, Jul 04, 2019 at 08:55:36PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the akpm-current tree got a conflict in:
>
> mm/memory_hotplug.c
>
> between commit:
>
> 514caf23a70f ("memremap: replace the altmap_valid field with a PGMAP_ALTMAP_VALID flag")
>
> from the hmm tree and commit:
>
> db30f881e2d7 ("mm/hotplug: kill is_dev_zone() usage in __remove_pages()")
There must be another commit involved for the 'unsigned long nr,
start_sec, end_sec;' lines..
> from the akpm-current tree.
>
> I fixed it up (I think - see below) and can carry the fix as
> necessary. This is now fixed as far as linux-next is concerned, but any
> non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging. You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc mm/memory_hotplug.c
> index 6166ba5a15f3,dfab21dc33dc..000000000000
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@@ -549,16 -537,14 +537,13 @@@ static void __remove_section(struct zon
> * sure that pages are marked reserved and zones are adjust properly by
> * calling offline_pages().
> */
> - void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> + void __remove_pages(struct zone *zone, unsigned long pfn,
> unsigned long nr_pages, struct vmem_altmap *altmap)
> {
> - unsigned long i;
> -- unsigned long map_offset = 0;
> - int sections_to_remove;
> ++ unsigned long map_offset;
> + unsigned long nr, start_sec, end_sec;
>
> - /* In the ZONE_DEVICE case device driver owns the memory region */
> - if (is_dev_zone(zone))
> - if (altmap)
> -- map_offset = vmem_altmap_offset(altmap);
> ++ map_offset = vmem_altmap_offset(altmap);
>
> clear_zone_contiguous(zone);
>
This looks OK to me. After the trees are merged vmem_altmap_offset()
returns 0 if !altmap, so the code is equivalent.
Jason
Hi Jason,
On Thu, 4 Jul 2019 12:55:43 +0000 Jason Gunthorpe <[email protected]> wrote:
>
> On Thu, Jul 04, 2019 at 08:55:36PM +1000, Stephen Rothwell wrote:
> >
> > Today's linux-next merge of the akpm-current tree got a conflict in:
> >
> > mm/memory_hotplug.c
> >
> > between commit:
> >
> > 514caf23a70f ("memremap: replace the altmap_valid field with a PGMAP_ALTMAP_VALID flag")
> >
> > from the hmm tree and commit:
> >
> > db30f881e2d7 ("mm/hotplug: kill is_dev_zone() usage in __remove_pages()")
>
> There must be another commit involved for the 'unsigned long nr,
> start_sec, end_sec;' lines..
Yeah, there was, but that didn't actually create a conflict. That hunk
is only there because I removed the initialisation of map_offset.
--
Cheers,
Stephen Rothwell
On Thu, Jul 04, 2019 at 11:01:33PM +1000, Stephen Rothwell wrote:
> Hi Jason,
>
> On Thu, 4 Jul 2019 12:55:43 +0000 Jason Gunthorpe <[email protected]> wrote:
> >
> > On Thu, Jul 04, 2019 at 08:55:36PM +1000, Stephen Rothwell wrote:
> > >
> > > Today's linux-next merge of the akpm-current tree got a conflict in:
> > >
> > > mm/memory_hotplug.c
> > >
> > > between commit:
> > >
> > > 514caf23a70f ("memremap: replace the altmap_valid field with a PGMAP_ALTMAP_VALID flag")
> > >
> > > from the hmm tree and commit:
> > >
> > > db30f881e2d7 ("mm/hotplug: kill is_dev_zone() usage in __remove_pages()")
> >
> > There must be another commit involved for the 'unsigned long nr,
> > start_sec, end_sec;' lines..
>
> Yeah, there was, but that didn't actually create a conflict. That hunk
> is only there because I removed the initialisation of map_offset.
BTW, do you use a script to get these conflicting patch commit ID
automatically? It is so helpful to have them.
Jason
Hi Jason,
On Thu, 4 Jul 2019 13:28:41 +0000 Jason Gunthorpe <[email protected]> wrote:
>
> BTW, do you use a script to get these conflicting patch commit ID
> automatically? It is so helpful to have them.
No, I just use gitk and a bit of searching. Though often there are not
many possible commits to search.
--
Cheers,
Stephen Rothwell
Guys, Andrew has kicked the subsection patches out of -mm because of
the merge conflicts. Can we hold off on the hmm cleanups for this
cycle?
On Thu, Jul 4, 2019 at 2:08 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Jason,
>
> On Thu, 4 Jul 2019 13:28:41 +0000 Jason Gunthorpe <[email protected]> wrote:
> >
> > BTW, do you use a script to get these conflicting patch commit ID
> > automatically? It is so helpful to have them.
>
> No, I just use gitk and a bit of searching. Though often there are not
> many possible commits to search.
>
> --
> Cheers,
> Stephen Rothwell
On Thu, Jul 04, 2019 at 04:29:55PM -0700, Dan Williams wrote:
> Guys, Andrew has kicked the subsection patches out of -mm because of
> the merge conflicts. Can we hold off on the hmm cleanups for this
> cycle?
I agree with you we should prioritize your subsection patches over
CH's cleanup if we cannot have both.
As I said, I'll drop CH's at Andrews request, but I do not want to
make any changes without being aligned with him.
Jason
On Fri, 5 Jul 2019 12:08:15 +0000 Jason Gunthorpe <[email protected]> wrote:
> On Thu, Jul 04, 2019 at 04:29:55PM -0700, Dan Williams wrote:
> > Guys, Andrew has kicked the subsection patches out of -mm because of
> > the merge conflicts. Can we hold off on the hmm cleanups for this
> > cycle?
>
> I agree with you we should prioritize your subsection patches over
> CH's cleanup if we cannot have both.
>
> As I said, I'll drop CH's at Andrews request, but I do not want to
> make any changes without being aligned with him.
OK, I had a shot at repairing the damage on top of current linux-next.
The great majority of the issues were in
mm-devm_memremap_pages-enable-sub-section-remap.patch.
Below are the rejects which I saw and below that is my attempt to
resolve it all. Dan, please go through this with a toothcomb. I've
just done an mmotm release with all this in it so please do whatever's
needed to verify that it's all working correctly.
--- kernel/memremap.c~mm-devm_memremap_pages-enable-sub-section-remap
+++ kernel/memremap.c
@@ -58,7 +58,7 @@ static unsigned long pfn_first(struct de
struct vmem_altmap *altmap = &pgmap->altmap;
unsigned long pfn;
- pfn = res->start >> PAGE_SHIFT;
+ pfn = PHYS_PFN(res->start);
if (pgmap->altmap_valid)
pfn += vmem_altmap_offset(altmap);
return pfn;
@@ -95,25 +94,21 @@ static void devm_memremap_pages_release(
pgmap->cleanup(pgmap->ref);
/* pages are dead and unused, undo the arch mapping */
- align_start = res->start & ~(PA_SECTION_SIZE - 1);
- align_size = ALIGN(res->start + resource_size(res), PA_SECTION_SIZE)
- - align_start;
-
- nid = page_to_nid(pfn_to_page(align_start >> PAGE_SHIFT));
+ nid = page_to_nid(pfn_to_page(PHYS_PFN(res->start)));
mem_hotplug_begin();
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
- pfn = align_start >> PAGE_SHIFT;
+ pfn = PHYS_PFN(res->start);
__remove_pages(page_zone(pfn_to_page(pfn)), pfn,
- align_size >> PAGE_SHIFT, NULL);
+ PHYS_PFN(resource_size(res)), NULL);
} else {
- arch_remove_memory(nid, align_start, align_size,
+ arch_remove_memory(nid, res->start, resource_size(res),
pgmap->altmap_valid ? &pgmap->altmap : NULL);
- kasan_remove_zero_shadow(__va(align_start), align_size);
+ kasan_remove_zero_shadow(__va(res->start), resource_size(res));
}
mem_hotplug_done();
- untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
+ untrack_pfn(NULL, PHYS_PFN(res->start), resource_size(res));
pgmap_array_delete(res);
dev_WARN_ONCE(dev, pgmap->altmap.alloc,
"%s: failed to free all reserved pages\n", __func__);
@@ -140,16 +135,13 @@ static void devm_memremap_pages_release(
*/
void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
{
- resource_size_t align_start, align_size, align_end;
- struct vmem_altmap *altmap = pgmap->altmap_valid ?
- &pgmap->altmap : NULL;
struct resource *res = &pgmap->res;
struct dev_pagemap *conflict_pgmap;
struct mhp_restrictions restrictions = {
/*
* We do not want any optional features only our own memmap
*/
- .altmap = altmap,
+ .altmap = pgmap->altmap_valid ? &pgmap->altmap : NULL,
};
pgprot_t pgprot = PAGE_KERNEL;
int error, nid, is_ram;
@@ -159,12 +151,7 @@ void *devm_memremap_pages(struct device
return ERR_PTR(-EINVAL);
}
- align_start = res->start & ~(PA_SECTION_SIZE - 1);
- align_size = ALIGN(res->start + resource_size(res), PA_SECTION_SIZE)
- - align_start;
- align_end = align_start + align_size - 1;
-
- conflict_pgmap = get_dev_pagemap(PHYS_PFN(align_start), NULL);
+ conflict_pgmap = get_dev_pagemap(PHYS_PFN(res->start), NULL);
if (conflict_pgmap) {
dev_WARN(dev, "Conflicting mapping in same section\n");
put_dev_pagemap(conflict_pgmap);
@@ -220,25 +207,25 @@ void *devm_memremap_pages(struct device
* arch_add_memory().
*/
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
- error = add_pages(nid, align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, &restrictions);
+ error = add_pages(nid, PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), &restrictions);
} else {
- error = kasan_add_zero_shadow(__va(align_start), align_size);
+ error = kasan_add_zero_shadow(__va(res->start), resource_size(res));
if (error) {
mem_hotplug_done();
goto err_kasan;
}
- error = arch_add_memory(nid, align_start, align_size,
- &restrictions);
+ error = arch_add_memory(nid, res->start, resource_size(res),
+ &restrictions);
}
if (!error) {
struct zone *zone;
zone = &NODE_DATA(nid)->node_zones[ZONE_DEVICE];
- move_pfn_range_to_zone(zone, align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, altmap);
+ move_pfn_range_to_zone(zone, PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), restrictions.altmap);
}
mem_hotplug_done();
From: Dan Williams <[email protected]>
Subject: mm/devm_memremap_pages: enable sub-section remap
Teach devm_memremap_pages() about the new sub-section capabilities of
arch_{add,remove}_memory(). Effectively, just replace all usage of
align_start, align_end, and align_size with res->start, res->end, and
resource_size(res). The existing sanity check will still make sure that
the two separate remap attempts do not collide within a sub-section (2MB
on x86).
Link: http://lkml.kernel.org/r/156092355542.979959.10060071713397030576.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <[email protected]>
Tested-by: Aneesh Kumar K.V <[email protected]> [ppc64]
Cc: Michal Hocko <[email protected]>
Cc: Toshi Kani <[email protected]>
Cc: J?r?me Glisse <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Pavel Tatashin <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Jane Chu <[email protected]>
Cc: Jeff Moyer <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Wei Yang <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
kernel/memremap.c | 57 +++++++++++++++++---------------------------
1 file changed, 23 insertions(+), 34 deletions(-)
--- a/kernel/memremap.c~mm-devm_memremap_pages-enable-sub-section-remap
+++ a/kernel/memremap.c
@@ -54,7 +54,7 @@ static void pgmap_array_delete(struct re
static unsigned long pfn_first(struct dev_pagemap *pgmap)
{
- return (pgmap->res.start >> PAGE_SHIFT) +
+ return PHYS_PFN(pgmap->res.start) +
vmem_altmap_offset(pgmap_altmap(pgmap));
}
@@ -98,7 +98,6 @@ static void devm_memremap_pages_release(
struct dev_pagemap *pgmap = data;
struct device *dev = pgmap->dev;
struct resource *res = &pgmap->res;
- resource_size_t align_start, align_size;
unsigned long pfn;
int nid;
@@ -108,25 +107,21 @@ static void devm_memremap_pages_release(
dev_pagemap_cleanup(pgmap);
/* pages are dead and unused, undo the arch mapping */
- align_start = res->start & ~(SECTION_SIZE - 1);
- align_size = ALIGN(res->start + resource_size(res), SECTION_SIZE)
- - align_start;
-
- nid = page_to_nid(pfn_to_page(align_start >> PAGE_SHIFT));
+ nid = page_to_nid(pfn_to_page(PHYS_PFN(res->start)));
mem_hotplug_begin();
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
- pfn = align_start >> PAGE_SHIFT;
+ pfn = PHYS_PFN(res->start);
__remove_pages(page_zone(pfn_to_page(pfn)), pfn,
- align_size >> PAGE_SHIFT, NULL);
+ PHYS_PFN(resource_size(res)), NULL);
} else {
- arch_remove_memory(nid, align_start, align_size,
+ arch_remove_memory(nid, res->start, resource_size(res),
pgmap_altmap(pgmap));
- kasan_remove_zero_shadow(__va(align_start), align_size);
+ kasan_remove_zero_shadow(__va(res->start), resource_size(res));
}
mem_hotplug_done();
- untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
+ untrack_pfn(NULL, PHYS_PFN(res->start), resource_size(res));
pgmap_array_delete(res);
dev_WARN_ONCE(dev, pgmap->altmap.alloc,
"%s: failed to free all reserved pages\n", __func__);
@@ -162,13 +157,12 @@ static void dev_pagemap_percpu_release(s
*/
void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
{
- resource_size_t align_start, align_size, align_end;
struct resource *res = &pgmap->res;
struct dev_pagemap *conflict_pgmap;
struct mhp_restrictions restrictions = {
/*
* We do not want any optional features only our own memmap
- */
+ */
.altmap = pgmap_altmap(pgmap),
};
pgprot_t pgprot = PAGE_KERNEL;
@@ -225,12 +219,7 @@ void *devm_memremap_pages(struct device
return ERR_PTR(error);
}
- align_start = res->start & ~(SECTION_SIZE - 1);
- align_size = ALIGN(res->start + resource_size(res), SECTION_SIZE)
- - align_start;
- align_end = align_start + align_size - 1;
-
- conflict_pgmap = get_dev_pagemap(PHYS_PFN(align_start), NULL);
+ conflict_pgmap = get_dev_pagemap(PHYS_PFN(res->start), NULL);
if (conflict_pgmap) {
dev_WARN(dev, "Conflicting mapping in same section\n");
put_dev_pagemap(conflict_pgmap);
@@ -238,7 +227,7 @@ void *devm_memremap_pages(struct device
goto err_array;
}
- conflict_pgmap = get_dev_pagemap(PHYS_PFN(align_end), NULL);
+ conflict_pgmap = get_dev_pagemap(PHYS_PFN(res->end), NULL);
if (conflict_pgmap) {
dev_WARN(dev, "Conflicting mapping in same section\n");
put_dev_pagemap(conflict_pgmap);
@@ -246,7 +235,7 @@ void *devm_memremap_pages(struct device
goto err_array;
}
- is_ram = region_intersects(align_start, align_size,
+ is_ram = region_intersects(res->start, resource_size(res),
IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE);
if (is_ram != REGION_DISJOINT) {
@@ -267,8 +256,8 @@ void *devm_memremap_pages(struct device
if (nid < 0)
nid = numa_mem_id();
- error = track_pfn_remap(NULL, &pgprot, PHYS_PFN(align_start), 0,
- align_size);
+ error = track_pfn_remap(NULL, &pgprot, PHYS_PFN(res->start), 0,
+ resource_size(res));
if (error)
goto err_pfn_remap;
@@ -286,16 +275,16 @@ void *devm_memremap_pages(struct device
* arch_add_memory().
*/
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
- error = add_pages(nid, align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, &restrictions);
+ error = add_pages(nid, PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), &restrictions);
} else {
- error = kasan_add_zero_shadow(__va(align_start), align_size);
+ error = kasan_add_zero_shadow(__va(res->start), resource_size(res));
if (error) {
mem_hotplug_done();
goto err_kasan;
}
- error = arch_add_memory(nid, align_start, align_size,
+ error = arch_add_memory(nid, res->start, resource_size(res),
&restrictions);
}
@@ -303,8 +292,8 @@ void *devm_memremap_pages(struct device
struct zone *zone;
zone = &NODE_DATA(nid)->node_zones[ZONE_DEVICE];
- move_pfn_range_to_zone(zone, align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, pgmap_altmap(pgmap));
+ move_pfn_range_to_zone(zone, PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), restrictions.altmap);
}
mem_hotplug_done();
@@ -316,8 +305,8 @@ void *devm_memremap_pages(struct device
* to allow us to do the work while not holding the hotplug lock.
*/
memmap_init_zone_device(&NODE_DATA(nid)->node_zones[ZONE_DEVICE],
- align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, pgmap);
+ PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), pgmap);
percpu_ref_get_many(pgmap->ref, pfn_end(pgmap) - pfn_first(pgmap));
error = devm_add_action_or_reset(dev, devm_memremap_pages_release,
@@ -328,9 +317,9 @@ void *devm_memremap_pages(struct device
return __va(res->start);
err_add_memory:
- kasan_remove_zero_shadow(__va(align_start), align_size);
+ kasan_remove_zero_shadow(__va(res->start), resource_size(res));
err_kasan:
- untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
+ untrack_pfn(NULL, PHYS_PFN(res->start), resource_size(res));
err_pfn_remap:
pgmap_array_delete(res);
err_array:
_
On Sat, Jul 6, 2019 at 10:04 PM Andrew Morton <[email protected]> wrote:
>
> On Fri, 5 Jul 2019 12:08:15 +0000 Jason Gunthorpe <[email protected]> wrote:
>
> > On Thu, Jul 04, 2019 at 04:29:55PM -0700, Dan Williams wrote:
> > > Guys, Andrew has kicked the subsection patches out of -mm because of
> > > the merge conflicts. Can we hold off on the hmm cleanups for this
> > > cycle?
> >
> > I agree with you we should prioritize your subsection patches over
> > CH's cleanup if we cannot have both.
> >
> > As I said, I'll drop CH's at Andrews request, but I do not want to
> > make any changes without being aligned with him.
>
> OK, I had a shot at repairing the damage on top of current linux-next.
> The great majority of the issues were in
> mm-devm_memremap_pages-enable-sub-section-remap.patch.
>
> Below are the rejects which I saw and below that is my attempt to
> resolve it all. Dan, please go through this with a toothcomb. I've
> just done an mmotm release with all this in it so please do whatever's
> needed to verify that it's all working correctly.
Apologies for the delay, I was offline last week for a move. This
looks good to me and even caught some "X >> PAGE_SHIFT" to
"PHYS_PFN(X)" conversions that I missed. It also passes my testing.
Thank you accommodating a rebase.
Now to take a look at Oscar's fixes.