Re: [PATCH v2] arm64: mm: fix pmd_leaf()

From: Will Deacon <will@kernel.org>
To: Muchun Song <songmuchun@bytedance.com>
Cc: Steven Price <steven.price@arm.com>,
	catalin.marinas@arm.com, akpm@linux-foundation.org,
	anshuman.khandual@arm.com, lengxujun2007@126.com, arnd@arndb.de,
	smuchun@gmail.com, duanxiongchun@bytedance.com,
	quic_qiancai@quicinc.com, aneesh.kumar@linux.ibm.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] arm64: mm: fix pmd_leaf()
Date: Tue, 19 Apr 2022 14:43:39 +0100	[thread overview]
Message-ID: <20220419134339.GA6143@willie-the-truck> (raw)
In-Reply-To: <YlgC877mS2LjsqS8@FVFYT0MHHV2J.usts.net>

On Thu, Apr 14, 2022 at 07:18:11PM +0800, Muchun Song wrote:
> On Thu, Apr 14, 2022 at 11:05:35AM +0100, Will Deacon wrote:
> > On Wed, Apr 13, 2022 at 11:39:49AM +0100, Steven Price wrote:
> > > On 13/04/2022 11:19, Will Deacon wrote:
> > > > The documentation/comment that Steven referred to also desperately needs
> > > > clarifying as it currently states:
> > > > 
> > > >   "Only meaningful when called on a valid entry."
> > > > 
> > > > whatever that means.
> > > 
> > > The intention at the time is that this had the same meaning as
> > > pmd_huge() (when CONFIG_HUGETLB_PAGE is defined), which would then match
> > > this patch. This is referred in the comment, albeit in a rather weak way:
> > > 
> > > >  * This differs from p?d_huge() by the fact that they are always available (if
> > > >  * the architecture supports large pages at the appropriate level) even
> > > >  * if CONFIG_HUGETLB_PAGE is not defined.
> > > 
> > > However, the real issue here is that the definition of pmd_leaf() isn't
> > > clear. I know what the original uses of it needed but since then it's
> > > been used in other areas, and I'm afraid my 'documentation' isn't
> > > precise enough to actually be useful.
> > > 
> > > At the time I wrote that comment I think I meant "valid" in the AArch64
> > > sense (i.e. the LSB of the entry). PROT_NONE isn't 'valid' by that
> > > definition (and I hadn't considered it). But of course that definition
> > > of 'valid' is pretty meaningless in the cross-architecture case.
> > 
> > arm64 'valid' + PROT_NONE is roughly what 'present' means. So we could say
> > that this only works for present entries, but then Muchun's latest patch
> > wants to work with !present which is why I tried to work this through.
> >
> 
> My bad. In the previous version, Aneesh seems want to make
> pmd_leaf() works for a not present page table entry, I am
> trying doing this in this version.  Seems like I do the right
> thing in the previous version from your explanation.
> 
> I'll use the previos version and fix pud_leaf() as well and
> update the documentation.  Do you think this is okay?

Yes, thanks, that sounds good to me. We can define both of these as present
&& !table.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel