linux-kernel - Re: [PATCH] damon: Use pmdp_get instead of drect dereferencing pmd.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM7-yPQp-3BRcuFjOfEfdsMzUNU2bswKWAdQpOe_3gV1TyH5AQ@mail.gmail.com>
Date:   Thu, 27 Jul 2023 22:15:49 +0100
From:   Yun Levi <ppbuk5246@...il.com>
To:     SeongJae Park <sj@...nel.org>
Cc:     akpm@...ux-foundation.org, damon@...ts.linux.dev,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] damon: Use pmdp_get instead of drect dereferencing pmd.

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@...il.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.