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 BA1C9EB64D9 for ; Wed, 14 Jun 2023 11:09:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1AC436B0074; Wed, 14 Jun 2023 07:09:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 15CF56B0075; Wed, 14 Jun 2023 07:09:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0249C6B0078; Wed, 14 Jun 2023 07:09:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id E8BF36B0074 for ; Wed, 14 Jun 2023 07:09:51 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id B8689140939 for ; Wed, 14 Jun 2023 11:09:51 +0000 (UTC) X-FDA: 80901083382.18.CD8517F Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf09.hostedemail.com (Postfix) with ESMTP id E8AAF140016 for ; Wed, 14 Jun 2023 11:09:49 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=linuxfoundation.org header.s=korg header.b="2Dvsaip/"; spf=pass (imf09.hostedemail.com: domain of gregkh@linuxfoundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=gregkh@linuxfoundation.org; dmarc=pass (policy=none) header.from=linuxfoundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686740990; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WhOu0XxN6uhlmRMWT5HlnNaXNdcEQwTVJ0R+gre2Hng=; b=qGjffBUpwO8q656CYy9Q8JvpMwbfU5Zp92SLzq2iI6tLTSZk44ttQWXDHoJqCMT4KSRaol xqRfR1uvHKYicxLdolZR5/nvzJuDJ6EkmmVKTmgBE3KpbMit6AkKOE8zeELnXowqolz/Ew p4jmqD6+nPRgUA51R+C/EpquyjR1nn8= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686740990; a=rsa-sha256; cv=none; b=UwowpsF/YJCJXMEGGieqYt7AgzAXZhlmZuE8m9Lhibhazvpguyd+bE4r9wBYECp8TlGTPC 3Z4cqO9tSdiWFfcfxCQEejMbJGteSIz01lYto5fBwILbyVK4WeibcFKxnv+6YO31Ggphz1 f512k3VJlBko6qOz9/D3dg+ZNVoeZR8= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=linuxfoundation.org header.s=korg header.b="2Dvsaip/"; spf=pass (imf09.hostedemail.com: domain of gregkh@linuxfoundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=gregkh@linuxfoundation.org; dmarc=pass (policy=none) header.from=linuxfoundation.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id C48D462820; Wed, 14 Jun 2023 11:09:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A84C3C433C8; Wed, 14 Jun 2023 11:09:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1686740988; bh=QlLA8ehxT/VDQBjkSwCszlwern9Sc5B23YkkiyOKqUI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=2Dvsaip/e3rSwQLYF2uKMQQ0FhSwfibEEHJRzl1bxV8Kt8EBXlSKgt1vtjw5mqv6Y BYEDnJ7Pjke3cJO3da1vvqKZ8sO6ZG2MJY/VvBkR5xo3po463zmUG6p7ibGRUK54g0 sxMVML4nymFhYW8j5jI2Wp1X1WfLKZxBP2SiFe4Y= Date: Wed, 14 Jun 2023 13:09:45 +0200 From: Greg KH To: Yajun Deng Cc: rafael@kernel.org, rppt@kernel.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] mm/mm_init.c: remove spinlock in early_pfn_to_nid() Message-ID: <2023061431-litigate-upchuck-7ed1@gregkh> References: <20230614110324.3839354-1-yajun.deng@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230614110324.3839354-1-yajun.deng@linux.dev> X-Rspamd-Queue-Id: E8AAF140016 X-Rspam-User: X-Stat-Signature: rehszf5n5qczya4u51k4tdf7tymruyga X-Rspamd-Server: rspam03 X-HE-Tag: 1686740989-794236 X-HE-Meta: U2FsdGVkX19SkTVDeNYerlv1wHZKFjGAGcUcfyInxldA5jWIeksE/Ec5luDUOnNDtnPP7xfKrGqNn7OgWp0wvnnv9d8Yr6emhhL5G/8K2VkDwc9hIiY6P5HOUPFeW+hKa3no0H7c2L+lE3qKWIfQOOZZk4wne/x1nY9vi9XDJ/llOSFsWmeQdbIsdbF9UAR0gvw+jkV6HLXLW8XmHgv/CDfBYK4olTlZwIuDVnBj+C0VZUUNyJUFAq+jxSfU0kRIQSryjEeAPX23l7saseL9lOKKSqy5+mqNdySdOiOcTj2iexUOQECsWR0wzxZ+718KrA6mGxizPIZJqzORfUMBzpU73SXJ9LcwiWiW0eHZbZmB5QjZxa1uSDTOGI4tsXR1EbOu9mP0QhoPNfpKtbglY6hlv/7Xb2OrgblVXD2kdALkCCQ1fothsY4HmeTlw7eiKI8GNL2AH7GLhKlbNuyRFIU1mQ9NfnUhdR0eMmU2/dVkGgx0x8ZnI11C1Q16kf1p4k9GflkiblMimGXOQXRxy0v95ym2RJ1IO15l11YiQkqFIJkeh39K9BbjzBrXbD8TDQlLUTxdIRjMVl1Myxku/y33tXHj3ImSj+SkBXWEfmwib1fZEsSHPEVUNRuGr+wcWvBhZ09LNQ8XFvxpT1JX9GfFL4hw+zohyMAx20od374njNlXHKGoAb7Pcv7IZ6rSxaLr339zl04cSgeVrAsQ3YspSsQIwb7VjSJMq6ThvWaUv8LX+zbK+B9PuuQSKSxSgOtcB7KMOUwNKzOeLIg9kdkfyFqCSjtsneLpPKenGw63LfCialTNNCl4Rs2fe/KPZMJVvI2im4shP/TQ5KZe+0sXq5beNY/QOVgx7e+xTe3V4DQL7KPurdrFSnui/pSROQkjGYDW0aNF8+gS8iYoelCnSfjgabSVQ92+kTcmicaym72hrSG/6IochjuSfPPMU32cZs1rGzJ/PVQQk/b KxRE8uMq sIBOmIONvjtBpR62eYlAuiHWOMYVUBmes+MKRd5pVQUZPPpk5f+SrHo3axld6dTuXnwHKj7T7CvORFZG/vlvwJ4iWcAJuf2R9TAYqUHzunziaCKNPinAoTjcsLtwhUxafX0Bo6xsFWSbXif7hLMjXexI8/UsSvum3q8gTG4aoM2LM1zORtxIatWj0IWpJS64yq9xwICIqpso4+MEnJNc8Y6ieeni1yYSX7z90eC7bEoToFE/YAq5ym2jw2VOzSoYEAVTencypGMs/JrfKmvAtW3Ro2KUpXzsJ2EwvRl5vuWlZsZi4qnc7M7tQ2wLbT8WsnX7LIdRMK+Ol2Rt14ftm3OX4kCtrJqp8bBAq 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: On Wed, Jun 14, 2023 at 07:03:24PM +0800, Yajun Deng wrote: > When the system boots, only one cpu is enabled before smp_init(). > So the spinlock is not needed in most cases, remove it. > > Add spinlock in get_nid_for_pfn() because it is after smp_init(). So this is two different things at once in the same patch? Or are they the same problem and both need to go in to solve it? And if a spinlock is not needed at early boot, is it really causing any problems? > > Signed-off-by: Yajun Deng > --- > drivers/base/node.c | 11 +++++++++-- > mm/mm_init.c | 18 +++--------------- > 2 files changed, 12 insertions(+), 17 deletions(-) > > diff --git a/drivers/base/node.c b/drivers/base/node.c > index 9de524e56307..844102570ff2 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c > @@ -748,8 +748,15 @@ int unregister_cpu_under_node(unsigned int cpu, unsigned int nid) > static int __ref get_nid_for_pfn(unsigned long pfn) > { > #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT > - if (system_state < SYSTEM_RUNNING) > - return early_pfn_to_nid(pfn); > + static DEFINE_SPINLOCK(early_pfn_lock); > + int nid; > + > + if (system_state < SYSTEM_RUNNING) { > + spin_lock(&early_pfn_lock); > + nid = early_pfn_to_nid(pfn); > + spin_unlock(&early_pfn_lock); Adding an external lock for when you call a function is VERY dangerous as you did not document this anywhere, and there's no way to enforce it properly at all. Does your change actually result in any boot time changes? How was this tested? thanks, greg k-h