mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm: remove useless code
@ 2025-04-27  6:13 Feng Lee
  2025-04-27  8:22 ` Barry Song
  0 siblings, 1 reply; 9+ messages in thread
From: Feng Lee @ 2025-04-27  6:13 UTC (permalink / raw)
  To: akpm
  Cc: david, ryan.roberts, libang.li, peterx, maobibo, baohua,
	lance.yang, anshuman.khandual, trivial, linux-kernel, Feng Lee

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-27  6:13 [PATCH] mm: remove useless code Feng Lee
@ 2025-04-27  8:22 ` Barry Song
  2025-04-28  7:17   ` David Hildenbrand
  0 siblings, 1 reply; 9+ messages in thread
From: Barry Song @ 2025-04-27  8:22 UTC (permalink / raw)
  To: Feng Lee
  Cc: akpm, david, ryan.roberts, libang.li, peterx, maobibo,
	lance.yang, anshuman.khandual, trivial, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-27  8:22 ` Barry Song
@ 2025-04-28  7:17   ` David Hildenbrand
  2025-04-28 11:03     ` Barry Song
  0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand @ 2025-04-28  7:17 UTC (permalink / raw)
  To: Barry Song, Feng Lee
  Cc: akpm, ryan.roberts, libang.li, peterx, maobibo, lance.yang,
	anshuman.khandual, trivial, linux-kernel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-28  7:17   ` David Hildenbrand
@ 2025-04-28 11:03     ` Barry Song
  2025-04-28 13:59       ` David Hildenbrand
  2025-04-29  8:13       ` Feng Lee
  0 siblings, 2 replies; 9+ messages in thread
From: Barry Song @ 2025-04-28 11:03 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Feng Lee, akpm, ryan.roberts, libang.li, peterx, maobibo,
	lance.yang, anshuman.khandual, trivial, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-28 11:03     ` Barry Song
@ 2025-04-28 13:59       ` David Hildenbrand
  2025-04-30 23:23         ` Barry Song
  2025-04-29  8:13       ` Feng Lee
  1 sibling, 1 reply; 9+ messages in thread
From: David Hildenbrand @ 2025-04-28 13:59 UTC (permalink / raw)
  To: Barry Song
  Cc: Feng Lee, akpm, ryan.roberts, libang.li, peterx, maobibo,
	lance.yang, anshuman.khandual, trivial, linux-kernel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-28 11:03     ` Barry Song
  2025-04-28 13:59       ` David Hildenbrand
@ 2025-04-29  8:13       ` Feng Lee
  1 sibling, 0 replies; 9+ messages in thread
From: Feng Lee @ 2025-04-29  8:13 UTC (permalink / raw)
  To: 21cnbao
  Cc: david, akpm, ryan.roberts, libang.li, peterx, maobibo,
	lance.yang, anshuman.khandual, trivial, linux-kernel

> 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-28 13:59       ` David Hildenbrand
@ 2025-04-30 23:23         ` Barry Song
  2025-05-08 12:47           ` Feng Lee
  2025-05-08 14:39           ` David Hildenbrand
  0 siblings, 2 replies; 9+ messages in thread
From: Barry Song @ 2025-04-30 23:23 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Feng Lee, akpm, ryan.roberts, libang.li, peterx, maobibo,
	lance.yang, anshuman.khandual, trivial, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-30 23:23         ` Barry Song
@ 2025-05-08 12:47           ` Feng Lee
  2025-05-08 14:39           ` David Hildenbrand
  1 sibling, 0 replies; 9+ messages in thread
From: Feng Lee @ 2025-05-08 12:47 UTC (permalink / raw)
  To: 21cnbao
  Cc: 379943137, akpm, anshuman.khandual, david, lance.yang, libang.li,
	linux-kernel, maobibo, peterx, ryan.roberts, trivial

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3903 bytes --]

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

Thank you all sincerely for your patience. I will carefully incorporate 
the valuable feedback into the necessary adjustments for the upcoming 
patch version.

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

Best regards,
Feng


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm: remove useless code
  2025-04-30 23:23         ` Barry Song
  2025-05-08 12:47           ` Feng Lee
@ 2025-05-08 14:39           ` David Hildenbrand
  1 sibling, 0 replies; 9+ messages in thread
From: David Hildenbrand @ 2025-05-08 14:39 UTC (permalink / raw)
  To: Barry Song
  Cc: Feng Lee, akpm, ryan.roberts, libang.li, peterx, maobibo,
	lance.yang, anshuman.khandual, trivial, linux-kernel


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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-05-08 14:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-27  6:13 [PATCH] mm: remove useless code Feng Lee
2025-04-27  8:22 ` Barry Song
2025-04-28  7:17   ` David Hildenbrand
2025-04-28 11:03     ` Barry Song
2025-04-28 13:59       ` David Hildenbrand
2025-04-30 23:23         ` Barry Song
2025-05-08 12:47           ` Feng Lee
2025-05-08 14:39           ` David Hildenbrand
2025-04-29  8:13       ` Feng Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®