[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <tencent_27AC10231AC4AB1EEC24EBB53960919BD907@qq.com>
Date: Thu, 8 May 2025 20:47:51 +0800
From: Feng Lee <379943137@...com>
To: 21cnbao@...il.com
Cc: 379943137@...com,
akpm@...ux-foundation.org,
anshuman.khandual@....com,
david@...hat.com,
lance.yang@...ux.dev,
libang.li@...group.com,
linux-kernel@...r.kernel.org,
maobibo@...ngson.cn,
peterx@...hat.com,
ryan.roberts@....com,
trivial@...nel.org
Subject: Re: [PATCH] mm: remove useless code
> On Tue, Apr 29, 2025 at 1:59 AM David Hildenbrand <david@...hat.com> wrote:
>>
>> On 28.04.25 13:03, Barry Song wrote:
>>> 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?
>>
>> 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
Powered by blists - more mailing lists