Message ID | 20240120065729.3276395-1-linmiaohe@huawei.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-31725-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:2bc4:b0:101:a8e8:374 with SMTP id hx4csp1477085dyb; Fri, 19 Jan 2024 22:58:16 -0800 (PST) X-Google-Smtp-Source: AGHT+IE02BCT32AVhqNI9ynYQpWxHa64K7MuOYGaUfunRXj7P02agK0vsK4xVf1MOGSakoyt9K1L X-Received: by 2002:a05:6e02:683:b0:361:a16b:ec4c with SMTP id o3-20020a056e02068300b00361a16bec4cmr1033675ils.124.1705733895766; Fri, 19 Jan 2024 22:58:15 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1705733895; cv=pass; d=google.com; s=arc-20160816; b=H64qgh2l3ideMNa3PTMNPG4z2hiRrtRV9CDtE9uC3emx0mxd0cE+juaM2Lc/ATBh4O imWg2NVEFZGZGDKfCWhPYEW4ehZVsXFsjidy12peX+XgSBX8oTdRmekHiRorGbCGM7i5 Gml35TW8DLicVaLPht8jVckFP9XIb5xULlHCdBDgtMDpwyAnpgl7ux8Sww85gdk9uIQG 6JIPWjy7mglQvnxAKgm8/x+jtbpDoLFNLegpQipOOURriQvoPtsHEIjep4r+j5J09IRE 254voZ7S2tDULpXkvsw9I5PuUe+uKwM5ihap2DPkcuaHpVp2vvvYXZBcQ8jyR4+w071z 0asg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from; bh=EVEE3EgTe4kOOWptawfLoXU6j9AoIRMi9wHn0wKIPmk=; fh=fwuY/9LtMwVBvGnsIpQ7PRbt/VMNbnlzpnIuAaf4+CU=; b=h326EK/1mjtEj6Y35Wqn1D7YDqEVT2O+xd1LDoumjAOx5+9Dle9ub5OwVGAq0APzzp qOYnIpKV1Z0fQhuL53iGfNwzh1GGdE7LFdqNXGmf4/gEEo2S2r3RgqmbKgliqFD1NZlA soMkCyUgyP12cEbsgp7I3HFhhPCodM9IdTzAjHGnkmtMYvAQT+Y3vb/dyWtcpcno08T6 7I0peYSzegXPvtTs7a1oAUMbyjW/9k5R+fSnZ4dnw1NawPdhRdAqV4OMveExjY+XL7H3 sWRgQEE8ZaKjzaDbq+P7ZWtA1J8MGxXAnVUuNLhBDl3WKPYvGSMXF3eCWFMDqk1nWwfK 64jQ== ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=huawei.com dmarc=pass fromdomain=huawei.com); spf=pass (google.com: domain of linux-kernel+bounces-31725-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-31725-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id d9-20020a633609000000b005cf2fd55ee2si4646619pga.324.2024.01.19.22.58.15 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Jan 2024 22:58:15 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-31725-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=huawei.com dmarc=pass fromdomain=huawei.com); spf=pass (google.com: domain of linux-kernel+bounces-31725-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-31725-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 82FFE285025 for <ouuuleilei@gmail.com>; Sat, 20 Jan 2024 06:58:15 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 6000DC2D6; Sat, 20 Jan 2024 06:58:01 +0000 (UTC) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2EA9C441F for <linux-kernel@vger.kernel.org>; Sat, 20 Jan 2024 06:57:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.188 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705733879; cv=none; b=MAUQ0N2OyV5DnvtUhGMZNSgMquud8/uC/vPXD6YjPr+E4Mll2LV6Ytqwn8LSJ+/bgINJ0mjcNYDIgj+CtXfcMEQgnTJAlX2Rx/pyjytfV2VOkMs/0aEYD9FxQ95cHk0pMo3spsRju11ch1OaqDw3asynYoAWKExqv4isyMkw5So= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705733879; c=relaxed/simple; bh=qhRyBx7He8vFVNLN50FLAxF5oIDaEeIlcjCOnIzlThU=; h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type; b=mvJZcFmLcTEGCPGykdFqpfXcW8I89GWXAjpUwBSkGBFFhdydMbNexxfor6JyeG47LWJ0mcQHmhBy5f6Uy+zn6KKSLAOm2z4tV1rvFMuOasW1iM/sWmIJazu7xDM34k+X1gZ+y/Hvj2rb4O003TruH3VOYIbFUSFegJBPrwB4+9A= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.188 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.162.254]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4TH6jz06rgzGpnH; Sat, 20 Jan 2024 14:57:27 +0800 (CST) Received: from canpemm500002.china.huawei.com (unknown [7.192.104.244]) by mail.maildlp.com (Postfix) with ESMTPS id 285AA18001C; Sat, 20 Jan 2024 14:57:48 +0800 (CST) Received: from huawei.com (10.173.135.154) by canpemm500002.china.huawei.com (7.192.104.244) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Sat, 20 Jan 2024 14:57:47 +0800 From: Miaohe Lin <linmiaohe@huawei.com> To: <naoya.horiguchi@nec.com>, <akpm@linux-foundation.org> CC: <linmiaohe@huawei.com>, <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org> Subject: [PATCH] mm/memory-failure: fix crash in split_huge_page_to_list from soft_offline_page Date: Sat, 20 Jan 2024 14:57:29 +0800 Message-ID: <20240120065729.3276395-1-linmiaohe@huawei.com> X-Mailer: git-send-email 2.33.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To canpemm500002.china.huawei.com (7.192.104.244) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1788591625581029588 X-GMAIL-MSGID: 1788591625581029588 |
Series |
mm/memory-failure: fix crash in split_huge_page_to_list from soft_offline_page
|
|
Commit Message
Miaohe Lin
Jan. 20, 2024, 6:57 a.m. UTC
When I did soft offline stress test, a machine was observed to crash with
the following message:
kernel BUG at include/linux/memcontrol.h:554!
invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
CPU: 5 PID: 3837 Comm: hwpoison.sh Not tainted 6.7.0-next-20240112-00001-g8ecf3e7fb7c8-dirty #97
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
RIP: 0010:folio_memcg+0xaf/0xd0
Code: 10 5b 5d c3 cc cc cc cc 48 c7 c6 08 b1 f2 b2 48 89 ef e8 b4 c5 f8 ff 90 0f 0b 48 c7 c6 d0 b0 f2 b2 48 89 ef e8 a2 c5 f8 ff 90 <0f> 0b 48 c7 c6 08 b1 f2 b2 48 89 ef e8 90 c5 f8 ff 90 0f 0b 66 66
RSP: 0018:ffffb6c043657c98 EFLAGS: 00000296
RAX: 000000000000004b RBX: ffff932bc1d1e401 RCX: ffff933abfb5c908
RDX: 0000000000000000 RSI: 0000000000000027 RDI: ffff933abfb5c900
RBP: ffffea6f04019080 R08: ffffffffb3338ce8 R09: 0000000000009ffb
R10: 00000000000004dd R11: ffffffffb3308d00 R12: ffffea6f04019080
R13: ffffea6f04019080 R14: 0000000000000001 R15: ffffb6c043657da0
FS: 00007f6c60f6b740(0000) GS:ffff933abfb40000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000559c3bc8b980 CR3: 0000000107f1c000 CR4: 00000000000006f0
Call Trace:
<TASK>
? die+0x32/0x90
? do_trap+0xde/0x110
? folio_memcg+0xaf/0xd0
? do_error_trap+0x60/0x80
? folio_memcg+0xaf/0xd0
? exc_invalid_op+0x53/0x70
? folio_memcg+0xaf/0xd0
? asm_exc_invalid_op+0x1a/0x20
? folio_memcg+0xaf/0xd0
? folio_memcg+0xae/0xd0
split_huge_page_to_list+0x4d/0x1380
? sysvec_apic_timer_interrupt+0xf/0x80
try_to_split_thp_page+0x3a/0xf0
soft_offline_page+0x1ea/0x8a0
soft_offline_page_store+0x52/0x90
kernfs_fop_write_iter+0x118/0x1b0
vfs_write+0x30b/0x430
ksys_write+0x5e/0xe0
do_syscall_64+0xb0/0x1b0
entry_SYSCALL_64_after_hwframe+0x6d/0x75
RIP: 0033:0x7f6c60d14697
Code: 10 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
RSP: 002b:00007ffe9b72b8d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 000000000000000c RCX: 00007f6c60d14697
RDX: 000000000000000c RSI: 0000559c3bc8b980 RDI: 0000000000000001
RBP: 0000559c3bc8b980 R08: 00007f6c60dd1460 R09: 000000007fffffff
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000000000c
R13: 00007f6c60e1a780 R14: 00007f6c60e16600 R15: 00007f6c60e15a00
The problem is that page->mapping is overloaded with slab->slab_list or
slabs fields now, so slab pages could be taken as non-LRU movable pages
if field slabs contains PAGE_MAPPING_MOVABLE or slab_list->prev is set
to LIST_POISON2. These slab pages will be treated as thp later leading
to crash in split_huge_page_to_list().
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Fixes: 130d4df57390 ("mm/sl[au]b: rearrange struct slab fields to allow larger rcu_head")
---
mm/memory-failure.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
Comments
On Sat, Jan 20, 2024 at 02:57:29PM +0800, Miaohe Lin wrote: > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 636280d04008..20058f7ac3e9 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1377,8 +1377,13 @@ void ClearPageHWPoisonTakenOff(struct page *page) > */ > static inline bool HWPoisonHandlable(struct page *page, unsigned long flags) > { > - /* Soft offline could migrate non-LRU movable pages */ > - if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page)) > + /* > + * Soft offline could migrate non-LRU movable pages. > + * Note that page->mapping is overloaded with slab->slab_list or slabs > + * fields which might make slab pages appear like non-LRU movable pages. > + * So __PageMovable() has to be done after PageSlab() is checked. > + */ > + if ((flags & MF_SOFT_OFFLINE) && !PageSlab(page) && __PageMovable(page)) > return true; > > return PageLRU(page) || is_free_buddy_page(page); I think would make more sense as + if (PageSlab(page)) + return false; .. and then leave the rest alone (including not touching the comment)
On 2024/1/21 10:00, Matthew Wilcox wrote: > On Sat, Jan 20, 2024 at 02:57:29PM +0800, Miaohe Lin wrote: >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >> index 636280d04008..20058f7ac3e9 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -1377,8 +1377,13 @@ void ClearPageHWPoisonTakenOff(struct page *page) >> */ >> static inline bool HWPoisonHandlable(struct page *page, unsigned long flags) >> { >> - /* Soft offline could migrate non-LRU movable pages */ >> - if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page)) >> + /* >> + * Soft offline could migrate non-LRU movable pages. >> + * Note that page->mapping is overloaded with slab->slab_list or slabs >> + * fields which might make slab pages appear like non-LRU movable pages. >> + * So __PageMovable() has to be done after PageSlab() is checked. >> + */ >> + if ((flags & MF_SOFT_OFFLINE) && !PageSlab(page) && __PageMovable(page)) >> return true; >> >> return PageLRU(page) || is_free_buddy_page(page); > > I think would make more sense as > > + if (PageSlab(page)) > + return false; Do you mean add PageSlab check above "if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page))" block so we don't need to add more comment? > > ... and then leave the rest alone (including not touching the comment)> . I have a concern that __PageMovable() seems unreliable now if we access page from random context. This might introduce some potential problems. For example, offline_pages() might be stumped with such pages without any progress until signal occurs IIUC: offline_pages .. do { scan_movable_pages if (__PageMovable(page)) -- It might be slab page here. ret will also be set to 0. goto found; do_migrate_range -- Failed to isolate slab page and retry. } while (!ret) -- retry since ret is 0. There might be many similar scenes, but I haven't taken them more closely. Maybe these are just dumb problems... Thanks.
On Mon, Jan 22, 2024 at 08:57:06PM +0800, Miaohe Lin wrote: > On 2024/1/21 10:00, Matthew Wilcox wrote: > > On Sat, Jan 20, 2024 at 02:57:29PM +0800, Miaohe Lin wrote: > >> { > >> - /* Soft offline could migrate non-LRU movable pages */ > >> - if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page)) > >> + /* > >> + * Soft offline could migrate non-LRU movable pages. > >> + * Note that page->mapping is overloaded with slab->slab_list or slabs > >> + * fields which might make slab pages appear like non-LRU movable pages. > >> + * So __PageMovable() has to be done after PageSlab() is checked. > >> + */ > >> + if ((flags & MF_SOFT_OFFLINE) && !PageSlab(page) && __PageMovable(page)) > >> return true; > >> > >> return PageLRU(page) || is_free_buddy_page(page); > > > > I think would make more sense as > > > > + if (PageSlab(page)) > > + return false; > > Do you mean add PageSlab check above "if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page))" block > so we don't need to add more comment? Yes, although not just that we don't need to add a comment. Fundamentally, if you see PageSlab, you don't need to test anything else, you know it's not migratable. > I have a concern that __PageMovable() seems unreliable now if we access page from random context. > This might introduce some potential problems. For example, offline_pages() might be stumped with > such pages without any progress until signal occurs IIUC: > > offline_pages > .. > do { > scan_movable_pages > if (__PageMovable(page)) -- It might be slab page here. ret will also be set to 0. > goto found; > do_migrate_range -- Failed to isolate slab page and retry. > } while (!ret) -- retry since ret is 0. > > There might be many similar scenes, but I haven't taken them more closely. Maybe these are > just dumb problems... Yep, lots of places are insufficiently careful about testing PageMovable. This will get fixed with memdescs, but we're a fair way from having memdescs ...
On 2024/1/22 22:36, Matthew Wilcox wrote: > On Mon, Jan 22, 2024 at 08:57:06PM +0800, Miaohe Lin wrote: >> On 2024/1/21 10:00, Matthew Wilcox wrote: >>> On Sat, Jan 20, 2024 at 02:57:29PM +0800, Miaohe Lin wrote: >>>> { >>>> - /* Soft offline could migrate non-LRU movable pages */ >>>> - if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page)) >>>> + /* >>>> + * Soft offline could migrate non-LRU movable pages. >>>> + * Note that page->mapping is overloaded with slab->slab_list or slabs >>>> + * fields which might make slab pages appear like non-LRU movable pages. >>>> + * So __PageMovable() has to be done after PageSlab() is checked. >>>> + */ >>>> + if ((flags & MF_SOFT_OFFLINE) && !PageSlab(page) && __PageMovable(page)) >>>> return true; >>>> >>>> return PageLRU(page) || is_free_buddy_page(page); >>> >>> I think would make more sense as >>> >>> + if (PageSlab(page)) >>> + return false; >> >> Do you mean add PageSlab check above "if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page))" block >> so we don't need to add more comment? > > Yes, although not just that we don't need to add a comment. > Fundamentally, if you see PageSlab, you don't need to test anything > else, you know it's not migratable. Yes, this really makes sense. > >> I have a concern that __PageMovable() seems unreliable now if we access page from random context. >> This might introduce some potential problems. For example, offline_pages() might be stumped with >> such pages without any progress until signal occurs IIUC: >> >> offline_pages >> .. >> do { >> scan_movable_pages >> if (__PageMovable(page)) -- It might be slab page here. ret will also be set to 0. >> goto found; >> do_migrate_range -- Failed to isolate slab page and retry. >> } while (!ret) -- retry since ret is 0. >> >> There might be many similar scenes, but I haven't taken them more closely. Maybe these are >> just dumb problems... > > Yep, lots of places are insufficiently careful about testing > PageMovable. This will get fixed with memdescs, but we're a fair way > from having memdescs ... I believe memdescs will fix all these mess, but we might need to fix them before memdescs becoming ready as a compromise. Thanks. > > . >
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 636280d04008..20058f7ac3e9 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1377,8 +1377,13 @@ void ClearPageHWPoisonTakenOff(struct page *page) */ static inline bool HWPoisonHandlable(struct page *page, unsigned long flags) { - /* Soft offline could migrate non-LRU movable pages */ - if ((flags & MF_SOFT_OFFLINE) && __PageMovable(page)) + /* + * Soft offline could migrate non-LRU movable pages. + * Note that page->mapping is overloaded with slab->slab_list or slabs + * fields which might make slab pages appear like non-LRU movable pages. + * So __PageMovable() has to be done after PageSlab() is checked. + */ + if ((flags & MF_SOFT_OFFLINE) && !PageSlab(page) && __PageMovable(page)) return true; return PageLRU(page) || is_free_buddy_page(page);