From: Chuhong Yuan <hslester96@gmail.com>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>,
linuxppc-dev@lists.ozlabs.org,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] powerpc/mm: Use refcount_t for refcount
Date: Sat, 10 Aug 2019 09:25:19 +0800 [thread overview]
Message-ID: <CANhBUQ3VpwxyMpQ29kphcowgmpPPTBLtBV19tBGVX82uUs=Wmw@mail.gmail.com> (raw)
In-Reply-To: <87y302segw.fsf@concordia.ellerman.id.au>
On Fri, Aug 9, 2019 at 8:36 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Chuhong Yuan <hslester96@gmail.com> writes:
> > Reference counters are preferred to use refcount_t instead of
> > atomic_t.
> > This is because the implementation of refcount_t can prevent
> > overflows and detect possible use-after-free.
> > So convert atomic_t ref counters to refcount_t.
> >
> > Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
>
> Thanks.
>
> We don't have a fast implementation of refcount_t, so I'm worried this
> could cause a measurable performance regression.
>
> Did you benchmark it at all?
>
I did not benchmark it and I don't have the testing environment...
> cheers
>
> > diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
> > index 2d0cb5ba9a47..f836fd5a6abc 100644
> > --- a/arch/powerpc/mm/book3s64/mmu_context.c
> > +++ b/arch/powerpc/mm/book3s64/mmu_context.c
> > @@ -231,7 +231,7 @@ static void pmd_frag_destroy(void *pmd_frag)
> > /* drop all the pending references */
> > count = ((unsigned long)pmd_frag & ~PAGE_MASK) >> PMD_FRAG_SIZE_SHIFT;
> > /* We allow PTE_FRAG_NR fragments from a PTE page */
> > - if (atomic_sub_and_test(PMD_FRAG_NR - count, &page->pt_frag_refcount)) {
> > + if (refcount_sub_and_test(PMD_FRAG_NR - count, &page->pt_frag_refcount)) {
> > pgtable_pmd_page_dtor(page);
> > __free_page(page);
> > }
> > diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> > index 7d0e0d0d22c4..40056896ce4e 100644
> > --- a/arch/powerpc/mm/book3s64/pgtable.c
> > +++ b/arch/powerpc/mm/book3s64/pgtable.c
> > @@ -277,7 +277,7 @@ static pmd_t *__alloc_for_pmdcache(struct mm_struct *mm)
> > return NULL;
> > }
> >
> > - atomic_set(&page->pt_frag_refcount, 1);
> > + refcount_set(&page->pt_frag_refcount, 1);
> >
> > ret = page_address(page);
> > /*
> > @@ -294,7 +294,7 @@ static pmd_t *__alloc_for_pmdcache(struct mm_struct *mm)
> > * count.
> > */
> > if (likely(!mm->context.pmd_frag)) {
> > - atomic_set(&page->pt_frag_refcount, PMD_FRAG_NR);
> > + refcount_set(&page->pt_frag_refcount, PMD_FRAG_NR);
> > mm->context.pmd_frag = ret + PMD_FRAG_SIZE;
> > }
> > spin_unlock(&mm->page_table_lock);
> > @@ -317,8 +317,7 @@ void pmd_fragment_free(unsigned long *pmd)
> > {
> > struct page *page = virt_to_page(pmd);
> >
> > - BUG_ON(atomic_read(&page->pt_frag_refcount) <= 0);
> > - if (atomic_dec_and_test(&page->pt_frag_refcount)) {
> > + if (refcount_dec_and_test(&page->pt_frag_refcount)) {
> > pgtable_pmd_page_dtor(page);
> > __free_page(page);
> > }
> > diff --git a/arch/powerpc/mm/pgtable-frag.c b/arch/powerpc/mm/pgtable-frag.c
> > index a7b05214760c..4ef8231b677f 100644
> > --- a/arch/powerpc/mm/pgtable-frag.c
> > +++ b/arch/powerpc/mm/pgtable-frag.c
> > @@ -24,7 +24,7 @@ void pte_frag_destroy(void *pte_frag)
> > /* drop all the pending references */
> > count = ((unsigned long)pte_frag & ~PAGE_MASK) >> PTE_FRAG_SIZE_SHIFT;
> > /* We allow PTE_FRAG_NR fragments from a PTE page */
> > - if (atomic_sub_and_test(PTE_FRAG_NR - count, &page->pt_frag_refcount)) {
> > + if (refcount_sub_and_test(PTE_FRAG_NR - count, &page->pt_frag_refcount)) {
> > pgtable_page_dtor(page);
> > __free_page(page);
> > }
> > @@ -71,7 +71,7 @@ static pte_t *__alloc_for_ptecache(struct mm_struct *mm, int kernel)
> > return NULL;
> > }
> >
> > - atomic_set(&page->pt_frag_refcount, 1);
> > + refcount_set(&page->pt_frag_refcount, 1);
> >
> > ret = page_address(page);
> > /*
> > @@ -87,7 +87,7 @@ static pte_t *__alloc_for_ptecache(struct mm_struct *mm, int kernel)
> > * count.
> > */
> > if (likely(!pte_frag_get(&mm->context))) {
> > - atomic_set(&page->pt_frag_refcount, PTE_FRAG_NR);
> > + refcount_set(&page->pt_frag_refcount, PTE_FRAG_NR);
> > pte_frag_set(&mm->context, ret + PTE_FRAG_SIZE);
> > }
> > spin_unlock(&mm->page_table_lock);
> > @@ -110,8 +110,7 @@ void pte_fragment_free(unsigned long *table, int kernel)
> > {
> > struct page *page = virt_to_page(table);
> >
> > - BUG_ON(atomic_read(&page->pt_frag_refcount) <= 0);
> > - if (atomic_dec_and_test(&page->pt_frag_refcount)) {
> > + if (refcount_dec_and_test(&page->pt_frag_refcount)) {
> > if (!kernel)
> > pgtable_page_dtor(page);
> > __free_page(page);
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 3a37a89eb7a7..7fe23a3faf95 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -14,6 +14,7 @@
> > #include <linux/uprobes.h>
> > #include <linux/page-flags-layout.h>
> > #include <linux/workqueue.h>
> > +#include <linux/refcount.h>
> >
> > #include <asm/mmu.h>
> >
> > @@ -147,7 +148,7 @@ struct page {
> > unsigned long _pt_pad_2; /* mapping */
> > union {
> > struct mm_struct *pt_mm; /* x86 pgds only */
> > - atomic_t pt_frag_refcount; /* powerpc */
> > + refcount_t pt_frag_refcount; /* powerpc */
> > };
> > #if ALLOC_SPLIT_PTLOCKS
> > spinlock_t *ptl;
> > --
> > 2.20.1
next prev parent reply other threads:[~2019-08-10 1:27 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-08 7:18 [PATCH] powerpc/mm: Use refcount_t for refcount Chuhong Yuan
2019-08-09 12:36 ` Michael Ellerman
2019-08-10 1:25 ` Chuhong Yuan [this message]
2021-11-26 17:55 ` Christophe Leroy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANhBUQ3VpwxyMpQ29kphcowgmpPPTBLtBV19tBGVX82uUs=Wmw@mail.gmail.com' \
--to=hslester96@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).