2018-07-02 15:46:01

by Oscar Salvador

[permalink] [raw]
Subject: [PATCH] mm/sparse: Make sparse_init_one_section void and remove check

From: Oscar Salvador <[email protected]>

sparse_init_one_section() is being called from two sites:
sparse_init() and sparse_add_one_section().
The former calls it from a for_each_present_section_nr() loop,
and the latter marks the section as present before calling it.
This means that when sparse_init_one_section() gets called, we already know
that the section is present.
So there is no point to double check that in the function.

This removes the check and makes the function void.

Signed-off-by: Oscar Salvador <[email protected]>
---
mm/sparse.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index b2848cc6e32a..f55e79fda03e 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -264,19 +264,14 @@ struct page *sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long pn
return ((struct page *)coded_mem_map) + section_nr_to_pfn(pnum);
}

-static int __meminit sparse_init_one_section(struct mem_section *ms,
+static void __meminit sparse_init_one_section(struct mem_section *ms,
unsigned long pnum, struct page *mem_map,
unsigned long *pageblock_bitmap)
{
- if (!present_section(ms))
- return -EINVAL;
-
ms->section_mem_map &= ~SECTION_MAP_MASK;
ms->section_mem_map |= sparse_encode_mem_map(mem_map, pnum) |
SECTION_HAS_MEM_MAP;
ms->pageblock_flags = pageblock_bitmap;
-
- return 1;
}

unsigned long usemap_size(void)
@@ -801,12 +796,11 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
#endif

section_mark_present(ms);
-
- ret = sparse_init_one_section(ms, section_nr, memmap, usemap);
+ sparse_init_one_section(ms, section_nr, memmap, usemap);

out:
pgdat_resize_unlock(pgdat, &flags);
- if (ret <= 0) {
+ if (ret < 0) {
kfree(usemap);
__kfree_section_memmap(memmap, altmap);
}
--
2.13.6



2018-07-02 16:06:57

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH] mm/sparse: Make sparse_init_one_section void and remove check

On Mon 02-07-18 17:43:25, [email protected] wrote:
> From: Oscar Salvador <[email protected]>
>
> sparse_init_one_section() is being called from two sites:
> sparse_init() and sparse_add_one_section().
> The former calls it from a for_each_present_section_nr() loop,
> and the latter marks the section as present before calling it.
> This means that when sparse_init_one_section() gets called, we already know
> that the section is present.
> So there is no point to double check that in the function.
>
> This removes the check and makes the function void.

Looks good.

> Signed-off-by: Oscar Salvador <[email protected]>

Acked-by: Michal Hocko <[email protected]>

> ---
> mm/sparse.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index b2848cc6e32a..f55e79fda03e 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -264,19 +264,14 @@ struct page *sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long pn
> return ((struct page *)coded_mem_map) + section_nr_to_pfn(pnum);
> }
>
> -static int __meminit sparse_init_one_section(struct mem_section *ms,
> +static void __meminit sparse_init_one_section(struct mem_section *ms,
> unsigned long pnum, struct page *mem_map,
> unsigned long *pageblock_bitmap)
> {
> - if (!present_section(ms))
> - return -EINVAL;
> -
> ms->section_mem_map &= ~SECTION_MAP_MASK;
> ms->section_mem_map |= sparse_encode_mem_map(mem_map, pnum) |
> SECTION_HAS_MEM_MAP;
> ms->pageblock_flags = pageblock_bitmap;
> -
> - return 1;
> }
>
> unsigned long usemap_size(void)
> @@ -801,12 +796,11 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
> #endif
>
> section_mark_present(ms);
> -
> - ret = sparse_init_one_section(ms, section_nr, memmap, usemap);
> + sparse_init_one_section(ms, section_nr, memmap, usemap);
>
> out:
> pgdat_resize_unlock(pgdat, &flags);
> - if (ret <= 0) {
> + if (ret < 0) {
> kfree(usemap);
> __kfree_section_memmap(memmap, altmap);
> }
> --
> 2.13.6
>

--
Michal Hocko
SUSE Labs

2018-07-02 18:48:55

by Pavel Tatashin

[permalink] [raw]
Subject: Re: [PATCH] mm/sparse: Make sparse_init_one_section void and remove check

On Mon, Jul 2, 2018 at 11:43 AM <[email protected]> wrote:
>
> From: Oscar Salvador <[email protected]>
>
> sparse_init_one_section() is being called from two sites:
> sparse_init() and sparse_add_one_section().
> The former calls it from a for_each_present_section_nr() loop,
> and the latter marks the section as present before calling it.
> This means that when sparse_init_one_section() gets called, we already know
> that the section is present.
> So there is no point to double check that in the function.
>
> This removes the check and makes the function void.
>
> Signed-off-by: Oscar Salvador <[email protected]>

Thank you Oscar.

Reviewed-by: Pavel Tatashin <[email protected]>

> ---
> mm/sparse.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)

2018-07-06 18:25:14

by Ross Zwisler

[permalink] [raw]
Subject: Re: [PATCH] mm/sparse: Make sparse_init_one_section void and remove check

On Mon, Jul 2, 2018 at 12:48 PM Pavel Tatashin
<[email protected]> wrote:
>
> On Mon, Jul 2, 2018 at 11:43 AM <[email protected]> wrote:
> >
> > From: Oscar Salvador <[email protected]>
> >
> > sparse_init_one_section() is being called from two sites:
> > sparse_init() and sparse_add_one_section().
> > The former calls it from a for_each_present_section_nr() loop,
> > and the latter marks the section as present before calling it.
> > This means that when sparse_init_one_section() gets called, we already know
> > that the section is present.
> > So there is no point to double check that in the function.
> >
> > This removes the check and makes the function void.
> >
> > Signed-off-by: Oscar Salvador <[email protected]>
>
> Thank you Oscar.
>
> Reviewed-by: Pavel Tatashin <[email protected]>

It looks like this change breaks "fsdax" mode namespaces in
next-20180705. The offending commit is:

commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void
and remove check")

Here is the stack trace I get when converting a raw mode namespace to
fsdax mode, and from then on during each reboot as the namespace is
being initialized:

[ 6.067166] BUG: unable to handle kernel paging request at ffffea0005000080
[ 6.068084] PGD 13ffdd067 P4D 13ffdd067 PUD 13ffdc067 PMD 0
[ 6.068771] Oops: 0002 [#1] PREEMPT SMP PTI
[ 6.069262] CPU: 11 PID: 180 Comm: kworker/u24:2 Not tainted
4.18.0-rc3-00193-g054620849110 #1
[ 6.070440] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.11.1-0-g0551a4be2c-prebuilt.qemu-project.org 04/01/2014
[ 6.071689] Workqueue: events_unbound async_run_entry_fn
[ 6.072261] RIP: 0010:memmap_init_zone+0x154/0x1cf
[ 6.072882] Code: 48 89 c3 48 c1 eb 0c e9 82 00 00 00 48 89 da 48
b8 00 00 00 00 00 ea ff ff b9 10 00 00 00 48 c1 e2 06 48 01 c2 31 c0
48 89 d7 <f3> ab 48 b8 ff ff ff ff ff ff 7f 00 48 23 45 c0 c7 42 34 01
00 00
[ 6.075396] RSP: 0018:ffffc900024bfa70 EFLAGS: 00010246
[ 6.076052] RAX: 0000000000000000 RBX: 0000000000140002 RCX: 0000000000000010
[ 6.076845] RDX: ffffea0005000080 RSI: 0000000000000000 RDI: ffffea0005000080
[ 6.077604] RBP: ffffc900024bfab0 R08: 0000000000000001 R09: ffff88010eb50d38
[ 6.078394] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 6.079331] R13: 0000000000000004 R14: 0000000000000001 R15: 0000000000000000
[ 6.080274] FS: 0000000000000000(0000) GS:ffff880115a00000(0000)
knlGS:0000000000000000
[ 6.081337] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 6.082092] CR2: ffffea0005000080 CR3: 0000000002824000 CR4: 00000000000006e0
[ 6.083032] Call Trace:
[ 6.083371] move_pfn_range_to_zone+0x168/0x180
[ 6.083965] devm_memremap_pages+0x29b/0x480
[ 6.084550] pmem_attach_disk+0x1ae/0x6c0 [nd_pmem]
[ 6.085204] ? devm_memremap+0x79/0xb0
[ 6.085714] nd_pmem_probe+0x7e/0xa0 [nd_pmem]
[ 6.086320] nvdimm_bus_probe+0x6e/0x160 [libnvdimm]
[ 6.086977] driver_probe_device+0x310/0x480
[ 6.087543] __device_attach_driver+0x86/0x100
[ 6.088136] ? __driver_attach+0x110/0x110
[ 6.088681] bus_for_each_drv+0x6e/0xb0
[ 6.089190] __device_attach+0xe2/0x160
[ 6.089705] device_initial_probe+0x13/0x20
[ 6.090266] bus_probe_device+0xa6/0xc0
[ 6.090772] device_add+0x41b/0x660
[ 6.091249] ? lock_acquire+0xa3/0x210
[ 6.091743] nd_async_device_register+0x12/0x40 [libnvdimm]
[ 6.092398] async_run_entry_fn+0x3e/0x170
[ 6.092921] process_one_work+0x230/0x680
[ 6.093455] worker_thread+0x3f/0x3b0
[ 6.093930] kthread+0x12f/0x150
[ 6.094362] ? process_one_work+0x680/0x680
[ 6.094903] ? kthread_create_worker_on_cpu+0x70/0x70
[ 6.095574] ret_from_fork+0x3a/0x50
[ 6.096069] Modules linked in: nd_pmem nd_btt dax_pmem device_dax
nfit libnvdimm
[ 6.097179] CR2: ffffea0005000080
[ 6.097764] ---[ end trace a5b8bd6a5500b68c ]---

- Ross

2018-07-06 19:08:44

by Ross Zwisler

[permalink] [raw]
Subject: [PATCH] mm/sparse.c: fix error path in sparse_add_one_section

The following commit in -next:

commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
remove check")

changed how the error handling in sparse_add_one_section() works.

Previously sparse_index_init() could return -EEXIST, and the function would
continue on happily. 'ret' would get unconditionally overwritten by the
result from sparse_init_one_section() and the error code after the 'out:'
label wouldn't be triggered.

With the above referenced commit, though, an -EEXIST error return from
sparse_index_init() now takes us through the function and into the error
case after 'out:'. This eventually causes a kernel BUG, probably because
we've just freed a memory section that we successfully set up and marked as
present:

BUG: unable to handle kernel paging request at ffffea0005000080
RIP: 0010:memmap_init_zone+0x154/0x1cf

Call Trace:
move_pfn_range_to_zone+0x168/0x180
devm_memremap_pages+0x29b/0x480
pmem_attach_disk+0x1ae/0x6c0 [nd_pmem]
? devm_memremap+0x79/0xb0
nd_pmem_probe+0x7e/0xa0 [nd_pmem]
nvdimm_bus_probe+0x6e/0x160 [libnvdimm]
driver_probe_device+0x310/0x480
__device_attach_driver+0x86/0x100
? __driver_attach+0x110/0x110
bus_for_each_drv+0x6e/0xb0
__device_attach+0xe2/0x160
device_initial_probe+0x13/0x20
bus_probe_device+0xa6/0xc0
device_add+0x41b/0x660
? lock_acquire+0xa3/0x210
nd_async_device_register+0x12/0x40 [libnvdimm]
async_run_entry_fn+0x3e/0x170
process_one_work+0x230/0x680
worker_thread+0x3f/0x3b0
kthread+0x12f/0x150
? process_one_work+0x680/0x680
? kthread_create_worker_on_cpu+0x70/0x70
ret_from_fork+0x3a/0x50

Fix this by clearing 'ret' back to 0 if sparse_index_init() returns
-EEXIST. This restores the previous behavior.

Signed-off-by: Ross Zwisler <[email protected]>
---
mm/sparse.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index 9574113fc745..d254bd2d3289 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -753,8 +753,12 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
* plus, it does a kmalloc
*/
ret = sparse_index_init(section_nr, pgdat->node_id);
- if (ret < 0 && ret != -EEXIST)
- return ret;
+ if (ret < 0) {
+ if (ret == -EEXIST)
+ ret = 0;
+ else
+ return ret;
+ }
memmap = kmalloc_section_memmap(section_nr, pgdat->node_id, altmap);
if (!memmap)
return -ENOMEM;
--
2.14.4


2018-07-06 21:24:37

by Oscar Salvador

[permalink] [raw]
Subject: Re: [PATCH] mm/sparse.c: fix error path in sparse_add_one_section

On Fri, Jul 06, 2018 at 01:06:58PM -0600, Ross Zwisler wrote:
> The following commit in -next:
>
> commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
> remove check")
>
> changed how the error handling in sparse_add_one_section() works.
>
> Previously sparse_index_init() could return -EEXIST, and the function would
> continue on happily. 'ret' would get unconditionally overwritten by the
> result from sparse_init_one_section() and the error code after the 'out:'
> label wouldn't be triggered.

My bad, I missed that.

> diff --git a/mm/sparse.c b/mm/sparse.c
> index 9574113fc745..d254bd2d3289 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -753,8 +753,12 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
> * plus, it does a kmalloc
> */
> ret = sparse_index_init(section_nr, pgdat->node_id);
> - if (ret < 0 && ret != -EEXIST)
> - return ret;
> + if (ret < 0) {
> + if (ret == -EEXIST)
> + ret = 0;
> + else
> + return ret;
> + }

sparse_index_init() can return:

-ENOMEM, -EEXIST or 0.

So what about this?:

diff --git a/mm/sparse.c b/mm/sparse.c
index f55e79fda03e..eb188eb6b82d 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -770,6 +770,7 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
ret = sparse_index_init(section_nr, pgdat->node_id);
if (ret < 0 && ret != -EEXIST)
return ret;
+ ret = 0;

Does this look more clean?
--
Oscar Salvador
SUSE L3

2018-07-06 21:33:30

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] mm/sparse.c: fix error path in sparse_add_one_section

On Fri, 6 Jul 2018 13:06:58 -0600 Ross Zwisler <[email protected]> wrote:

> commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
> remove check")
>
> changed how the error handling in sparse_add_one_section() works.
>
> Previously sparse_index_init() could return -EEXIST, and the function would
> continue on happily. 'ret' would get unconditionally overwritten by the
> result from sparse_init_one_section() and the error code after the 'out:'
> label wouldn't be triggered.
>
> With the above referenced commit, though, an -EEXIST error return from
> sparse_index_init() now takes us through the function and into the error
> case after 'out:'. This eventually causes a kernel BUG, probably because
> we've just freed a memory section that we successfully set up and marked as
> present:

Thanks.

And gee it would be nice if some of this code was commented. I
*assume* what's happening with that -EEXIST is that
sparse_add_one_section() is discovering that the root mem_section was
already initialized so things are OK. Maybe. My mind-reading skills
aren't so good on Fridays.

And sparse_index_init() sure looks like it needs locking to avoid races
around mem_section[root]. Or perhaps we're known to be single-threaded
here.

2018-07-06 21:56:06

by Ross Zwisler

[permalink] [raw]
Subject: Re: [PATCH] mm/sparse.c: fix error path in sparse_add_one_section

On Fri, Jul 06, 2018 at 11:23:27PM +0200, Oscar Salvador wrote:
> On Fri, Jul 06, 2018 at 01:06:58PM -0600, Ross Zwisler wrote:
> > The following commit in -next:
> >
> > commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
> > remove check")
> >
> > changed how the error handling in sparse_add_one_section() works.
> >
> > Previously sparse_index_init() could return -EEXIST, and the function would
> > continue on happily. 'ret' would get unconditionally overwritten by the
> > result from sparse_init_one_section() and the error code after the 'out:'
> > label wouldn't be triggered.
>
> My bad, I missed that.
>
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index 9574113fc745..d254bd2d3289 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -753,8 +753,12 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
> > * plus, it does a kmalloc
> > */
> > ret = sparse_index_init(section_nr, pgdat->node_id);
> > - if (ret < 0 && ret != -EEXIST)
> > - return ret;
> > + if (ret < 0) {
> > + if (ret == -EEXIST)
> > + ret = 0;
> > + else
> > + return ret;
> > + }
>
> sparse_index_init() can return:
>
> -ENOMEM, -EEXIST or 0.
>
> So what about this?:
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index f55e79fda03e..eb188eb6b82d 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -770,6 +770,7 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
> ret = sparse_index_init(section_nr, pgdat->node_id);
> if (ret < 0 && ret != -EEXIST)
> return ret;
> + ret = 0;
>
> Does this look more clean?

Sure, that's probably better.

Andrew, what's the easiest way forward? I can send out a v2, you can fold
this into his previous patch, or something else?

2018-07-06 22:00:45

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] mm/sparse.c: fix error path in sparse_add_one_section

On Fri, 6 Jul 2018 15:54:37 -0600 Ross Zwisler <[email protected]> wrote:

> On Fri, Jul 06, 2018 at 11:23:27PM +0200, Oscar Salvador wrote:
> > On Fri, Jul 06, 2018 at 01:06:58PM -0600, Ross Zwisler wrote:
> > > The following commit in -next:
> > >
> > > commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
> > > remove check")
> > >
> > > changed how the error handling in sparse_add_one_section() works.
> > >
> > > Previously sparse_index_init() could return -EEXIST, and the function would
> > > continue on happily. 'ret' would get unconditionally overwritten by the
> > > result from sparse_init_one_section() and the error code after the 'out:'
> > > label wouldn't be triggered.
> >
> > My bad, I missed that.
> >
> > > diff --git a/mm/sparse.c b/mm/sparse.c
> > > index 9574113fc745..d254bd2d3289 100644
> > > --- a/mm/sparse.c
> > > +++ b/mm/sparse.c
> > > @@ -753,8 +753,12 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
> > > * plus, it does a kmalloc
> > > */
> > > ret = sparse_index_init(section_nr, pgdat->node_id);
> > > - if (ret < 0 && ret != -EEXIST)
> > > - return ret;
> > > + if (ret < 0) {
> > > + if (ret == -EEXIST)
> > > + ret = 0;
> > > + else
> > > + return ret;
> > > + }
> >
> > sparse_index_init() can return:
> >
> > -ENOMEM, -EEXIST or 0.
> >
> > So what about this?:
> >
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index f55e79fda03e..eb188eb6b82d 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -770,6 +770,7 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
> > ret = sparse_index_init(section_nr, pgdat->node_id);
> > if (ret < 0 && ret != -EEXIST)
> > return ret;
> > + ret = 0;
> >
> > Does this look more clean?
>
> Sure, that's probably better.
>
> Andrew, what's the easiest way forward? I can send out a v2, you can fold
> this into his previous patch, or something else?

Whatever ;) v2 works.

2018-07-06 22:35:17

by Ross Zwisler

[permalink] [raw]
Subject: [PATCH v2] mm/sparse.c: fix error path in sparse_add_one_section

The following commit in -next:

commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
remove check")

changed how the error handling in sparse_add_one_section() works.

Previously sparse_index_init() could return -EEXIST, and the function would
continue on happily. 'ret' would get unconditionally overwritten by the
result from sparse_init_one_section() and the error code after the 'out:'
label wouldn't be triggered.

With the above referenced commit, though, an -EEXIST error return from
sparse_index_init() now takes us through the function and into the error
case after 'out:'. This eventually causes a kernel BUG, probably because
we've just freed a memory section that we successfully set up and marked as
present:

BUG: unable to handle kernel paging request at ffffea0005000080
RIP: 0010:memmap_init_zone+0x154/0x1cf

Call Trace:
move_pfn_range_to_zone+0x168/0x180
devm_memremap_pages+0x29b/0x480
pmem_attach_disk+0x1ae/0x6c0 [nd_pmem]
? devm_memremap+0x79/0xb0
nd_pmem_probe+0x7e/0xa0 [nd_pmem]
nvdimm_bus_probe+0x6e/0x160 [libnvdimm]
driver_probe_device+0x310/0x480
__device_attach_driver+0x86/0x100
? __driver_attach+0x110/0x110
bus_for_each_drv+0x6e/0xb0
__device_attach+0xe2/0x160
device_initial_probe+0x13/0x20
bus_probe_device+0xa6/0xc0
device_add+0x41b/0x660
? lock_acquire+0xa3/0x210
nd_async_device_register+0x12/0x40 [libnvdimm]
async_run_entry_fn+0x3e/0x170
process_one_work+0x230/0x680
worker_thread+0x3f/0x3b0
kthread+0x12f/0x150
? process_one_work+0x680/0x680
? kthread_create_worker_on_cpu+0x70/0x70
ret_from_fork+0x3a/0x50

Fix this by clearing 'ret' back to 0 if sparse_index_init() returns
-EEXIST. This restores the previous behavior.

Signed-off-by: Ross Zwisler <[email protected]>
---
mm/sparse.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/mm/sparse.c b/mm/sparse.c
index f55e79fda03e..eb188eb6b82d 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -770,6 +770,7 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat,
ret = sparse_index_init(section_nr, pgdat->node_id);
if (ret < 0 && ret != -EEXIST)
return ret;
+ ret = 0;
memmap = kmalloc_section_memmap(section_nr, pgdat->node_id, altmap);
if (!memmap)
return -ENOMEM;
--
2.14.4


2018-07-07 06:02:55

by Oscar Salvador

[permalink] [raw]
Subject: Re: [PATCH v2] mm/sparse.c: fix error path in sparse_add_one_section

On Fri, Jul 06, 2018 at 04:33:58PM -0600, Ross Zwisler wrote:
> The following commit in -next:
>
> commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and
> remove check")
>
> changed how the error handling in sparse_add_one_section() works.
>
> Previously sparse_index_init() could return -EEXIST, and the function would
> continue on happily. 'ret' would get unconditionally overwritten by the
> result from sparse_init_one_section() and the error code after the 'out:'
> label wouldn't be triggered.
>
> With the above referenced commit, though, an -EEXIST error return from
> sparse_index_init() now takes us through the function and into the error
> case after 'out:'. This eventually causes a kernel BUG, probably because
> we've just freed a memory section that we successfully set up and marked as
> present:
>
> BUG: unable to handle kernel paging request at ffffea0005000080
> RIP: 0010:memmap_init_zone+0x154/0x1cf
>
> Call Trace:
> move_pfn_range_to_zone+0x168/0x180
> devm_memremap_pages+0x29b/0x480
> pmem_attach_disk+0x1ae/0x6c0 [nd_pmem]
> ? devm_memremap+0x79/0xb0
> nd_pmem_probe+0x7e/0xa0 [nd_pmem]
> nvdimm_bus_probe+0x6e/0x160 [libnvdimm]
> driver_probe_device+0x310/0x480
> __device_attach_driver+0x86/0x100
> ? __driver_attach+0x110/0x110
> bus_for_each_drv+0x6e/0xb0
> __device_attach+0xe2/0x160
> device_initial_probe+0x13/0x20
> bus_probe_device+0xa6/0xc0
> device_add+0x41b/0x660
> ? lock_acquire+0xa3/0x210
> nd_async_device_register+0x12/0x40 [libnvdimm]
> async_run_entry_fn+0x3e/0x170
> process_one_work+0x230/0x680
> worker_thread+0x3f/0x3b0
> kthread+0x12f/0x150
> ? process_one_work+0x680/0x680
> ? kthread_create_worker_on_cpu+0x70/0x70
> ret_from_fork+0x3a/0x50
>
> Fix this by clearing 'ret' back to 0 if sparse_index_init() returns
> -EEXIST. This restores the previous behavior.
>
> Signed-off-by: Ross Zwisler <[email protected]>

Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE L3