From mboxrd@z Thu Jan 1 00:00:00 1970 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 69282C3DA49 for ; Thu, 18 Jul 2024 16:17:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 00F166B0093; Thu, 18 Jul 2024 12:17:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED9F16B0095; Thu, 18 Jul 2024 12:17:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D7ABB6B0096; Thu, 18 Jul 2024 12:17:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id B58DC6B0093 for ; Thu, 18 Jul 2024 12:17:27 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 67DB1140EE4 for ; Thu, 18 Jul 2024 16:17:27 +0000 (UTC) X-FDA: 82353378534.03.79F2B5D Received: from h3cspam02-ex.h3c.com (smtp.h3c.com [60.191.123.50]) by imf14.hostedemail.com (Postfix) with ESMTP id 4EFC310001D for ; Thu, 18 Jul 2024 16:17:22 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of zhang.chunA@h3c.com designates 60.191.123.50 as permitted sender) smtp.mailfrom=zhang.chunA@h3c.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721319412; 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:in-reply-to:references:references; bh=x9yFdvvGI/btYJss5oHzV6mmuKMx+PjRZRjnP03Usl4=; b=twrUfg7OAmfxU5i03zeiiB5+50Cnnq1AXrKGgyPPwh4+aS1UGG3LZZfIfRK5E+PR9TmNoG UZoEJuHWI1rPTCWM3FPFCb6HURJsPivUN3d/ZV9xoMbk2ZZu1dOYuYcDR0zG0fKYCh/ZVd CdhLEceLSp64HTZLWaMi2hEz+eO077A= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of zhang.chunA@h3c.com designates 60.191.123.50 as permitted sender) smtp.mailfrom=zhang.chunA@h3c.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721319412; a=rsa-sha256; cv=none; b=gxbsDCEPB90E9EDsq/wYEqPc+wabZHiiS0/ciWE6qybFUIoIw+1L+ri4dc0Rukd9t/lBSk LO3mqmmpCkMJfEGn/CbDqkNUBJEWG9kB5h0C2TT0EsLpYXXHbwyONBgRZnHeCJv6BOUnPt Fx3DU7UFJYVTI0MZ+HCmIu9xqo0nv3U= Received: from mail.maildlp.com ([172.25.15.154]) by h3cspam02-ex.h3c.com with ESMTP id 46IGH1dk085127; Fri, 19 Jul 2024 00:17:01 +0800 (GMT-8) (envelope-from zhang.chunA@h3c.com) Received: from DAG6EX09-BJD.srv.huawei-3com.com (unknown [10.153.34.11]) by mail.maildlp.com (Postfix) with ESMTP id 9F4DB2004BAA; Fri, 19 Jul 2024 00:21:30 +0800 (CST) Received: from localhost.localdomain.com (10.99.206.13) by DAG6EX09-BJD.srv.huawei-3com.com (10.153.34.11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.2.1258.27; Fri, 19 Jul 2024 00:17:05 +0800 From: zhangchun To: CC: , , , , , , Subject: [PATCH v2] =?UTF-8?q?mm:=20Give=20kmap=5Flock=20before=20call=20flus?= =?UTF-8?q?h=5Ftlb=5Fkernel=5Frang=EF=BC=8Cavoid=20kmap=5Fhigh=20deadlock.?= Date: Fri, 19 Jul 2024 00:18:32 +0800 Message-ID: <1721319512-23585-1-git-send-email-zhang.chuna@h3c.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <20240710103611.809895ff809df9ed411bfaa8@linux-foundation.org> References: <20240710103611.809895ff809df9ed411bfaa8@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.99.206.13] X-ClientProxiedBy: BJSMTP01-EX.srv.huawei-3com.com (10.63.20.132) To DAG6EX09-BJD.srv.huawei-3com.com (10.153.34.11) X-DNSRBL: X-MAIL:h3cspam02-ex.h3c.com 46IGH1dk085127 X-Stat-Signature: wcadt567t1x1uii6u4ka31bdxxaqdrgj X-Rspam-User: X-Rspamd-Queue-Id: 4EFC310001D X-Rspamd-Server: rspam02 X-HE-Tag: 1721319442-797590 X-HE-Meta: U2FsdGVkX1+jBMMdqShDrNZHGcvpqSzaXweANsCjkpXsW3uxazmQGF5HE0rQblEpx+kcbldm0V3MLU+i0ZbFaz9DmUGFuMY1tJQQwb+MbHnjD+Ik0L89Rzv1DmqMpP7bPNuQDuo0/3/8oulJib1MkE+gEnca10WKXt3xKBTPdKFwQpGa3SxzLib/JeHFFjoNnkZGpph5oFVJ5gsMlXOjPz1v//RVRCFxnoQ3jsNyQ/wyvqjLtIyi/BFV9wIMkQEnjCYLmWPjkAYR2NYrSG5CUAfYLpX9ukwzGcyzgGZ7azk8C1feKcVRpARxarqyrlcYshEHhP0MpOdr5CieWz/C9AVCf9s+RzaqpPosqszr/4Us9lt4/wb3D+Kt8EWAhmcasm7qbCp6UAx/t+fozZdOPiYPYdeUG6av3PqmxxcrgiYEj33K/y+dL72zLcnjOYA9I/IcK1mmb5B95yr8U5OZ3TTHkUNn5bzo5HcyIoFKsEO1A+u5JJM8gGAC1g2OMzGUCJz67nUmj6Ck2UfaAA0XtjUEN/oyeECXvDutMzzeg7LErEW4UDZ6hL/WR2oDN6y4UavcU08skCDXjs2bcBJsC3ELrSi5lfUiONMAe9ZFwvJsTkt08FEFmXDjE464D7Aw2MKUgI5s7zyvt2jFZN1uNUy19Vn7FnAqeJosFC1Qf4BGZ1/LlZXA3ve52VfzWAr4ItfgDF7hPYfqaV576kgp8Yy6Mjp30NX6+UqCSVLxRDfIZY6487xJLhsm8GF46gAfNHpqPvX6BKagJ8A981/jWl8HqpIOBDd7uAmtbgOhljrzM4mdF/dDICUSmwiC2CCwoRiyBPehnJ6LgH13uY5p35SRkfPljGkQD2FyJpQwb34k7hZM0lFgekzDde3vHntuCHfD6sUYDis3izMsXtKeFb45neoedX2JeHu9SnktrCMzGAS9ewWF6yQfWtOuZrGdt3KyC6oHnLWFxafpbI0 A61Io3Jc 2IYHLUGqjQgEzUfrmxCNKoG+/qRbisRR0oSJL3yy5Q18P7w4loaUWfwDtjb1pfSVWzVY4n4VGEa7bHnCwDFVn/+j5/R6J8VpvF6oNnpFEl1/YsG+jfQZZm4q8oFhrBrFAfkhYql+KV4smAXqm2n6DpyjSHbGBUAeVlxg4v1CpPgb7ycaSuBehD8fOAu9Nz40UIdc9 X-Bogosity: Ham, tests=bogofilter, spamicity=0.002474, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Very sorry to disturb! Just a friendly ping to check in on the status of the patch "Give kmap_lock before call flush_tlb_kernel_rang，avoid kmap_high deadlock.". Please let me know if there is any additional information from my side. Sincerely look forward to your suggestions and guidance! >>> >> --- a/mm/highmem.c >>> >> +++ b/mm/highmem.c >>> >> @@ -220,8 +220,11 @@ static void flush_all_zero_pkmaps(void) >>> >> set_page_address(page, NULL); >>> >> need_flush = 1; >>> >> } >>> >> - if (need_flush) >>> >> + if (need_flush) { >>> >> + unlock_kmap(); >>> >> flush_tlb_kernel_range(PKMAP_ADDR(0), PKMAP_ADDR(LAST_PKMAP)); >>> >> + lock_kmap(); >>> >> + } >>> >> } >>> >>> >Why is dropping the lock like this safe? What data is it protecting >>> >and why is it OK to leave that data unprotected here? >>> >>> 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. >>No, the risk here is that when the lock is dropped, other threads will modify the data. And when this thread (the one running >>flush_all_zero_pkmaps()) retakes the lock, that data may now be unexpectedly altered. >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]. >If pkmap_count[last_pkmap_nr] != 0, Thread A continue to call get_next_pkmap_nr and get next 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; >} -- 1.8.3.1