- Linux-stable-mirror - lists.linaro.org

Re: [Linux-stable-mirror] 4.14: WARNING: CPU: 4 PID: 2895 at block/blk-mq.c:1144 with virtio-blk (also 4.12 stable)

by Christian Borntraeger

On 11/21/2017 09:14 PM, Jens Axboe wrote: > On 11/21/2017 01:12 PM, Christian Borntraeger wrote: >> >> >> On 11/21/2017 08:30 PM, Jens Axboe wrote: >>> On 11/21/2017 12:15 PM, Christian Borntraeger wrote: >>>> >>>> >>>> On 11/21/2017 07:39 PM, Jens Axboe wrote: >>>>> On 11/21/2017 11:27 AM, Jens Axboe wrote: >>>>>> On 11/21/2017 11:12 AM, Christian Borntraeger wrote: >>>>>>> >>>>>>> >>>>>>> On 11/21/2017 07:09 PM, Jens Axboe wrote: >>>>>>>> On 11/21/2017 10:27 AM, Jens Axboe wrote: >>>>>>>>> On 11/21/2017 03:14 AM, Christian Borntraeger wrote: >>>>>>>>>> Bisect points to >>>>>>>>>> >>>>>>>>>> 1b5a7455d345b223d3a4658a9e5fce985b7998c1 is the first bad commit >>>>>>>>>> commit 1b5a7455d345b223d3a4658a9e5fce985b7998c1 >>>>>>>>>> Author: Christoph Hellwig <hch(a)lst.de> >>>>>>>>>> Date: Mon Jun 26 12:20:57 2017 +0200 >>>>>>>>>> >>>>>>>>>> blk-mq: Create hctx for each present CPU >>>>>>>>>> >>>>>>>>>> commit 4b855ad37194f7bdbb200ce7a1c7051fecb56a08 upstream. >>>>>>>>>> >>>>>>>>>> Currently we only create hctx for online CPUs, which can lead to a lot >>>>>>>>>> of churn due to frequent soft offline / online operations. Instead >>>>>>>>>> allocate one for each present CPU to avoid this and dramatically simplify >>>>>>>>>> the code. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Christoph Hellwig <hch(a)lst.de> >>>>>>>>>> Reviewed-by: Jens Axboe <axboe(a)kernel.dk> >>>>>>>>>> Cc: Keith Busch <keith.busch(a)intel.com> >>>>>>>>>> Cc: linux-block(a)vger.kernel.org >>>>>>>>>> Cc: linux-nvme(a)lists.infradead.org >>>>>>>>>> Link: http://lkml.kernel.org/r/20170626102058.10200-3-hch@lst.de >>>>>>>>>> Signed-off-by: Thomas Gleixner <tglx(a)linutronix.de> >>>>>>>>>> Cc: Oleksandr Natalenko <oleksandr(a)natalenko.name> >>>>>>>>>> Cc: Mike Galbraith <efault(a)gmx.de> >>>>>>>>>> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> >>>>>>>>> >>>>>>>>> I wonder if we're simply not getting the masks updated correctly. I'll >>>>>>>>> take a look. >>>>>>>> >>>>>>>> Can't make it trigger here. We do init for each present CPU, which means >>>>>>>> that if I offline a few CPUs here and register a queue, those still show >>>>>>>> up as present (just offline) and get mapped accordingly. >>>>>>>> >>>>>>>> From the looks of it, your setup is different. If the CPU doesn't show >>>>>>>> up as present and it gets hotplugged, then I can see how this condition >>>>>>>> would trigger. What environment are you running this in? We might have >>>>>>>> to re-introduce the cpu hotplug notifier, right now we just monitor >>>>>>>> for a dead cpu and handle that. >>>>>>> >>>>>>> I am not doing a hot unplug and the replug, I use KVM and add a previously >>>>>>> not available CPU. >>>>>>> >>>>>>> in libvirt/virsh speak: >>>>>>> <vcpu placement='static' current='1'>4</vcpu> >>>>>> >>>>>> So that's why we run into problems. It's not present when we load the device, >>>>>> but becomes present and online afterwards. >>>>>> >>>>>> Christoph, we used to handle this just fine, your patch broke it. >>>>>> >>>>>> I'll see if I can come up with an appropriate fix. >>>>> >>>>> Can you try the below? >>>> >>>> >>>> It does prevent the crash but it seems that the new CPU is not "used " after the hotplug for mq: >>>> >>>> >>>> output with 2 cpus: >>>> /sys/kernel/debug/block/vda >>>> /sys/kernel/debug/block/vda/hctx0 >>>> /sys/kernel/debug/block/vda/hctx0/cpu0 >>>> /sys/kernel/debug/block/vda/hctx0/cpu0/completed >>>> /sys/kernel/debug/block/vda/hctx0/cpu0/merged >>>> /sys/kernel/debug/block/vda/hctx0/cpu0/dispatched >>>> /sys/kernel/debug/block/vda/hctx0/cpu0/rq_list >>>> /sys/kernel/debug/block/vda/hctx0/active >>>> /sys/kernel/debug/block/vda/hctx0/run >>>> /sys/kernel/debug/block/vda/hctx0/queued >>>> /sys/kernel/debug/block/vda/hctx0/dispatched >>>> /sys/kernel/debug/block/vda/hctx0/io_poll >>>> /sys/kernel/debug/block/vda/hctx0/sched_tags_bitmap >>>> /sys/kernel/debug/block/vda/hctx0/sched_tags >>>> /sys/kernel/debug/block/vda/hctx0/tags_bitmap >>>> /sys/kernel/debug/block/vda/hctx0/tags >>>> /sys/kernel/debug/block/vda/hctx0/ctx_map >>>> /sys/kernel/debug/block/vda/hctx0/busy >>>> /sys/kernel/debug/block/vda/hctx0/dispatch >>>> /sys/kernel/debug/block/vda/hctx0/flags >>>> /sys/kernel/debug/block/vda/hctx0/state >>>> /sys/kernel/debug/block/vda/sched >>>> /sys/kernel/debug/block/vda/sched/dispatch >>>> /sys/kernel/debug/block/vda/sched/starved >>>> /sys/kernel/debug/block/vda/sched/batching >>>> /sys/kernel/debug/block/vda/sched/write_next_rq >>>> /sys/kernel/debug/block/vda/sched/write_fifo_list >>>> /sys/kernel/debug/block/vda/sched/read_next_rq >>>> /sys/kernel/debug/block/vda/sched/read_fifo_list >>>> /sys/kernel/debug/block/vda/write_hints >>>> /sys/kernel/debug/block/vda/state >>>> /sys/kernel/debug/block/vda/requeue_list >>>> /sys/kernel/debug/block/vda/poll_stat >>> >>> Try this, basically just a revert. >> >> Yes, seems to work. >> >> Tested-by: Christian Borntraeger <borntraeger(a)de.ibm.com> > > Great, thanks for testing. > >> Do you know why the original commit made it into 4.12 stable? After all >> it has no Fixes tag and no cc stable- > > I was wondering the same thing when you said it was in 4.12.stable and > not in 4.12 release. That patch should absolutely not have gone into > stable, it's not marked as such and it's not fixing a problem that is > stable worthy. In fact, it's causing a regression... > > Greg? Upstream commit is mentioned higher up, start of the email. > Forgot to cc Greg?

7 years, 2 months

3
5
0 0

[Linux-stable-mirror] Autoselect patches for stable (Was: Re: [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV)

by Emil Velikov

Hi all, Since I'm going slightly off-topic, I've tweaked the subject line and trimmed some of the conversation. I believe everyone in the CC list might be interested in the following, yet feel free to adjust. Above all, I'd kindly ask everyone to skim through and draw their conclusions. If the ideas put forward have some value - great, if not - let my email rot. On 17 November 2017 at 13:57, Greg KH <gregkh(a)linuxfoundation.org> wrote: >> >> I still have no idea how this autoselect picks up patches that do *not* >> have cc: stable nor Fixes: from us. What information do you have that we >> don't for making that call? > > I'll let Sasha describe how he's doing this, but in the end, does it > really matter _how_ it is done, vs. the fact that it seems to at least > one human reviewer that this is a patch that _should_ be included based > on the changelog text and the code patch? > > By having this review process that Sasha is providing, he's saying > "Here's a patch that I think might be good for stable, do you object?" > > If you do, great, no harm done, all is fine, the patch is dropped. If > you don't object, just ignore the email and the patch gets merged. > > If you don't want any of this to happen for your subsystem at all, then > also fine, just let us know and we will ignore it entirely. > Let me start with saying that I'm handling the releases for Mesa 3D - the project providing OpenGL, Vulkan and many other userspace graphics drivers. I've been doing that for 3 years now, which admittedly is quite a short time relative to the kernel. There is a procedure quite similar to the kernel, with a few differences - see below for details. We also autoselect patches, hence my interest in the heuristics applied for nominating patches ;-) That aside, here are some things I've learned from my experience. Some of those may not be applicable - hope you'll find them useful: - Try to reference developers to existing documentation/procedure. Was just reminded that even long standing developers can forget detail X or Y. - CC developers for the important stuff - aka do not CC on each accepted patch. Accepted patches are merged in pre-release branch and a email with accepted/deferred/rejected list is sent. Patches that had conflicts merging, and ones that are rejected have their author in the CC list. Rejected patches have brief description + developers are contacted beforehand. - Autoselect patches are merged only with the approval from the respective developers. IMHO this engages developers into the process, without distracting them too much. It is by no means a perfect system - input and changes are always appreciated. That said, here are some suggestions which should make autosel smoother: - Document the autoselect process Information about about What, Why, and [ideally] How - analogous to the normal stable nominations. Insert reference to the process in the patch notification email. - Make the autoselect nominations _more_ distinct than the normal stable ones. Maintainers will want to put more cognitive effort into the patches. HTH Emil

7 years, 2 months

7
13
0 0

[Linux-stable-mirror] Patch "mm: check the return value of lookup_page_ext for all call sites" has been added to the 4.4-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled mm: check the return value of lookup_page_ext for all call sites to the 4.4-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: mm-check-the-return-value-of-lookup_page_ext-for-all-call-sites.patch and it can be found in the queue-4.4 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. >From f86e4271978bd93db466d6a95dad4b0fdcdb04f6 Mon Sep 17 00:00:00 2001 From: Yang Shi <yang.shi(a)linaro.org> Date: Fri, 3 Jun 2016 14:55:38 -0700 Subject: mm: check the return value of lookup_page_ext for all call sites From: Yang Shi <yang.shi(a)linaro.org> commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. Per the discussion with Joonsoo Kim [1], we need check the return value of lookup_page_ext() for all call sites since it might return NULL in some cases, although it is unlikely, i.e. memory hotplug. Tested with ltp with "page_owner=0". [1] http://lkml.kernel.org/r/20160519002809.GA10245@js1304-P5Q-DELUXE [akpm(a)linux-foundation.org: fix build-breaking typos] [arnd(a)arndb.de: fix build problems from lookup_page_ext] Link: http://lkml.kernel.org/r/6285269.2CksypHdYp@wuerfel [akpm(a)linux-foundation.org: coding-style fixes] Link: http://lkml.kernel.org/r/1464023768-31025-1-git-send-email-yang.shi@linaro.… Signed-off-by: Yang Shi <yang.shi(a)linaro.org> Signed-off-by: Arnd Bergmann <arnd(a)arndb.de> Cc: Joonsoo Kim <iamjoonsoo.kim(a)lge.com> Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds(a)linux-foundation.org> Signed-off-by: Michal Hocko <mhocko(a)suse.com> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- include/linux/page_idle.h | 43 ++++++++++++++++++++++++++++++++++++------- mm/debug-pagealloc.c | 6 ++++++ mm/page_alloc.c | 6 ++++++ mm/page_owner.c | 16 ++++++++++++++++ mm/vmstat.c | 2 ++ 5 files changed, 66 insertions(+), 7 deletions(-) --- a/include/linux/page_idle.h +++ b/include/linux/page_idle.h @@ -46,33 +46,62 @@ extern struct page_ext_operations page_i static inline bool page_is_young(struct page *page) { - return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline void set_page_young(struct page *page) { - set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + set_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline bool test_and_clear_page_young(struct page *page) { - return test_and_clear_bit(PAGE_EXT_YOUNG, - &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_and_clear_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline bool page_is_idle(struct page *page) { - return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_bit(PAGE_EXT_IDLE, &page_ext->flags); } static inline void set_page_idle(struct page *page) { - set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + set_bit(PAGE_EXT_IDLE, &page_ext->flags); } static inline void clear_page_idle(struct page *page) { - clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + clear_bit(PAGE_EXT_IDLE, &page_ext->flags); } #endif /* CONFIG_64BIT */ --- a/mm/debug-pagealloc.c +++ b/mm/debug-pagealloc.c @@ -34,6 +34,8 @@ static inline void set_page_poison(struc struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (page_ext) + return; __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -42,6 +44,8 @@ static inline void clear_page_poison(str struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (page_ext) + return; __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -50,6 +54,8 @@ static inline bool page_poison(struct pa struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (page_ext) + return false; return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -569,6 +569,9 @@ static inline void set_page_guard(struct return; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + return; + __set_bit(PAGE_EXT_DEBUG_GUARD, &page_ext->flags); INIT_LIST_HEAD(&page->lru); @@ -586,6 +589,9 @@ static inline void clear_page_guard(stru return; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + return; + __clear_bit(PAGE_EXT_DEBUG_GUARD, &page_ext->flags); set_page_private(page, 0); --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -53,6 +53,8 @@ void __reset_page_owner(struct page *pag for (i = 0; i < (1 << order); i++) { page_ext = lookup_page_ext(page + i); + if (unlikely(!page_ext)) + continue; __clear_bit(PAGE_EXT_OWNER, &page_ext->flags); } } @@ -60,6 +62,7 @@ void __reset_page_owner(struct page *pag void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) { struct page_ext *page_ext = lookup_page_ext(page); + struct stack_trace trace = { .nr_entries = 0, .max_entries = ARRAY_SIZE(page_ext->trace_entries), @@ -67,6 +70,9 @@ void __set_page_owner(struct page *page, .skip = 3, }; + if (unlikely(!page_ext)) + return; + save_stack_trace(&trace); page_ext->order = order; @@ -79,6 +85,12 @@ void __set_page_owner(struct page *page, gfp_t __get_page_owner_gfp(struct page *page) { struct page_ext *page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + /* + * The caller just returns 0 if no valid gfp + * So return 0 here too. + */ + return 0; return page_ext->gfp_mask; } @@ -194,6 +206,8 @@ read_page_owner(struct file *file, char } page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; /* * Some pages could be missed by concurrent allocation or free, @@ -257,6 +271,8 @@ static void init_pages_in_zone(pg_data_t continue; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; /* Maybe overraping zone */ if (test_bit(PAGE_EXT_OWNER, &page_ext->flags)) --- a/mm/vmstat.c +++ b/mm/vmstat.c @@ -1091,6 +1091,8 @@ static void pagetypeinfo_showmixedcount_ continue; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags)) continue; Patches currently in stable-queue which might be from yang.shi(a)linaro.org are queue-4.4/mm-check-the-return-value-of-lookup_page_ext-for-all-call-sites.patch

7 years, 2 months

1
0
0 0

[Linux-stable-mirror] Patch "mm/page_ext.c: check if page_ext is not prepared" has been added to the 4.4-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled mm/page_ext.c: check if page_ext is not prepared to the 4.4-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: mm-page_ext.c-check-if-page_ext-is-not-prepared.patch and it can be found in the queue-4.4 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. >From e492080e640c2d1235ddf3441cae634cfffef7e1 Mon Sep 17 00:00:00 2001 From: Jaewon Kim <jaewon31.kim(a)samsung.com> Date: Wed, 15 Nov 2017 17:39:07 -0800 Subject: mm/page_ext.c: check if page_ext is not prepared From: Jaewon Kim <jaewon31.kim(a)samsung.com> commit e492080e640c2d1235ddf3441cae634cfffef7e1 upstream. online_page_ext() and page_ext_init() allocate page_ext for each section, but they do not allocate if the first PFN is !pfn_present(pfn) or !pfn_valid(pfn). Then section->page_ext remains as NULL. lookup_page_ext checks NULL only if CONFIG_DEBUG_VM is enabled. For a valid PFN, __set_page_owner will try to get page_ext through lookup_page_ext. Without CONFIG_DEBUG_VM lookup_page_ext will misuse NULL pointer as value 0. This incurrs invalid address access. This is the panic example when PFN 0x100000 is not valid but PFN 0x13FC00 is being used for page_ext. section->page_ext is NULL, get_entry returned invalid page_ext address as 0x1DFA000 for a PFN 0x13FC00. To avoid this panic, CONFIG_DEBUG_VM should be removed so that page_ext will be checked at all times. Unable to handle kernel paging request at virtual address 01dfa014 ------------[ cut here ]------------ Kernel BUG at ffffff80082371e0 [verbose debug info unavailable] Internal error: Oops: 96000045 [#1] PREEMPT SMP Modules linked in: PC is at __set_page_owner+0x48/0x78 LR is at __set_page_owner+0x44/0x78 __set_page_owner+0x48/0x78 get_page_from_freelist+0x880/0x8e8 __alloc_pages_nodemask+0x14c/0xc48 __do_page_cache_readahead+0xdc/0x264 filemap_fault+0x2ac/0x550 ext4_filemap_fault+0x3c/0x58 __do_fault+0x80/0x120 handle_mm_fault+0x704/0xbb0 do_page_fault+0x2e8/0x394 do_mem_abort+0x88/0x124 Pre-4.7 kernels also need commit f86e4271978b ("mm: check the return value of lookup_page_ext for all call sites"). Link: http://lkml.kernel.org/r/20171107094131.14621-1-jaewon31.kim@samsung.com Fixes: eefa864b701d ("mm/page_ext: resurrect struct page extending code for debugging") Signed-off-by: Jaewon Kim <jaewon31.kim(a)samsung.com> Acked-by: Michal Hocko <mhocko(a)suse.com> Cc: Vlastimil Babka <vbabka(a)suse.cz> Cc: Minchan Kim <minchan(a)kernel.org> Cc: Joonsoo Kim <js1304(a)gmail.com> Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds(a)linux-foundation.org> Signed-off-by: Michal Hocko <mhocko(a)suse.com> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- mm/page_ext.c | 4 ---- 1 file changed, 4 deletions(-) --- a/mm/page_ext.c +++ b/mm/page_ext.c @@ -106,7 +106,6 @@ struct page_ext *lookup_page_ext(struct struct page_ext *base; base = NODE_DATA(page_to_nid(page))->node_page_ext; -#ifdef CONFIG_DEBUG_VM /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -115,7 +114,6 @@ struct page_ext *lookup_page_ext(struct */ if (unlikely(!base)) return NULL; -#endif offset = pfn - round_down(node_start_pfn(page_to_nid(page)), MAX_ORDER_NR_PAGES); return base + offset; @@ -180,7 +178,6 @@ struct page_ext *lookup_page_ext(struct { unsigned long pfn = page_to_pfn(page); struct mem_section *section = __pfn_to_section(pfn); -#ifdef CONFIG_DEBUG_VM /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -189,7 +186,6 @@ struct page_ext *lookup_page_ext(struct */ if (!section->page_ext) return NULL; -#endif return section->page_ext + pfn; } Patches currently in stable-queue which might be from jaewon31.kim(a)samsung.com are queue-4.4/mm-page_ext.c-check-if-page_ext-is-not-prepared.patch

7 years, 2 months

1
0
0 0

[Linux-stable-mirror] [PATCH] mm: migrate: fix an incorrect call of prep_transhuge_page()

by Zi Yan

From: Zi Yan <zi.yan(a)cs.rutgers.edu> In [1], Andrea reported that during memory hotplug/hot remove prep_transhuge_page() is called incorrectly on non-THP pages for migration, when THP is on but THP migration is not enabled. This leads to a bad state of target pages for migration. This patch fixes it by only calling prep_transhuge_page() when we are certain that the target page is THP. [1] https://lkml.org/lkml/2017/11/20/411 Cc: stable(a)vger.kernel.org # v4.14 Fixes: 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration") Reported-by: Andrea Reale <ar(a)linux.vnet.ibm.com> Signed-off-by: Zi Yan <zi.yan(a)cs.rutgers.edu> Cc: Naoya Horiguchi <n-horiguchi(a)ah.jp.nec.com> Cc: "Jérôme Glisse" <jglisse(a)redhat.com> --- include/linux/migrate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/migrate.h b/include/linux/migrate.h index 895ec0c4942e..a2246cf670ba 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -54,7 +54,7 @@ static inline struct page *new_page_nodemask(struct page *page, new_page = __alloc_pages_nodemask(gfp_mask, order, preferred_nid, nodemask); - if (new_page && PageTransHuge(page)) + if (new_page && PageTransHuge(new_page)) prep_transhuge_page(new_page); return new_page; -- 2.14.2

7 years, 2 months

3
3
0 0

Re: [Linux-stable-mirror] [PATCH] mm: migrate: fix an incorrect call of prep_transhuge_page()

by Michal Hocko

On Wed 22-11-17 07:29:38, Zi Yan wrote: > On 22 Nov 2017, at 7:13, Zi Yan wrote: > > > On 22 Nov 2017, at 5:14, Michal Hocko wrote: > > > >> On Wed 22-11-17 10:35:10, Michal Hocko wrote: > >> [...] > >>> Moreover I am not really sure this is really working properly. Just look > >>> at the split_huge_page. It moves all the tail pages to the LRU list > >>> while migrate_pages has a list of pages to migrate. So we will migrate > >>> the head page and all the rest will get back to the LRU list. What > >>> guarantees that they will get migrated as well. > >> > >> OK, so this is as I've expected. It doesn't work! Some pfn walker based > >> migration will just skip tail pages see madvise_inject_error. > >> __alloc_contig_migrate_range will simply fail on THP page see > >> isolate_migratepages_block so we even do not try to migrate it. > >> do_move_page_to_node_array will simply migrate head and do not care > >> about tail pages. do_mbind splits the page and then fall back to pte > >> walk when thp migration is not supported but it doesn't handle tail > >> pages if the THP migration path is not able to allocate a fresh THP > >> AFAICS. Memory hotplug should be safe because it doesn't skip the whole > >> THP when doing pfn walk. > >> > >> Unless I am missing something here this looks like a huge mess to me. > > > > +Kirill > > > > First, I agree with you that splitting a THP and only migrating its head page > > is a mess. But what you describe is also the behavior of migrate_page() > > _before_ THP migration support is added. I thought that was intended. > > > > Look at http://elixir.free-electrons.com/linux/v4.13.15/source/mm/migrate.c#L1091, > > unmap_and_move() splits THPs and only migrates the head page in v4.13 before THP > > migration is added. I think the behavior was introduced since v4.5 (I just skimmed > > v4.0 to v4.13 code and did not have time to use git blame), before that THPs are > > not migrated but shown as successfully migrated (at least from v4.4’s code). > > Sorry, I misread v4.4’s code, it also does ‘splitting a THP and migrating its head page’. > This behavior was there for a long time, at least since v3.0. > > The code in unmap_and_move() is: > > if (unlikely(PageTransHuge(page))) > if (unlikely(split_huge_page(page))) > goto out; I _think_ that this all should be handled at migrate_pages layer. Try to migrate THP and fallback to split_huge_page into to the list when it fails. I haven't checked whether there is something which would prevent that though. THP tricks in specific paths then should be removed. -- Michal Hocko SUSE Labs

7 years, 2 months

1
0
0 0

[Linux-stable-mirror] [PATCH] mm/hugetlb: Fix NULL-pointer dereference on 5-level paging machine

by Kirill A. Shutemov

I've made mistake during converting hugetlb code to 5-level paging: in huge_pte_alloc() we have to use p4d_alloc(), not p4d_offset(). Otherwise it leads to crash -- NULL-pointer dereference in pud_alloc() if p4d table is not yet allocated. It only can happen in 5-level paging mode. In 4-level paging mode p4d_offset() always returns pgd, so we are fine. Signed-off-by: Kirill A. Shutemov <kirill.shutemov(a)linux.intel.com> Fixes: c2febafc6773 ("mm: convert generic code to 5-level paging") Cc: <stable(a)vger.kernel.org> # v4.11+ --- mm/hugetlb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 2d2ff5e8bf2b..94a4c0b63580 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -4617,7 +4617,9 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, pte_t *pte = NULL; pgd = pgd_offset(mm, addr); - p4d = p4d_offset(pgd, addr); + p4d = p4d_alloc(mm, pgd, addr); + if (!p4d) + return NULL; pud = pud_alloc(mm, p4d, addr); if (pud) { if (sz == PUD_SIZE) { -- 2.15.0

7 years, 2 months

3
2
0 0

[Linux-stable-mirror] early preview of -stable review branches

by Milosz Wasilewski

Hi, Is there a way to get notifications about patches queued up for the -rc1 commits in linux-stable-rc.git? I currently use the push that is associated with email to this list from Greg. But that only gives 48h window to do the building and testing. If there is a way to get the patches earlier, that would give more time for testing activities. Any hints are much appreciated. Best Regards, Milosz

7 years, 2 months

3
3
0 0

Re: [Linux-stable-mirror] [PATCH] mm: migrate: fix an incorrect call of prep_transhuge_page()

by Michal Hocko

On Wed 22-11-17 04:18:35, Zi Yan wrote: > On 22 Nov 2017, at 3:54, Michal Hocko wrote: [...] > > I would keep the two checks consistent. But that leads to a more > > interesting question. new_page_nodemask does > > > > if (thp_migration_supported() && PageTransHuge(page)) { > > order = HPAGE_PMD_ORDER; > > gfp_mask |= GFP_TRANSHUGE; > > } > > > > How come it is safe to allocate an order-0 page if > > !thp_migration_supported() when we are about to migrate THP? This > > doesn't make any sense to me. Are we working around this somewhere else? > > Why shouldn't we simply return NULL here? > > If !thp_migration_supported(), we will first split a THP and migrate > its head page. This process is done in unmap_and_move() after > get_new_page() (the function pointer to this new_page_nodemask()) is > called. The situation can be PageTransHuge(page) is true here, but the > page is split in unmap_and_move(), so we want to return a order-0 page > here. This deserves a big fat comment in the code because this is not clear from the code! > I think the confusion comes from that there is no guarantee of THP > allocation when we are doing THP migration. If we can allocate a THP > during THP migration, we are good. Otherwise, we want to fallback to > the old way, splitting the original THP and migrating the head page, > to preserve the original code behavior. I understand that but that should be done explicitly rather than relying on two functions doing the right thing because this is just too fragile. Moreover I am not really sure this is really working properly. Just look at the split_huge_page. It moves all the tail pages to the LRU list while migrate_pages has a list of pages to migrate. So we will migrate the head page and all the rest will get back to the LRU list. What guarantees that they will get migrated as well. This all looks like a mess! -- Michal Hocko SUSE Labs

7 years, 2 months

1
1
0 0

Re: [Linux-stable-mirror] [PATCH] mm: migrate: fix an incorrect call of prep_transhuge_page()

by Andrea Reale

On Tue 21 Nov 2017, 17:35, Zi Yan wrote: > On 21 Nov 2017, at 17:12, Andrew Morton wrote: > > > On Mon, 20 Nov 2017 21:18:55 -0500 Zi Yan <zi.yan(a)sent.com> wrote: > > > >> This patch fixes it by only calling prep_transhuge_page() when we are > >> certain that the target page is THP. > > > > What are the user-visible effects of the bug? > > By inspecting the code, if called on a non-THP, prep_transhuge_page() will > 1) change the value of the mapping of (page + 2), since it is used for THP deferred list; > 2) change the lru value of (page + 1), since it is used for THP’s dtor. > > Both can lead to data corruption of these two pages. Pragmatically and from the point of view of the memory_hotplug subsys, the effect is a kernel crash when pages are being migrated during a memory hot remove offline and migration target pages are found in a bad state. Best, Andrea

7 years, 2 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror