Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 19 Apr 2022 14:43:39 +0100
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()
Message-ID: <20220419134339.GA6143@willie-the-truck>
References: <20220411122653.40284-1-songmuchun@bytedance.com>
 <20220413101929.GA1229@willie-the-truck>
 <64d4288e-7776-a3fd-5ee4-70486dfd0394@arm.com>
 <20220414100535.GB2298@willie-the-truck>
 <YlgC877mS2LjsqS8@FVFYT0MHHV2J.usts.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YlgC877mS2LjsqS8@FVFYT0MHHV2J.usts.net>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

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