From: zhangchun <zhang.chuna@h3c.com>
To: <akpm@linux-foundation.org>
Cc: <jiaoxupo@h3c.com>, <linux-kernel@vger.kernel.org>,
<linux-mm@kvack.org>, <shaohaojize@126.com>,
<zhang.chuna@h3c.com>, <zhang.zhansheng@h3c.com>,
<zhang.zhengming@h3c.com>
Subject: [PATCH v3] mm: Give kmap_lock before call flush_tlb_kernel_rang,avoid kmap_high deadlock.
Date: Tue, 20 Aug 2024 00:10:06 +0800 [thread overview]
Message-ID: <1724083806-21956-1-git-send-email-zhang.chuna@h3c.com> (raw)
In-Reply-To: <20240723172609.7694b76e90bb18a0fd360ede@linux-foundation.org>
CPU 0: CPU 1:
kmap_high(){ kmap_xxx() {
... irq_disable();
spin_lock(&kmap_lock)
...
map_new_virtual ...
flush_all_zero_pkmaps
flush_tlb_kernel_range /* CPU0 holds the kmap_lock */
smp_call_function_many spin_lock(&kmap_lock)
... ....
spin_unlock(&kmap_lock)
...
CPU 0 holds the kmap_lock, waiting for CPU 1 respond to IPI. But CPU 1 has disabled irqs, waiting for kmap_lock,
cannot answer the IPI. Fix this by releasing kmap_lock before call flush_tlb_kernel_range, avoid kmap_lock
deadlock. Like this:
if (need_flush) {
unlock_kmap();
flush_tlb_kernel_range(PKMAP_ADDR(0), PKMAP_ADDR(LAST_PKMAP));
lock_kmap();
}
Dropping the lock is safe. kmap_lock is used to protect pkmap_count, pkmap_page_table and last_pkmap_nr(static variable).
When call flush_tlb_kernel_range(PKMAP_ADDR(0),
PKMAP_ADDR(LAST_PKMAP)), flush_tlb_kernel_range will neither modify nor read these variables. Leave that data unprotected
here is safe.
map_new_virtual aims to find an usable entry pkmap_count[last_pkmap_nr]. When read and modify the pkmap_count[last_pkmap_nr],
the kmap_lock is not dropped.
"if (!pkmap_count[last_pkmap_nr])" determine pkmap_count[last_pkmap_nr] is usable or not. If unusable, try agin.
Furthermore, the value of static variable last_pkmap_nr is stored in a local variable last_pkmap_nr, when kmap_lock is acquired,
this is thread-safe.
In an extreme case, if Thread A and Thread B access the same last_pkmap_nr, Thread A calls function flush_tlb_kernel_range and
release the kmap_lock, and Thread B then acquires the kmap_lock and modifies the variable pkmap_count[last_pkmap_nr]. After
Thread A completes the execution of function flush_tlb_kernel_range, it will check the variable pkmap_count[last_pkmap_nr].
static inline unsigned long map_new_virtual(struct page *page)
{
unsigned long vaddr;
int count;
unsigned int last_pkmap_nr; // local variable to store static variable last_pkmap_nr
unsigned int color = get_pkmap_color(page);
start:
...
flush_all_zero_pkmaps();// release kmap_lock, then acquire it
count = get_pkmap_entries_count(color);
}
...
if (!pkmap_count[last_pkmap_nr]) // pkmap_count[last_pkmap_nr] is used or not
break; /* Found a usable entry */
if (--count)
continue;
...
vaddr = PKMAP_ADDR(last_pkmap_nr);
set_pte_at(&init_mm, vaddr,
&(pkmap_page_table[last_pkmap_nr]), mk_pte(page, kmap_prot));
pkmap_count[last_pkmap_nr] = 1;
...
return vaddr;
}
Fixes: 3297e760776a ("highmem: atomic highmem kmap page pinning")
Signed-off-by: zhangchun <zhang.chuna@h3c.com>
Co-developed-by: zhangzhansheng <zhang.zhansheng@h3c.com>
Signed-off-by: zhangzhansheng <zhang.zhansheng@h3c.com>
Suggested-by: Matthew Wilcox <willy@infradead.org>
Reviewed-by: zhangzhengming <zhang.zhengming@h3c.com>
---
mm/highmem.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/mm/highmem.c b/mm/highmem.c index ef3189b..07f2c67 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -231,8 +231,18 @@ static void flush_all_zero_pkmaps(void)
set_page_address(page, NULL);
need_flush = 1;
}
- if (need_flush)
+ if (need_flush) {
+ /*
+ * In multi-core system one CPU holds the kmap_lock, waiting
+ * for other CPUs respond to IPI. But other CPUS has disabled
+ * irqs, waiting for kmap_lock, cannot answer the IPI. Release
+ * kmap_lock before call flush_tlb_kernel_range, avoid kmap_lock
+ * deadlock.
+ */
+ unlock_kmap();
flush_tlb_kernel_range(PKMAP_ADDR(0), PKMAP_ADDR(LAST_PKMAP));
+ lock_kmap();
+ }
}
void __kmap_flush_unused(void)
--
1.8.3.1
next prev parent reply other threads:[~2024-08-19 17:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-10 12:20 [PATCH v2] mm: Give kmap_lock before call flush_tlb_kernel_rang,avoid kmap_high deadlock zhangchun
2024-07-10 17:36 ` Andrew Morton
2024-07-11 7:07 ` zhangchun
2024-07-11 21:13 ` Andrew Morton
2024-07-12 7:54 ` zhangchun
2024-07-18 16:18 ` zhangchun
2024-07-24 0:26 ` Andrew Morton
2024-08-19 16:10 ` zhangchun [this message]
2024-09-03 11:52 ` [PATCH v3] " zhangchun
2024-10-08 3:23 ` zhangchun
2024-10-14 7:41 ` zhangchun
2024-10-31 15:18 ` [PATCH v4] " zhangchun
2025-02-18 6:30 ` zhangchun
-- strict thread matches above, loose matches on Subject: below --
2024-07-29 12:13 [PATCH v3] " zhangchun
[not found] <1724083806-21956-1-git-send-email-akpm@linux-foundation.org>
2024-10-08 3:19 ` zhangchun
2024-10-08 3:20 ` zhangchun
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=1724083806-21956-1-git-send-email-zhang.chuna@h3c.com \
--to=zhang.chuna@h3c.com \
--cc=akpm@linux-foundation.org \
--cc=jiaoxupo@h3c.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=shaohaojize@126.com \
--cc=zhang.zhansheng@h3c.com \
--cc=zhang.zhengming@h3c.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).