Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1643875 > unrolled thread
| Started by | Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> |
|---|---|
| First post | 2017-05-18 08:50 +0200 |
| Last post | 2017-05-19 15:20 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v1] mm: drop NULL return check of pte_offset_map_lock() Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-05-18 08:50 +0200
Re: [PATCH v1] mm: drop NULL return check of pte_offset_map_lock() Vlastimil Babka <vbabka@suse.cz> - 2017-05-19 15:20 +0200
| From | Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> |
|---|---|
| Date | 2017-05-18 08:50 +0200 |
| Subject | [PATCH v1] mm: drop NULL return check of pte_offset_map_lock() |
| Message-ID | <tInq1-6iy-5@gated-at.bofh.it> |
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@ah.jp.nec.com>
---
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) &&
--
2.7.0
[toc] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2017-05-19 15:20 +0200 |
| Message-ID | <tIPYZ-2G4-9@gated-at.bofh.it> |
| In reply to | #1643875 |
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@ah.jp.nec.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> 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) &&
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web