From patchwork Mon Aug 21 09:13:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tong Tiangen X-Patchwork-Id: 13359283 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A09CEEE49A8 for ; Mon, 21 Aug 2023 09:13:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 167E08D0009; Mon, 21 Aug 2023 05:13:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1167E8D0001; Mon, 21 Aug 2023 05:13:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F1F808D0009; Mon, 21 Aug 2023 05:13:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id E19708D0001 for ; Mon, 21 Aug 2023 05:13:22 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id ADAC5140B66 for ; Mon, 21 Aug 2023 09:13:22 +0000 (UTC) X-FDA: 81147548244.25.B772212 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by imf15.hostedemail.com (Postfix) with ESMTP id 053E4A001B for ; Mon, 21 Aug 2023 09:13:19 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of tongtiangen@huawei.com designates 45.249.212.255 as permitted sender) smtp.mailfrom=tongtiangen@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692609200; a=rsa-sha256; cv=none; b=Y2+LztX9BJwTqcY8q3B8fZti+JSp8N1JvDJ8POQt6qgcFjUaLM9iONSSQ+ry5T6sc/Fj49 CGlksagPtKoIZvYH05aPt7SOkgrQjJKVKoP3ApxV1l7JNz9suvoI82xwPvoF/ZT+n3t0IR 4G6r6c0odkgXVfqqXTklWoMHnjH4Jjk= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of tongtiangen@huawei.com designates 45.249.212.255 as permitted sender) smtp.mailfrom=tongtiangen@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692609200; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:in-reply-to: references; bh=G+HNfSj/RKbDIZE8b7A2cIZb/ltY7CJc09beoqnE3nA=; b=O64HESUVTML7OVFmpPk1fy9CzGKQzM90OYJk1qoFgPp6yoDujopp9YcqDKVzECIUhXfUbq nJZA2QeMQRjmUohsweqTAk+5PJaJ/Nl+eDaIIvWH92gH3P3RkbT8ReAI0idZvhgIc8RqaC HICKWNC+YXkSvjAsQxOMhTaIi6GhnzM= Received: from kwepemm600017.china.huawei.com (unknown [172.30.72.57]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4RTmv81yWhz1L9FL; Mon, 21 Aug 2023 17:11:48 +0800 (CST) Received: from localhost.localdomain (10.175.112.125) by kwepemm600017.china.huawei.com (7.193.23.234) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Mon, 21 Aug 2023 17:13:14 +0800 From: Tong Tiangen To: Andrew Morton , Naoya Horiguchi , Miaohe Lin , CC: , , Tong Tiangen Subject: [PATCH v2] mm: memory-failure: use rcu lock instead of tasklist_lock when collect_procs() Date: Mon, 21 Aug 2023 17:13:12 +0800 Message-ID: <20230821091312.2034844-1-tongtiangen@huawei.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Originating-IP: [10.175.112.125] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemm600017.china.huawei.com (7.193.23.234) X-CFilter-Loop: Reflected X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 053E4A001B X-Stat-Signature: py4yyepgi1kmxunu9xx8prggobh3xeh8 X-HE-Tag: 1692609199-514709 X-HE-Meta: U2FsdGVkX1+IMoIa1qSweZbhjukMJ4pHW8bjUONo07tPoIDnrfE+Y4NNwhFyCamf8S1yAOdfDOZDsshF3Ih9OXqIl6nc0/EJpNpzjLKkKfmnymLGO4vm3yPrMOdu4u2me1uikihLnNaEs9RKVaK2MiiEN3QJ4YIqiZ20txck3f5XI1FRP1n/Jf/jA+dht8t5GgLqC51qKkRCfimfWjTobREWTdPw7yBIr6bEEmw3FkckPK0NiamS34dz6v5Myf0enUYOaISmi2KaQuC7SUNM4mdtdB0TKHv6NdZ67i718cuVF0g7tbBuohmjxdCBTNLWlKg7h6/Oq5cEODTs/Iz6t6jtG1e6wqyOCV4Q3bcs3fDtcfXIwz3UHYRfKuyUZ0MrujP4krhb2Lt4yy+OfIT3va3MNg39SHFeyDXrZcJP3zKOf2QdoRjlnMn9n2KDC7XeLZXQZ0kV673nmZU6s9IvY9v96OAeYDDRHv6JYmZOYp4qotAtbmiXclNRWDWMhqqMhAbZFRT6lX604Rag4v6tiY8QcafMZq0TGt8qr/KAE5P8gm+yjxfvF35z7cwVIc1DMr26Hwk3S/8Aogvi/6oGsp7R0COBcH2UVxl8ivo7fklvCxoL0kWGIUcKO0GX6WSWkFifMj0XHfRW2heaJZXxreMoMXul17j7N1vtAOKBxOQmaJgtg55gb+EeD1sOaA7qdLLFr0VtnLbkOKZgIJOZ5949LxTdVRbsTQBK1/SMZs/bpV6CAgiAHU/aBss/N+PqFbjVdAE0JEf28ueKTSbB8uTANk0aJgzEUN9kF9bU32zi3U42MjkbHAx9DfeIJNZNLRGZy6xRggMFkCUr8q1ETGdOyLPsGeKeyd5GHpDGZdJqTycS43vyYucBEJ2VTwIrl4KWfHDRDQpNklGYxWDqIOc5l28MZ5oB/uf32Tc6fUHQ2uOYcSZnjgZm4QBSv5hEDTqoYl0YxL1OtAgzNde 7cAojH03 ujMPC99yjopJyUhftgPCQUAfRQXl4M9UlNYUbDc63F7rwvmS5+LlVTF5FbPSMw0kevwexm6y6KgA0qFvHhyqnCWBpVcawNXv5HOPf7YhOdsbe11NQ9YdFoaDs7ecZXwiL/RaXs7qnVL9vufrSBhVcXi+KXiNbl0dQ/fjKNZ7BdlPx+tKWTdgIQUPjUOQtkraUSwk3A55YBX0jZEtf23Gk4fKMGF7EWwga4pc/jGTTYFwVfPKrVcmfeWr8iEsZNGXd/3TcPM7VUk0SKasTWAZkHxcuoQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: We found a softlock issue in our test, analyzed the logs, and found that the relevant CPU call trace as follows: CPU0: _do_fork -> copy_process() -> write_lock_irq(&tasklist_lock) //Disable irq,waiting for //tasklist_lock CPU1: wp_page_copy() ->pte_offset_map_lock() -> spin_lock(&page->ptl); //Hold page->ptl -> ptep_clear_flush() -> flush_tlb_others() ... -> smp_call_function_many() -> arch_send_call_function_ipi_mask() -> csd_lock_wait() //Waiting for other CPUs respond //IPI CPU2: collect_procs_anon() -> read_lock(&tasklist_lock) //Hold tasklist_lock ->for_each_process(tsk) -> page_mapped_in_vma() -> page_vma_mapped_walk() -> map_pte() ->spin_lock(&page->ptl) //Waiting for page->ptl We can see that CPU1 waiting for CPU0 respond IPI，CPU0 waiting for CPU2 unlock tasklist_lock, CPU2 waiting for CPU1 unlock page->ptl. As a result, softlockup is triggered. For collect_procs_anon(), we will not modify the tasklist, but only perform read traversal. Therefore, we can use rcu lock instead of spin lock tasklist_lock, from this, we can break the softlock chain above. The same logic can also be applied to: - collect_procs_file() - collect_procs_fsdax() - collect_procs_ksm() Signed-off-by: Tong Tiangen Acked-by: Naoya Horiguchi --- Since v1: - 1. According to Matthew's suggestion, only the comments of find_early_kill_thread() are modified, no need to hold the rcu lock. Changes since RFC[1]: - 1. According to Naoya's suggestion, modify the tasklist_lock in the comment about locking order in mm/filemap.c. - 2. According to Kefeng's suggestion, optimize the implementation of find_early_kill_thread() without functional changes. - 3. Modify the title description. [1] https://lore.kernel.org/lkml/20230815130154.1100779-1-tongtiangen@huawei.com/ --- mm/filemap.c | 3 --- mm/ksm.c | 4 ++-- mm/memory-failure.c | 16 ++++++++-------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 014b73eb96a1..dfade1ef1765 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -121,9 +121,6 @@ * bdi.wb->list_lock (zap_pte_range->set_page_dirty) * ->inode->i_lock (zap_pte_range->set_page_dirty) * ->private_lock (zap_pte_range->block_dirty_folio) - * - * ->i_mmap_rwsem - * ->tasklist_lock (memory_failure, collect_procs_ao) */ static void page_cache_delete(struct address_space *mapping, diff --git a/mm/ksm.c b/mm/ksm.c index 8d6aee05421d..981af9c72e7a 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -2925,7 +2925,7 @@ void collect_procs_ksm(struct page *page, struct list_head *to_kill, struct anon_vma *av = rmap_item->anon_vma; anon_vma_lock_read(av); - read_lock(&tasklist_lock); + rcu_read_lock(); for_each_process(tsk) { struct anon_vma_chain *vmac; unsigned long addr; @@ -2944,7 +2944,7 @@ void collect_procs_ksm(struct page *page, struct list_head *to_kill, } } } - read_unlock(&tasklist_lock); + rcu_read_unlock(); anon_vma_unlock_read(av); } } diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 7b01fffe7a79..4d6e43c88489 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -547,8 +547,8 @@ static void kill_procs(struct list_head *to_kill, int forcekill, bool fail, * on behalf of the thread group. Return task_struct of the (first found) * dedicated thread if found, and return NULL otherwise. * - * We already hold read_lock(&tasklist_lock) in the caller, so we don't - * have to call rcu_read_lock/unlock() in this function. + * We already hold rcu lock in the caller, so we don't have to call + * rcu_read_lock/unlock() in this function. */ static struct task_struct *find_early_kill_thread(struct task_struct *tsk) { @@ -609,7 +609,7 @@ static void collect_procs_anon(struct page *page, struct list_head *to_kill, return; pgoff = page_to_pgoff(page); - read_lock(&tasklist_lock); + rcu_read_lock(); for_each_process(tsk) { struct anon_vma_chain *vmac; struct task_struct *t = task_early_kill(tsk, force_early); @@ -626,7 +626,7 @@ static void collect_procs_anon(struct page *page, struct list_head *to_kill, add_to_kill_anon_file(t, page, vma, to_kill); } } - read_unlock(&tasklist_lock); + rcu_read_unlock(); anon_vma_unlock_read(av); } @@ -642,7 +642,7 @@ static void collect_procs_file(struct page *page, struct list_head *to_kill, pgoff_t pgoff; i_mmap_lock_read(mapping); - read_lock(&tasklist_lock); + rcu_read_lock(); pgoff = page_to_pgoff(page); for_each_process(tsk) { struct task_struct *t = task_early_kill(tsk, force_early); @@ -662,7 +662,7 @@ static void collect_procs_file(struct page *page, struct list_head *to_kill, add_to_kill_anon_file(t, page, vma, to_kill); } } - read_unlock(&tasklist_lock); + rcu_read_unlock(); i_mmap_unlock_read(mapping); } @@ -685,7 +685,7 @@ static void collect_procs_fsdax(struct page *page, struct task_struct *tsk; i_mmap_lock_read(mapping); - read_lock(&tasklist_lock); + rcu_read_lock(); for_each_process(tsk) { struct task_struct *t = task_early_kill(tsk, true); @@ -696,7 +696,7 @@ static void collect_procs_fsdax(struct page *page, add_to_kill_fsdax(t, page, vma, to_kill, pgoff); } } - read_unlock(&tasklist_lock); + rcu_read_unlock(); i_mmap_unlock_read(mapping); } #endif /* CONFIG_FS_DAX */