linux-kernel - Re: [PATCH] mm: remove useless code

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAGsJ_4zn158TQV7Nc+vK-kmu6S4kOiFSZyUO7aK9dhwhrEq2cw@mail.gmail.com>
Date: Mon, 28 Apr 2025 23:03:54 +1200
From: Barry Song <21cnbao@...il.com>
To: David Hildenbrand <david@...hat.com>
Cc: Feng Lee <379943137@...com>, akpm@...ux-foundation.org, ryan.roberts@....com, 
	libang.li@...group.com, peterx@...hat.com, maobibo@...ngson.cn, 
	lance.yang@...ux.dev, anshuman.khandual@....com, trivial@...nel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: remove useless code

On Mon, Apr 28, 2025 at 7:17 PM David Hildenbrand <david@...hat.com> wrote:
>
> On 27.04.25 10:22, Barry Song wrote:
> > On Sun, Apr 27, 2025 at 2:16 PM Feng Lee <379943137@...com> wrote:
> >>
> >> Remove unused conditional macros.
> >>
> >> Signed-off-by: Feng Lee <379943137@...com>
> >> ---
> >>   include/linux/pgtable.h | 2 --
> >>   1 file changed, 2 deletions(-)
> >>
> >> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> >> index b50447ef1c92..47c5a54b7551 100644
> >> --- a/include/linux/pgtable.h
> >> +++ b/include/linux/pgtable.h
> >> @@ -1164,9 +1164,7 @@ static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio)
> >>   }
> >>   #endif
> >>
> >> -#ifndef __HAVE_ARCH_PGD_OFFSET_GATE
> >>   #define pgd_offset_gate(mm, addr)      pgd_offset(mm, addr)
> >> -#endif
> >
> > Do you know who else had pgd_offset_gate() before except ia64?
> >
> > /* Look up a pgd entry in the gate area.  On IA-64, the gate-area
> >     resides in the kernel-mapped segment, hence we use pgd_offset_k()
> >     here.  */
> > #define pgd_offset_gate(mm, addr) pgd_offset_k(addr)
> >
> > btw, do we still
> > need pgd_offset_gate() given that nobody needs it now?
> >
> >     1   1168  include/linux/pgtable.h <<GLOBAL>>
> >               #define pgd_offset_gate(mm, addr) pgd_offset(mm, addr)
> >
> >     2   1112  mm/gup.c <<get_gate_page>>
> >               pgd = pgd_offset_gate(mm, address);
> >
>
> Right, we should just remove pgd_offset_gate() completely in this patch
> and simply make the single caller use pgd_offset().

Yes, exactly. The original patch doesn’t seem to be appropriate.

>
> I think we can even do:
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 84461d384ae2b..05dd87ccce155 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1106,10 +1106,7 @@ static int get_gate_page(struct mm_struct *mm,
> unsigned long address,
>          /* user gate pages are read-only */
>          if (gup_flags & FOLL_WRITE)
>                  return -EFAULT;
> -       if (address > TASK_SIZE)
> -               pgd = pgd_offset_k(address);
> -       else
> -               pgd = pgd_offset_gate(mm, address);
> +       pgd = pgd_offset(address);
>          if (pgd_none(*pgd))
>                  return -EFAULT;
>          p4d = p4d_offset(pgd, address);
>
> Unless I am missing something important :)

Technically, it appears to be correct. However, it seems that
pgd_offset_k is primarily used to improve readability by
distinguishing between kernel space and user space?

>
> --
> Cheers,
>
> David / dhildenb
>

Thanks
Barry