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 B71AFC52D7C for ; Fri, 9 Aug 2024 12:08:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 308006B008C; Fri, 9 Aug 2024 08:08:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 291606B0092; Fri, 9 Aug 2024 08:08:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0E3C76B0095; Fri, 9 Aug 2024 08:08:34 -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 DFBC06B008C for ; Fri, 9 Aug 2024 08:08:33 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id E8F5F1A0BD3 for ; Fri, 9 Aug 2024 12:08:32 +0000 (UTC) X-FDA: 82432584864.06.E4E4496 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf29.hostedemail.com (Postfix) with ESMTP id 0B6AE120030 for ; Fri, 9 Aug 2024 12:08:30 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=qVaIQJt3; dkim=pass header.d=linutronix.de header.s=2020e header.b="/79bjhG1"; dmarc=pass (policy=none) header.from=linutronix.de; spf=pass (imf29.hostedemail.com: domain of tglx@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=tglx@linutronix.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723205257; a=rsa-sha256; cv=none; b=2pc+bvIlHCatqV4EXtpnuvVTj9+SQgqkMXo7PqduRyuZQFk2PHsvk6gwMclwWasrTHzX+L VxNA+3Et3w+TknOwjpeyAIk4fQfpW+S0YIfP9Hvu0TRwFn2JZbSz6Q5UXABrm0L5lNr36b tHrN9I9Yu/3myfM5dqTTK1v7p2PjXHw= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=qVaIQJt3; dkim=pass header.d=linutronix.de header.s=2020e header.b="/79bjhG1"; dmarc=pass (policy=none) header.from=linutronix.de; spf=pass (imf29.hostedemail.com: domain of tglx@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=tglx@linutronix.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723205257; 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=rdiGSFNTrBSYFwpgP8eah13N5RSzPLohIKvfJai2S0w=; b=PgVjvBUAwKUoQPTV3dTaWW3X286eZrOonbPXrUWnNAvNaOI2m191DF3Ngg+vf3BlpmyXl1 s/GVxcWxEhJOJKYqIQDXuKY0vm1us/NeBbvxXvMdUnU4f2SQ8fF3bH3/bgqkrynvjs+Vmx xOdzbt+TKv33A6OAU5rM4MnwWNxifU0= From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1723205308; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=rdiGSFNTrBSYFwpgP8eah13N5RSzPLohIKvfJai2S0w=; b=qVaIQJt3ulHehi7WSoVKySGiDzelyn+pRR9wbJgnWO2lp+Qo7AVqYgugRvjWBA4m7AnI5g RSkAWMfsf0wMO1SxKPQ1Z9ubxGTX1QtKWxOQMjuNBog7jhsnDOnIJMSDA47fs7C6pAaD/j /NTaSYvqc5qqHZ352tolGC+8vN/wRq9LkuYPNfe50M73OT1tBf+zZ3qNXulCLH0w4Pb++n h+xPRK4/bE2McX5JcsVG10zAS5tpgG3ORG6F8SCX7pvrhPJx63VtzTIjQf6tRYd4Wv5dRL T1G4oa8Gskr9gSAhxvNNCPXsl7Bvwy6Y4TP8oWDZ+v/Ynvg/K49hQvJ2LEoonw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1723205308; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=rdiGSFNTrBSYFwpgP8eah13N5RSzPLohIKvfJai2S0w=; b=/79bjhG1R37llYQMGwou1YZqcLFThCIIXln46S8iy0NBQP/SGfyspEPmffXtpRRabZzXZH 09nuUsMPC+J/+BCg== To: Peter Xu Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, "Aneesh Kumar K . V" , Michael Ellerman , Oscar Salvador , Dan Williams , James Houghton , Matthew Wilcox , Nicholas Piggin , Rik van Riel , Dave Jiang , Andrew Morton , x86@kernel.org, Ingo Molnar , Rick P Edgecombe , "Kirill A . Shutemov" , linuxppc-dev@lists.ozlabs.org, Mel Gorman , Hugh Dickins , Borislav Petkov , David Hildenbrand , Vlastimil Babka , Dave Hansen , Christophe Leroy , Huang Ying Subject: Re: [PATCH v4 4/7] mm/x86: Make pud_leaf() only care about PSE bit In-Reply-To: References: <20240807194812.819412-1-peterx@redhat.com> <20240807194812.819412-5-peterx@redhat.com> <87bk240y8h.ffs@tglx> Date: Fri, 09 Aug 2024 14:08:28 +0200 Message-ID: <87mslluceb.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 0B6AE120030 X-Stat-Signature: 7qu4cufk4nq7d7xr6omtwfn6o5d96fjr X-Rspam-User: X-HE-Tag: 1723205310-338624 X-HE-Meta: U2FsdGVkX1/yVlSOByKaDE5j74yZIFgHR547n2y3656Dl+UkrfgnFb9NMGqxqB2c0+ZvzHVSa+vINNC6Jg7YLm45eQEq0ULo+r0232SYpMXDnGlabtRbZdR05VQ6tYltUjhd2huqFGY0mg/v3SzHIGkiscnefojpzYXDbQQ439tu9Njanx4Scp0A4Sk5J2Bm71KCRqzXH3irjLoG1WUM03O/Hcy9xrY3bsElxO9TVf4hqi7BCjWQIU+1RHP71466jZXVZHbR8wWl+6G4VJOIjIKohuGvmgdXuybs4ApmOxtoBjQ8vyP6orNUXzdHPyE0RP2F1c/yP+I/SKdQ9YEqv3o1FUS0AVOHjb3p/woEEQPCnrldRplgB66bFpwieuO2kPr93QemoXvsFgGsby4VjFQuL/kF2RvXFBd79nNvESm2gwIo9QvtN8rDn4C7XDwL+hg38ULspkas69aun2AgCCyphOwUFHA1v7ezm6JTt7dVrrcc7TdU3BzRna1m28E21VCR8td7kRth9+XACjWJFZ+Cyeq5w4xScTo1oLE+Oaj4SJxTcwIVMi96RarNnzN+9fg8314eurE7XxWPOfrDwvHX227SZeOm9K8S+skGE7EA++GkZkPU8pFECz4/+jqI4HKNLV3+HR7eDB5MBuz7+gCLUIjWnbZKGDYhcFmmbgAjRLutIgo7oc9pfsjFXizNY+zHL2DSYvrn5Va2KpTNMOT5T+cmryvFNMIwVxUV9jb2B//axOHwYkS5UaZL3ckLl7ijhrer+satyBFhcaTTS9alfdCKXd6PPjXnpyC2Ea6j1m/SB0S88sI+1Muel1hThjwdPirNgR5S8gTH9ZZYJQ7AddvX8EWOgEw0VNBv1D91QdMgs4CNMV0dEWR1GwBRV3I9kqP7LpEp/Fjj8foU6rnfNRfCilbZ/yYcf5mLMLSkaHEc6qMr41sIA4Zfj0F1FakU1GroAiJobkOlY5f Ih9O7V2j 2AP6PWrQFXhsptDxsBSXQLZDKdl1wt9cgcBNp5VFIlLXuOwsXGM3GYjFyEPiVeSzqMnqMBMYTHXE4vlpeEvuW8EoOWOrPjcQF+6S0Vei5WbqG42CfmVwvXFzjWAYPNi50S43Wp1Aq0zA7C1cPSKXmEW3iFn+RLebpwtwElPKKj7HWviZ8Ci6dob2FQchQzzwy76KlXVzyVOXNb+i2optKOeaAcCr2/2yUmOT+vIJG9PMn5Pr5e42Rv/bOXIP/CubWutd3a02BaHBGhFkCes0svyrjWfAdscs/cTSI 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: List-Subscribe: List-Unsubscribe: On Thu, Aug 08 2024 at 10:54, Peter Xu wrote: > On Thu, Aug 08, 2024 at 12:22:38AM +0200, Thomas Gleixner wrote: >> On Wed, Aug 07 2024 at 15:48, Peter Xu wrote: >> > An entry should be reported as PUD leaf even if it's PROT_NONE, in which >> > case PRESENT bit isn't there. I hit bad pud without this when testing dax >> > 1G on zapping a PROT_NONE PUD. >> >> That does not qualify as a change log. What you hit is irrelevant unless >> you explain the actual underlying problem. See Documentation/process/ >> including the TIP documentation. > > Firstly, thanks a lot for the reviews. > > I thought the commit message explained exactly what is the underlying > problem, no? > > The problem is even if PROT_NONE, as long as the PSE bit is set on the PUD > it should be treated as a PUD leaf. Sure. But why should it be treated so. > Currently, the code will return pud_leaf()==false for those PROT_NONE > PUD entries, and IMHO that is wrong. Your humble opinion is fine, but hardly a technical argument. > This patch wants to make it right. I still think that's mostly what I put > there in the commit message.. > > Would you please suggest something so I can try to make it better, > otherwise? Or it'll be helpful too if you could point out which part of > the two documentations I should reference. https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog A good structure is to explain the context, the problem and the solution in separate paragraphs and this order >> > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h >> > index e39311a89bf4..a2a3bd4c1bda 100644 >> > --- a/arch/x86/include/asm/pgtable.h >> > +++ b/arch/x86/include/asm/pgtable.h >> > @@ -1078,8 +1078,7 @@ static inline pmd_t *pud_pgtable(pud_t pud) >> > #define pud_leaf pud_leaf >> > static inline bool pud_leaf(pud_t pud) >> > { >> > - return (pud_val(pud) & (_PAGE_PSE | _PAGE_PRESENT)) == >> > - (_PAGE_PSE | _PAGE_PRESENT); >> > + return pud_val(pud) & _PAGE_PSE; >> > } >> >> And the changelog does not explain why this change is not affecting any >> existing user of pud_leaf(). > > That's what I want to do: I want to affect them.. Fine. Just the change log does not tell me what the actual problem is ("I hit something" does not qualify) and "should be reported" is not helpful either as it does not explain anything > And IMHO it's mostly fine before because mprotect() is broken with 1g > anyway, and I guess nobody managed to populate any pud entry with PROT_NONE > on dax 1g before, and that's what this whole series is trying to fix. Again your humble opinion matters, but technical facts and analysis matter way more. Thanks, tglx