1. Patchew
  2. linux
  3. View series

[PATCH] mm: remove useless code

Feng Lee posted 1 patch 9 months, 2 weeks ago 
include/linux/pgtable.h | 2 --
1 file changed, 2 deletions(-)
[PATCH] mm: remove useless code
Posted by Feng Lee 9 months, 2 weeks ago 
Remove unused conditional macros.

Signed-off-by: Feng Lee <379943137@qq.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
 
 #ifndef __HAVE_ARCH_MOVE_PTE
 #define move_pte(pte, old_addr, new_addr)	(pte)
-- 
2.49.0
Re: [PATCH] mm: remove useless code
Posted by Barry Song 9 months, 2 weeks ago 
On Sun, Apr 27, 2025 at 2:16 PM Feng Lee <379943137@qq.com> wrote:
>
> Remove unused conditional macros.
>
> Signed-off-by: Feng Lee <379943137@qq.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);

>
>  #ifndef __HAVE_ARCH_MOVE_PTE
>  #define move_pte(pte, old_addr, new_addr)      (pte)
> --
> 2.49.0

Thanks
Barry
Re: [PATCH] mm: remove useless code
Posted by David Hildenbrand 9 months, 2 weeks ago 
On 27.04.25 10:22, Barry Song wrote:
> On Sun, Apr 27, 2025 at 2:16 PM Feng Lee <379943137@qq.com> wrote:
>>
>> Remove unused conditional macros.
>>
>> Signed-off-by: Feng Lee <379943137@qq.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().

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 :)

-- 
Cheers,

David / dhildenb
Re: [PATCH] mm: remove useless code
Posted by Barry Song 9 months, 2 weeks ago 
On Mon, Apr 28, 2025 at 7:17 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 27.04.25 10:22, Barry Song wrote:
> > On Sun, Apr 27, 2025 at 2:16 PM Feng Lee <379943137@qq.com> wrote:
> >>
> >> Remove unused conditional macros.
> >>
> >> Signed-off-by: Feng Lee <379943137@qq.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
Re: [PATCH] mm: remove useless code
Posted by Feng Lee 9 months, 2 weeks ago 
> On Mon, Apr 28, 2025 at 7:17 PM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 27.04.25 10:22, Barry Song wrote:
>>> On Sun, Apr 27, 2025 at 2:16 PM Feng Lee <379943137@qq.com> wrote:
>>>>
>>>> Remove unused conditional macros.
>>>>
>>>> Signed-off-by: Feng Lee <379943137@qq.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?

Agreed, readability seems to be the key consideration here.

>
>>
>> --
>> Cheers,
>>
>> David / dhildenb
>>
>
> Thanks
> Barry

Best regards,
Feng
Re: [PATCH] mm: remove useless code
Posted by David Hildenbrand 9 months, 2 weeks ago 
On 28.04.25 13:03, Barry Song wrote:
> On Mon, Apr 28, 2025 at 7:17 PM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 27.04.25 10:22, Barry Song wrote:
>>> On Sun, Apr 27, 2025 at 2:16 PM Feng Lee <379943137@qq.com> wrote:
>>>>
>>>> Remove unused conditional macros.
>>>>
>>>> Signed-off-by: Feng Lee <379943137@qq.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?

Yeah, but this is GUP ... ("user") ... looks like that check/handling 
was in there ever since git happened.

get_gate_vma() only exists on x86-64 and uml.

I wonder if that could ever actually reside > TASK_SIZE such that we 
would even need that.

But this whole gate stuff is confusing ... IIRC it's a single VMA shared 
by all processes, and not actually linked in the maple tree etc.

-- 
Cheers,

David / dhildenb
Re: [PATCH] mm: remove useless code
Posted by Barry Song 9 months, 2 weeks ago 
On Tue, Apr 29, 2025 at 1:59 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 28.04.25 13:03, Barry Song wrote:
> > On Mon, Apr 28, 2025 at 7:17 PM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 27.04.25 10:22, Barry Song wrote:
> >>> On Sun, Apr 27, 2025 at 2:16 PM Feng Lee <379943137@qq.com> wrote:
> >>>>
> >>>> Remove unused conditional macros.
> >>>>
> >>>> Signed-off-by: Feng Lee <379943137@qq.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?
>
> Yeah, but this is GUP ... ("user") ... looks like that check/handling
> was in there ever since git happened.
>
> get_gate_vma() only exists on x86-64 and uml.
>
> I wonder if that could ever actually reside > TASK_SIZE such that we
> would even need that.

I assume that reside > TASK_SIZE can only be true on 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)

Since IA64 is dead, is the code also dead? It seems we can safely move
forward with the approach you're proposing.

>
> But this whole gate stuff is confusing ... IIRC it's a single VMA shared
> by all processes, and not actually linked in the maple tree etc.
>
> --
> Cheers,
>
> David / dhildenb
>

Thanks
Barry
Re: [PATCH] mm: remove useless code
Posted by David Hildenbrand 9 months, 1 week ago 
>> Yeah, but this is GUP ... ("user") ... looks like that check/handling
>> was in there ever since git happened.
>>
>> get_gate_vma() only exists on x86-64 and uml.
>>
>> I wonder if that could ever actually reside > TASK_SIZE such that we
>> would even need that.
> 
> I assume that reside > TASK_SIZE can only be true on 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)
> 
> Since IA64 is dead, is the code also dead? It seems we can safely move
> forward with the approach you're proposing.

Thanks for verifying!

-- 
Cheers,

David / dhildenb

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.