* [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 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
* 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
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®