Re: [PATCH v1] mm: drop NULL return check of pte_offset_map_lock()

From: Vlastimil Babka
Date: Fri May 19 2017 - 09:16:07 EST


On 05/18/2017 08:42 AM, Naoya Horiguchi wrote:
> pte_offset_map_lock() finds and takes ptl, and returns pte.
> But some callers return without unlocking the ptl when pte == NULL,
> which seems weird.
>
> Git history said that !pte check in change_pte_range() was introduced in
> commit 1ad9f620c3a2 ("mm: numa: recheck for transhuge pages under lock
> during protection changes") and still remains after commit 175ad4f1e7a2
> ("mm: mprotect: use pmd_trans_unstable instead of taking the pmd_lock")
> which partially reverts 1ad9f620c3a2. So I think that it's just dead code.
>
> Many other caller of pte_offset_map_lock() never check NULL return, so
> let's do likewise.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>

Acked-by: Vlastimil Babka <vbabka@xxxxxxx>

> ---
> mm/memory.c | 2 --
> mm/mprotect.c | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git v4.11-rc6-mmotm-2017-04-13-14-50/mm/memory.c v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/memory.c
> index 8ae6700..c17fad1d 100644
> --- v4.11-rc6-mmotm-2017-04-13-14-50/mm/memory.c
> +++ v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/memory.c
> @@ -4040,8 +4040,6 @@ static int __follow_pte_pmd(struct mm_struct *mm, unsigned long address,
> goto out;
>
> ptep = pte_offset_map_lock(mm, pmd, address, ptlp);
> - if (!ptep)
> - goto out;
> if (!pte_present(*ptep))
> goto unlock;
> *ptepp = ptep;
> diff --git v4.11-rc6-mmotm-2017-04-13-14-50/mm/mprotect.c v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/mprotect.c
> index 8fd010f..d60a1ee 100644
> --- v4.11-rc6-mmotm-2017-04-13-14-50/mm/mprotect.c
> +++ v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/mprotect.c
> @@ -58,8 +58,6 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
> * reading.
> */
> pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> - if (!pte)
> - return 0;
>
> /* Get target node for single threaded private VMAs */
> if (prot_numa && !(vma->vm_flags & VM_SHARED) &&
>