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;
MIME-Version: 1.0
References: <20230727183745.682880-1-ppbuk5246@gmail.com> <20230727195453.67611-1-sj@kernel.org>
In-Reply-To: <20230727195453.67611-1-sj@kernel.org>
From:   Yun Levi <ppbuk5246@gmail.com>
Date:   Thu, 27 Jul 2023 22:15:49 +0100
Message-ID: <CAM7-yPQp-3BRcuFjOfEfdsMzUNU2bswKWAdQpOe_3gV1TyH5AQ@mail.gmail.com>
Subject: Re: [PATCH] damon: Use pmdp_get instead of drect dereferencing pmd.
To:     SeongJae Park <sj@kernel.org>
Cc:     akpm@linux-foundation.org, damon@lists.linux.dev,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi SJ.

Thanks for looking into this.

> Nit for the subject, what about s/drect/directly/?  Also, let's remove the
> period at the end.
>
> On Fri, 28 Jul 2023 03:37:45 +0900 Levi Yun <ppbuk5246@gmail.com> wrote:

Thanks. But I don't know why period is added thou I sent via git send-email ..

> > -     if (pmd_trans_huge(*pmd)) {
> > +     if (pmd_trans_huge(pmdp_get_lockless(pmd))) {
>
> I don't think we really need to use pmdp_get_lockless() here, since we will do
> this check again with the lock, and we have the fallback for the intermediate
> changes.

Agree. I'll change. Thanks :)

> > @@ -434,26 +437,29 @@ static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
> >  {
> >       pte_t *pte;
> >       pte_t ptent;
> > +     pmd_t pmde;
>
> This would cause below build warning if CONFIG_TRANSPARENT_HUGEPAGE is not
> defined.
>
> .../mm/damon/vaddr.c:440:8: warning: unused variable 'pmde' [-Wunused-variable]
>   440 |  pmd_t pmde;
>       |        ^~~~
>
>
> >       spinlock_t *ptl;
> >       struct folio *folio;
> >       struct damon_young_walk_private *priv = walk->private;
> >
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -     if (pmd_trans_huge(*pmd)) {
> > +     if (pmd_trans_huge(pmdp_get_lockless(pmd))) {

Oh.. I miss this. Many thanks..!

> Thanks,
> SJ

Many Thanks..!

---------
Sincerely,
Levi.