Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 25 May 2021 22:54:02 -0700 (PDT)
Subject:     Re: [PATCH -next 4/4] riscv: mm: add THP support on 64-bit
In-Reply-To: <CAMuHMdWxQO1ik_-On3qZMN9+r8V8YMgfg2EArxiMDvoHQRo=+g@mail.gmail.com>
CC:     sunnanyong@huawei.com, Paul Walmsley <paul.walmsley@sifive.com>,
        aou@eecs.berkeley.edu, linux-riscv@lists.infradead.org,
        linux-kernel@vger.kernel.org, Atish Patra <Atish.Patra@wdc.com>,
        wangkefeng.wang@huawei.com, jszhang@kernel.org
From:   Palmer Dabbelt <palmerdabbelt@google.com>
To:     geert@linux-m68k.org
Message-ID: <mhng-187c9f60-c350-4b13-9c3b-e1afe8cb145c@palmerdabbelt-glaptop>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 25 May 2021 05:14:45 PDT (-0700), geert@linux-m68k.org wrote:
> Hi Nanyong,
>
> On Fri, Apr 30, 2021 at 9:58 AM Nanyong Sun <sunnanyong@huawei.com> wrote:
>> Bring Transparent HugePage support to riscv. A
>> transparent huge page is always represented as a pmd.
>>
>> Signed-off-by: Nanyong Sun <sunnanyong@huawei.com>
>
> Thanks for your patch, which is now commit e88b333142e4aba7 ("riscv:
> mm: add THP support on 64-bit") in riscv/for-next.
>
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>
>> @@ -438,6 +459,141 @@ static inline int ptep_clear_flush_young(struct vm_area_struct *vma,
>>         return ptep_test_and_clear_young(vma, address, ptep);
>>  }
>>
>> +/*
>> + * THP functions
>> + */
>> +static inline pmd_t pte_pmd(pte_t pte)
>> +{
>> +       return __pmd(pte_val(pte));
>> +}
>> +
>> +static inline pmd_t pmd_mkhuge(pmd_t pmd)
>> +{
>> +       return pmd;
>> +}
>> +
>> +static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>> +{
>> +       return __pmd(pmd_val(pmd) & ~(_PAGE_PRESENT|_PAGE_PROT_NONE));
>> +}
>> +
>> +#define __pmd_to_phys(pmd)  (pmd_val(pmd) >> _PAGE_PFN_SHIFT << PAGE_SHIFT)
>> +
>> +static inline unsigned long pmd_pfn(pmd_t pmd)s
>> +{
>> +       return ((__pmd_to_phys(pmd) & PMD_MASK) >> PAGE_SHIFT);
>> +}
>> +
>> +static inline pmd_t mk_pmd(struct page *page, pgprot_t prot)
>> +{
>> +       return pfn_pmd(page_to_pfn(page), prot);
>> +}
>> +
>> +static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>> +{
>> +       return pte_pmd(pte_modify(pmd_pte(pmd), newprot));
>> +}
>> +
>> +#define pmd_write pmd_write
>> +static inline int pmd_write(pmd_t pmd)
>> +{
>> +       return pte_write(pmd_pte(pmd));
>> +}
>> +
>> +static inline int pmd_dirty(pmd_t pmd)
>> +{
>> +       return pte_dirty(pmd_pte(pmd));
>> +}
>> +
>> +static inline int pmd_young(pmd_t pmd)
>> +{
>> +       return pte_young(pmd_pte(pmd));
>> +}
>> +
>> +static inline pmd_t pmd_mkold(pmd_t pmd)
>> +{
>> +       return pte_pmd(pte_mkold(pmd_pte(pmd)));
>> +}
>> +
>> +static inline pmd_t pmd_mkyoung(pmd_t pmd)
>> +{
>> +       return pte_pmd(pte_mkyoung(pmd_pte(pmd)));
>> +}
>> +
>> +static inline pmd_t pmd_mkwrite(pmd_t pmd)
>> +{
>> +       return pte_pmd(pte_mkwrite(pmd_pte(pmd)));
>> +}
>> +
>> +static inline pmd_t pmd_wrprotect(pmd_t pmd)
>> +{
>> +       return pte_pmd(pte_wrprotect(pmd_pte(pmd)));
>> +}
>> +
>> +static inline pmd_t pmd_mkclean(pmd_t pmd)
>> +{
>> +       return pte_pmd(pte_mkclean(pmd_pte(pmd)));
>> +}
>> +
>> +static inline pmd_t pmd_mkdirty(pmd_t pmd)
>> +{
>> +       return pte_pmd(pte_mkdirty(pmd_pte(pmd)));
>> +}
>> +
>> +static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr,
>> +                               pmd_t *pmdp, pmd_t pmd)
>> +{
>> +       return set_pte_at(mm, addr, (pte_t *)pmdp, pmd_pte(pmd));
>> +}
>
> This conflicts with the existing definition recently added in commit
> 756a8896b6166bba ("riscv: mremap speedup - enable HAVE_MOVE_PUD and
> HAVE_MOVE_PMD"):
>
>     arch/riscv/include/asm/pgtable.h:579:20: error: redefinition of ‘set_pmd_at’
>       579 | static inline void set_pmd_at(struct mm_struct *mm,
> unsigned long addr,
>           |                    ^~~~~~~~~~
>     arch/riscv/include/asm/pgtable.h:420:20: note: previous definition
> of ‘set_pmd_at’ was here
>       420 | static inline void set_pmd_at(struct mm_struct *mm,
> unsigned long addr,
>           |                    ^~~~~~~~~~

Sorry about that.  I was trying to re-arrange for-next to keep patch 
sets on branches to avoid the rebasing like we had last time, but that 
makes merges like this more of a headache and just makes the whole thing 
a bit clunky.

I've fixed this up, along with just making the history linear again.  

Thanks!

>
>> +
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> +static inline int pmd_trans_huge(pmd_t pmd)
>> +{
>> +       return pmd_leaf(pmd);
>> +}
>> +
>> +#define __HAVE_ARCH_PMDP_SET_ACCESS_FLAGS
>> +static inline int pmdp_set_access_flags(struct vm_area_struct *vma,
>> +                                       unsigned long address, pmd_t *pmdp,
>> +                                       pmd_t entry, int dirty)
>> +{
>> +       return ptep_set_access_flags(vma, address, (pte_t *)pmdp, pmd_pte(entry), dirty);
>> +}
>> +
>> +#define __HAVE_ARCH_PMDP_TEST_AND_CLEAR_YOUNG
>> +static inline int pmdp_test_and_clear_young(struct vm_area_struct *vma,
>> +                                       unsigned long address, pmd_t *pmdp)
>> +{
>> +       return ptep_test_and_clear_young(vma, address, (pte_t *)pmdp);
>> +}
>> +
>> +#define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR
>> +static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm,
>> +                                       unsigned long address, pmd_t *pmdp)
>> +{
>> +       return pte_pmd(ptep_get_and_clear(mm, address, (pte_t *)pmdp));
>> +}
>> +
>> +#define __HAVE_ARCH_PMDP_SET_WRPROTECT
>> +static inline void pmdp_set_wrprotect(struct mm_struct *mm,
>> +                                       unsigned long address, pmd_t *pmdp)
>> +{
>> +       ptep_set_wrprotect(mm, address, (pte_t *)pmdp);
>> +}
>> +
>> +#define pmdp_establish pmdp_establish
>> +static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
>> +                               unsigned long address, pmd_t *pmdp, pmd_t pmd)
>> +{
>> +       return __pmd(atomic_long_xchg((atomic_long_t *)pmdp, pmd_val(pmd)));
>> +}
>> +
>> +#define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
>> +void flush_pmd_tlb_range(struct vm_area_struct *vma, unsigned long start,
>> +                       unsigned long end);
>> +
>> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>> +
>>  /*
>>   * Encode and decode a swap entry
>>   *
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds