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 52BE4C3DA4A for ; Wed, 14 Aug 2024 21:28:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DFFF66B00B2; Wed, 14 Aug 2024 17:28:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DB0716B00B3; Wed, 14 Aug 2024 17:28:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C78206B00B4; Wed, 14 Aug 2024 17:28:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id A7A3F6B00B2 for ; Wed, 14 Aug 2024 17:28:52 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 66C081611D9 for ; Wed, 14 Aug 2024 21:28:52 +0000 (UTC) X-FDA: 82452140904.01.0F1FFB1 Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com [209.85.218.49]) by imf16.hostedemail.com (Postfix) with ESMTP id 8804E18001E for ; Wed, 14 Aug 2024 21:28:50 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=QmxcbXXw; spf=pass (imf16.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.218.49 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723670873; 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:dkim-signature; bh=rBCZrf3s8CAVk4wrOG7aazIiqNpvS17dF6ZS4s+MwpQ=; b=aZ7CX1oYfH+GdcTkCM6EJ/GKzR2VP97rN9OSf2fhRjxJsUsdS2AE+TK8ycnX5dvVUzao89 Jm3upps6u4NjifP492DpfbWn40KwW/OZJ29REM460y+RLjLlztiU/98X+bfJaMfBRbCz6z sumbYpXXUzJHyWGWSByQZpy1+6r9hxk= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=QmxcbXXw; spf=pass (imf16.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.218.49 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723670873; a=rsa-sha256; cv=none; b=47vOmK5bJEMBtOGx0P1HhiVgeaVQPAKjCMYd5WUKY9fJ8fNzyrqPe2V3I14KtkcjDay+7S BvP8vycuRybQnBIHFlpho5+W8oXtaU6vw8VkV55nHMGhUXjCj8OEdqCUeFGQaXdhpGz5CV MPY7HY4PzRwexC1kQCPH8LlciFMWvYY= Received: by mail-ej1-f49.google.com with SMTP id a640c23a62f3a-a7d2a9a23d9so49277666b.3 for ; Wed, 14 Aug 2024 14:28:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723670929; x=1724275729; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=rBCZrf3s8CAVk4wrOG7aazIiqNpvS17dF6ZS4s+MwpQ=; b=QmxcbXXwuCr9vSqI3NtUuLuLD7lShncXlhSMtvQiCYbjPSJOQfya7897wqqu/+8ltv RUwFMctXU4sbE13fk5/8C0suXtZgwluA98EOkNrA1JytswKtKVbmFlGvMzjScuPSuuoC 47UETSIOBH7TQvUThRpIBDvIBv6sAmcoCTW3ffoLqWClfO3akYQKVTtXnwYJqyZMYpmg C3JmQvBohATjvyTTevcvKmvyBKngv46gFmnp1X+Pi2YN2+qz85peDvn3LqJBc/2Ry/mC mNWsdcZ0U/z4VTaP2KTehBXfKzx72fnGZYVlD1ik9SMzqxm64FJkXrzC/4V8sb9m7X4+ B2Rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723670929; x=1724275729; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rBCZrf3s8CAVk4wrOG7aazIiqNpvS17dF6ZS4s+MwpQ=; b=PVUOJQZzaoO9yrb8DtbOpYjpMhXqWiTWsSnpN5KWUx9tgL5ypDZu4QPpUBX3DsiwP4 QA2IDORrnJDNCShi6lAYBamfSF0xJKOsVVlHkt3EnG/lVvBlHrYgZhoc6a4dHLyxIjsw yWP7j2giWQgmjMGFmtmcKscKFDH8pt6FIszOW412yBNlsXqDWBjSrEiRjI5sIeonxgA6 zxVwwSeW11PyJhjzbijEsjEwdJ7hY2GvhU/dN+crzP2zAwGNjp6ece389iBuTRLbkhsS 4Ikd970ZTHrSePeFXZrjKxYi2EAfh2+w7p/W6tVIz3QN85lNE95GAuEvPkNjacTGFXRf dMuw== X-Forwarded-Encrypted: i=1; AJvYcCXJMDP62sCgH8yKjiEj3EykPqZhU+GetMTMv5H6phrl24/ZJwzr8wagmTJwJDxB4zzHpF3jOho+PA6wnXxblzvIVqY= X-Gm-Message-State: AOJu0YxkLc+/tUo5OX5akxTU/gmwA6pCKsCzVqAgb2/WkTcNZskVZaIt 87Um+Zo1wFRRdP0w7SdDtXNkw4Fd/sGLklFcd9nFwNybq3XTfjATGV4kDWTqyBeh704xwENzbIi /SXfg8TyAVcrOvMIyASoK900cnHg= X-Google-Smtp-Source: AGHT+IG1ba1R/vxnHTpTL7Edk9ie+e8zqinIa/uDyCVY7MiU+Rgu7ul1WUnO0BLHfW97YdSI5YIs1BeHYWZ8y4mmfDk= X-Received: by 2002:a17:907:d3c4:b0:a7a:a06b:eec9 with SMTP id a640c23a62f3a-a8366c2f611mr293074066b.4.1723670928513; Wed, 14 Aug 2024 14:28:48 -0700 (PDT) MIME-Version: 1.0 References: <20240814145256.1683498-1-mjguzik@gmail.com> <20240814141614.56337d7cd3f0671d8edc7676@linux-foundation.org> In-Reply-To: <20240814141614.56337d7cd3f0671d8edc7676@linux-foundation.org> From: Mateusz Guzik Date: Wed, 14 Aug 2024 23:28:36 +0200 Message-ID: Subject: Re: [PATCH] mm: whack now bogus comment in pmd_install() concerning a fence To: Andrew Morton Cc: npiggin@gmail.com, david@redhat.com, willy@infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: hyamotiofshkysqgz764anm66589841m X-Rspam-User: X-Rspamd-Queue-Id: 8804E18001E X-Rspamd-Server: rspam02 X-HE-Tag: 1723670930-424511 X-HE-Meta: U2FsdGVkX1+BBHVHeVEds1h8iN5BWJ2Zf00PwAQg3R5ihvaVguxJE/xOKUAeZOUqOaT1RsIWuPVTgIdAp4iOx7+mjj/EUNcNZCk8Hlc693jtmTxqmrV/c/L3iHQn/pRiDE8+VxnOPPEQwr2noTcwxcA8Gj1uAvxI4IAnep5vhNmjTUs141qHXadP2iquLtA1GftyCzNMkrA17I8cuWFJoaGHtp/nPOG9C8eq+ODAf6cRzxNI/nq14Ev03B2iX+XY+s8Uu2fEj9/kbFDy1SpzZ6wwmzAEWcfLE9C1HTQumQTO8moYtVAK2GxPBD2TIpTGtCMVGAh6q7P1SQSs5yHAOf1oI9kdhaooXwS2NADPrS1PUBqvHGh8WRAiKVDjQpCfobXVxgVkG3os0ZdjYpzBVs+3l1yYZ4FhNQE5xDKzqjdIUhm5gsHQz43X0qyFqyCPliWcw2e4CMgvwDiHavOY8VKNsub1IjssaXyMCY0kv6yMW52FEQ1+kfOKBIUKwGaxAUnwPZiMGpBlXazdIThKBzW8qxiI5kUqne5A6ez68gbl8NJhf+PEpdz4xQdf6OEgwDSoaB2rGRaeIcgFWKzbaGtdmLPx2MFOyx4HVK+igjCMbaXDji/oXcMYideweCFqdg2ZOkg/DMCM/FBcxRobnzbmbHwu4buYNDwRwa49W1HVwPawemi4UN+xQb84KlbsPzK867Wv9upl9Keln/6Vkv8RrPCWvtQ2DJf5bbEGrfhwmvDzZ8n7pzWooBjb3RRHNw/F8cyg+pnP3wSN1lNpLXVsgOs7a5Mcqr0Zs3js2YWe1u+6UntSbW0eoyv40KDY8BbOHE0l4pZ5Lr4agJ6Jl1cdiGlStfAmFtvLVUbOSE5fr8Z3mWb/RHSIPiNS7Ep5KvIYJbLCztRzvo8J702WNiLab+rIXAaroBnssmSKqdTI901c93ScsJAcwBVvksDVdqiBayA/7NPktJF1s8N vUxf/E6r AJhgrT5RTPgLl6VhkJ96WqcvtRQIKO69QHAzhOCogmf715+FmgO3XqUCORs02yIukKcMPACf5vBKwa3RV8A2CTk9nZGelnuPOd8HBXiVBWPjci4zXV6ac0jfgcg4cmvMecMsh+TyTfF5CSH9evrt8IeLoKqLqiMjPpd4CFjnnAa2eAssQ7I5asK9RyrQW5MDgbahKeTntQx/T/88hJTdEoT2nzyXGkoqMgDaP7+VjqkKVYY8bdk0qohZBy9E3EGZIRud8lE/sf+To4QEgZE6adUzn6lMHc7PhjRNRDnDuSffGgg9qkZgC+e95sllzlh43vRSGt5Padj6m1eJo+HPuYPOscC6kWGzqZ9hyTCgdQSH4HYVAg8KK6N/7MqPUjZ5ZJOYGhcv5l2LCI39uJhaeG4xl82MYwBCOC/g615uJa/g0Fo4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000006, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Aug 14, 2024 at 11:16=E2=80=AFPM Andrew Morton wrote: > > On Wed, 14 Aug 2024 16:52:56 +0200 Mateusz Guzik wrot= e: > > > Commit 362a61ad6119 ("fix SMP data race in pagetable setup vs walking") > > added the following: > > > > + smp_wmb(); /* Could be smp_wmb__xxx(before|after)_spin_lock */ > > + > > spin_lock(&mm->page_table_lock); > > > > However, over the years the fence along with the comment got moved > > around the file, eventually landing in a spot where it is *NOT* followe= d > > by a lock acquire (or any other operation which might happen to provide > > any fence on a given arch), rendering the comment stale. > > > > ... > > > > I fully concede I could not be arsed to check if the fence is still > > needed to begin with, I ran into this while looking at something else. > > The comment puzzled me for a minute suggesting pmd_populate has an > > immediate lock acquire inside. > > > > ... > > > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -436,7 +436,7 @@ void pmd_install(struct mm_struct *mm, pmd_t *pmd, = pgtable_t *pte) > > * seen in-order. See the alpha page table accessors for = the > > * smp_rmb() barriers in page table walking code. > > */ > > - smp_wmb(); /* Could be smp_wmb__xxx(before|after)_spin_lo= ck */ > > + smp_wmb(); > > pmd_populate(mm, pmd, *pte); > > *pte =3D NULL; > > } > > It's best to document all such barriers, so the preferred patch would > be to fix the comment rather than removing it. > > And if the barrier now does nothing then of course removing the thing > would be best. > > So I'd suggest that the wrong comment be left there, if only to tell > developers why the barrier used to be there! The comment above it (only partially seen in the context) documents what the purpose is. The comment I'm removing merely mentions a no longer applicable optimization opportunity: it used to be immediately followed by spin_lock. If the architecture at hand provides a full fence when acquiring a lock *and* has a costly smp_wmb, then a hypothetical smp_wmb__before_spin_lock could be used to elide it. --=20 Mateusz Guzik