From: Ross Zwisler <ross.zwisler@linux.intel.com> To: pasha.tatashin@oracle.com, linux-nvdimm@lists.01.org Cc: osalvador@techadventures.net, bhe@redhat.com, Dave Hansen <dave.hansen@linux.intel.com>, LKML <linux-kernel@vger.kernel.org>, Linux MM <linux-mm@kvack.org>, Michal Hocko <mhocko@suse.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Andrew Morton <akpm@linux-foundation.org>, Vlastimil Babka <vbabka@suse.cz>, osalvador@suse.de Subject: [PATCH v2] mm/sparse.c: fix error path in sparse_add_one_section Date: Fri, 6 Jul 2018 16:33:58 -0600 [thread overview] Message-ID: <20180706223358.742-1-ross.zwisler@linux.intel.com> (raw) In-Reply-To: <20180706190658.6873-1-ross.zwisler@linux.intel.com> 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 <ross.zwisler@linux.intel.com> --- 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 _______________________________________________ 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: Ross Zwisler <ross.zwisler@linux.intel.com> To: pasha.tatashin@oracle.com, linux-nvdimm@lists.01.org Cc: Ross Zwisler <ross.zwisler@linux.intel.com>, osalvador@techadventures.net, bhe@redhat.com, Dave Hansen <dave.hansen@linux.intel.com>, LKML <linux-kernel@vger.kernel.org>, Linux MM <linux-mm@kvack.org>, Michal Hocko <mhocko@suse.com>, Vlastimil Babka <vbabka@suse.cz>, Andrew Morton <akpm@linux-foundation.org>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, osalvador@suse.de Subject: [PATCH v2] mm/sparse.c: fix error path in sparse_add_one_section Date: Fri, 6 Jul 2018 16:33:58 -0600 [thread overview] Message-ID: <20180706223358.742-1-ross.zwisler@linux.intel.com> (raw) In-Reply-To: <20180706190658.6873-1-ross.zwisler@linux.intel.com> 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 <ross.zwisler@linux.intel.com> --- 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
next prev parent reply other threads:[~2018-07-06 22:34 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-02 15:43 [PATCH] mm/sparse: Make sparse_init_one_section void and remove check osalvador 2018-07-02 16:05 ` Michal Hocko 2018-07-02 18:47 ` Pavel Tatashin 2018-07-06 18:23 ` Ross Zwisler 2018-07-06 18:23 ` Ross Zwisler 2018-07-06 19:06 ` [PATCH] mm/sparse.c: fix error path in sparse_add_one_section Ross Zwisler 2018-07-06 19:06 ` Ross Zwisler 2018-07-06 21:23 ` Oscar Salvador 2018-07-06 21:23 ` Oscar Salvador 2018-07-06 21:54 ` Ross Zwisler 2018-07-06 21:54 ` Ross Zwisler 2018-07-06 21:58 ` Andrew Morton 2018-07-06 21:58 ` Andrew Morton 2018-07-06 21:32 ` Andrew Morton 2018-07-06 21:32 ` Andrew Morton 2018-07-06 22:33 ` Ross Zwisler [this message] 2018-07-06 22:33 ` [PATCH v2] " Ross Zwisler 2018-07-07 6:01 ` Oscar Salvador 2018-07-07 6:01 ` Oscar Salvador
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=20180706223358.742-1-ross.zwisler@linux.intel.com \ --to=ross.zwisler@linux.intel.com \ --cc=akpm@linux-foundation.org \ --cc=bhe@redhat.com \ --cc=dave.hansen@linux.intel.com \ --cc=kirill.shutemov@linux.intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-nvdimm@lists.01.org \ --cc=mhocko@suse.com \ --cc=osalvador@suse.de \ --cc=osalvador@techadventures.net \ --cc=pasha.tatashin@oracle.com \ --cc=vbabka@suse.cz \ /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.