Received-SPF: pass (google.com: domain of linux-kernel+bounces-91993-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: <20240304103757.235352-1-21cnbao@gmail.com> <706b7129-85f6-4470-9fd9-f955a8e6bd7c@arm.com>
 <37f1e6da-412b-4bb4-88b7-4c49f21f5fe9@redhat.com> <CAGsJ_4yJ3yCyN_KgBO8W+jFx8RN6_JhS9OwX3FH6X_gpU7g62w@mail.gmail.com>
 <804524c8-772c-42d0-93a5-90d77f13f304@redhat.com> <CAGsJ_4yqUW46xyDtZ4X1wQZ2_0bLM85Euz2BufERa75Rg+gVyw@mail.gmail.com>
 <87r0gp868d.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4xUdqpWaYDrNV3Fbh=3cWyoDJz3AbWa=mfsKbka+MAVag@mail.gmail.com>
 <87il2182nr.fsf@yhuang6-desk2.ccr.corp.intel.com>
In-Reply-To: <87il2182nr.fsf@yhuang6-desk2.ccr.corp.intel.com>
From: Barry Song <21cnbao@gmail.com>
Date: Tue, 5 Mar 2024 22:21:43 +1300
Message-ID: <CAGsJ_4wYVogJD=ROfX195MPZrqK+=ibuycPBeFjrD1i9SvOqrw@mail.gmail.com>
Subject: Re: [RFC PATCH] mm: hold PTL from the first PTE while reclaiming a
 large folio
To: "Huang, Ying" <ying.huang@intel.com>
Cc: David Hildenbrand <david@redhat.com>, Ryan Roberts <ryan.roberts@arm.com>, akpm@linux-foundation.org, 
	linux-mm@kvack.org, chrisl@kernel.org, yuzhao@google.com, 
	hanchuanhua@oppo.com, linux-kernel@vger.kernel.org, willy@infradead.org, 
	xiang@kernel.org, mhocko@suse.com, shy828301@gmail.com, 
	wangkefeng.wang@huawei.com, Barry Song <v-songbaohua@oppo.com>, 
	Hugh Dickins <hughd@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 5, 2024 at 10:12=E2=80=AFPM Huang, Ying <ying.huang@intel.com> =
wrote:
>
> Barry Song <21cnbao@gmail.com> writes:
>
> > On Tue, Mar 5, 2024 at 8:55=E2=80=AFPM Huang, Ying <ying.huang@intel.co=
m> wrote:
> >>
> >> Barry Song <21cnbao@gmail.com> writes:
> >>
> >> > On Tue, Mar 5, 2024 at 10:15=E2=80=AFAM David Hildenbrand <david@red=
hat.com> wrote:
> >> >> > But we did "resolve" those bugs by entirely untouching all PTEs i=
f we
> >> >> > found some PTEs were skipped in try_to_unmap_one [1].
> >> >> >
> >> >> > While we find we only get the PTL from 2nd, 3rd but not
> >> >> > 1st PTE, we entirely give up on try_to_unmap_one, and leave
> >> >> > all PTEs untouched.
> >> >> >
> >> >> > /* we are not starting from head */
> >> >> > if (!IS_ALIGNED((unsigned long)pvmw.pte, CONT_PTES * sizeof(*pvmw=
pte))) {
> >> >> >                     ret =3D false;
> >> >> >                     atomic64_inc(&perf_stat.mapped_walk_start_fro=
m_non_head);
> >> >> >                     set_pte_at(mm, address, pvmw.pte, pteval);
> >> >> >                     page_vma_mapped_walk_done(&pvmw);
> >> >> >                     break;
> >> >> > }
> >> >> > This will ensure all PTEs still have a unified state such as CONT=
-PTE
> >> >> > after try_to_unmap fails.
> >> >> > I feel this could have some false postive because when racing
> >> >> > with unmap, 1st PTE might really become pte_none. So explicitly
> >> >> > holding PTL from 1st PTE seems a better way.
> >> >>
> >> >> Can we estimate the "cost" of holding the PTL?
> >> >>
> >> >
> >> > This is just moving PTL acquisition one or two PTE earlier in those =
corner
> >> > cases. In normal cases, it doesn't affect when PTL is held.
> >>
> >> The mTHP may be mapped at the end of page table.  In that case, the PT=
L
> >> will be held longer.  Or am I missing something?
> >
> > no. this patch doesn't change when we release PTL but change when we
> > get PTL.
> >
> > when the original code iterates nr_pages PTEs in a large folio, it will=
 skip
> > invalid PTEs, when it meets a valid one, it will acquire PTL. so if it =
gets
> > intermediate PTE values some other threads are modifying, it might
> > skip PTE0, or sometimes PTE0 and PTE1 according to my test. but
> > arriving at PTE2, likely other threads have written a new value, so we
> > will begin to hold PTL and iterate till the end of the large folio.
>
> Is there any guarantee that the mTHP will always be mapped at the
> beginning of the page table (PTE0)?  IIUC, mTHP can be mapped at PTE496.
> If so, with your patch, PTL will be held from PTE0 instead of PTE496 in
> some cases.

I agree. but in another discussion[1], the plan is if we find a large folio=
 has
been deferred split, we split it before try_to_unmap and pageout. otherwise=
,
we may result in lots of redundant I/O, because PTE0-495 will still be
pageout()-ed.

[1] https://lore.kernel.org/linux-mm/a4a9054f-2040-4f70-8d10-a5af4972e5aa@a=
rm.com/

>
> --
> Best Regards,
> Huang, Ying
>
> > The proposal is that we directly get PTL from PTE0, thus we don't get
> > intermediate values for the head of nr_pages PTEs. this will ensure
> > a large folio is either completely unmapped or completely mapped.
> > but not partially mapped and partially unmapped.
> >
> >>
> >> --
> >> Best Regards,
> >> Huang, Ying
> >>
> >>
> >> > In normal cases, page_vma_mapped_walk will find PTE0 is present, thu=
s hold
> >> > PTL immediately. in corner cases, page_vma_mapped_walk races with br=
eak-
> >> > before-make, after skipping one or two PTEs whose states are transfe=
rring,
> >> > it will find a present pte then acquire lock.
> >> >
> >> >> --
> >> >> Cheers,
> >> >>
> >> >> David / dhildenb
> >> >

Thanks
Barry