Received-SPF: pass (google.com: domain of linux-kernel+bounces-150887-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240409192301.907377-5-david@redhat.com> <20240418145003.8780-1-ioworker0@gmail.com>
 <f8f30747-1313-4939-a2ad-3accd14ba01f@redhat.com>
In-Reply-To: <f8f30747-1313-4939-a2ad-3accd14ba01f@redhat.com>
From: Lance Yang <ioworker0@gmail.com>
Date: Fri, 19 Apr 2024 08:31:58 +0800
Message-ID: <CAK1f24nO-7QUYxXsYqDH=Hg7J_Hn9rxpkfQzaBBOpqFnzbCATQ@mail.gmail.com>
Subject: Re: [PATCH v1 04/18] mm: track mapcount of large folios in single value
To: David Hildenbrand <david@redhat.com>
Cc: akpm@linux-foundation.org, cgroups@vger.kernel.org, chris@zankel.net, 
	corbet@lwn.net, dalias@libc.org, fengwei.yin@intel.com, 
	glaubitz@physik.fu-berlin.de, hughd@google.com, jcmvbkbc@gmail.com, 
	linmiaohe@huawei.com, linux-doc@vger.kernel.org, 
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-mm@kvack.org, linux-sh@vger.kernel.org, 
	linux-trace-kernel@vger.kernel.org, muchun.song@linux.dev, 
	naoya.horiguchi@nec.com, peterx@redhat.com, richardycc@google.com, 
	ryan.roberts@arm.com, shy828301@gmail.com, willy@infradead.org, 
	ysato@users.sourceforge.jp, ziy@nvidia.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 18, 2024 at 11:09=E2=80=AFPM David Hildenbrand <david@redhat.co=
m> wrote:
>
> On 18.04.24 16:50, Lance Yang wrote:
> > Hey David,
> >
> > FWIW, just a nit below.
>
> Hi!
>

Thanks for clarifying!

> Thanks, but that was done on purpose.
>
> This way, we'll have a memory barrier (due to at least one
> atomic_inc_and_test()) between incrementing the folio refcount
> (happening before the rmap change) and incrementing the mapcount.
>
> Is it required? Not 100% sure, refcount vs. mapcount checks are always a
> bit racy. But doing it this way let me sleep better at night ;)

Yep, I understood :)

Thanks,
Lance

>
> [with no subpage mapcounts, we'd do the atomic_inc_and_test on the large
> mapcount and have the memory barrier there again; but that's stuff for
> the future]
>
> Thanks!


>
> >
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 2608c40dffad..08bb6834cf72 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1143,7 +1143,6 @@ static __always_inline unsigned int __folio_add_r=
map(struct folio *folio,
> >               int *nr_pmdmapped)
> >   {
> >       atomic_t *mapped =3D &folio->_nr_pages_mapped;
> > -     const int orig_nr_pages =3D nr_pages;
> >       int first, nr =3D 0;
> >
> >       __folio_rmap_sanity_checks(folio, page, nr_pages, level);
> > @@ -1155,6 +1154,7 @@ static __always_inline unsigned int __folio_add_r=
map(struct folio *folio,
> >                       break;
> >               }
> >
> > +             atomic_add(nr_pages, &folio->_large_mapcount);
> >               do {
> >                       first =3D atomic_inc_and_test(&page->_mapcount);
> >                       if (first) {
> > @@ -1163,7 +1163,6 @@ static __always_inline unsigned int __folio_add_r=
map(struct folio *folio,
> >                                       nr++;
> >                       }
> >               } while (page++, --nr_pages > 0);
> > -             atomic_add(orig_nr_pages, &folio->_large_mapcount);
> >               break;
> >       case RMAP_LEVEL_PMD:
> >               first =3D atomic_inc_and_test(&folio->_entire_mapcount);
> >
> > Thanks,
> > Lance
> >
>
> --
> Cheers,
>
> David / dhildenb
>