mm/memory: improve insert_pages() error handling
Subsystems (2)
v3 cover mm/memory: improve insert_pages() error handling
Improve insert_pages() error handling.
The first patch simplifies error handling by initializing the error status to zero and assigning error codes at their respective failure sites.
The second patch returns -ENOMEM when walk_to_pmd() fails. A NULL return from walk_to_pmd() indicates failure to allocate an upper page-table level, so -ENOMEM is more appropriate than -EFAULT and is consistent with the subsequent pte_alloc() failure.
Changes in v3:
* drop the walk_to_pmd() rename following review
* use the mm/memory: prefix consistently across the series
* carry review tags from v2
* rebase onto current mm-new
Avi Weiss (2):
mm/memory: simplify error handling in insert_pages() mm/memory: return -ENOMEM for page-table allocation failure in
insert_pages()
mm/memory.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
base-commit: d2aad7fdcda7ae8a726926f2d6de7fe9e8ee7563
Signature · 1 line
2.43.0
PATCH v3 1/2 mm/memory: simplify error handling in insert_pages()
Initialize error return status to zero and then set it as needed at each point of failure.
Assign -ENOMEM explicitly when pte_alloc() fails as the pte_alloc() macro returns a boolean.
--- mm/memory.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
mm/memory.c +11 -11
@@ -2565,20 +2565,22 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,unsigned long curr_page_idx =0 ;unsigned long remaining_pages_total = *num;unsigned long pages_to_write_in_pmd; -int ret; +int err =0 ; more: - ret = -EFAULT ; pmd =walk_to_pmd (mm, addr); -if (!pmd) +if (!pmd) { + err = -EFAULT ; goto out; + } pages_to_write_in_pmd = min_t(unsigned long, remaining_pages_total,PTRS_PER_PTE -pte_index (addr));/* Allocate the PTE if necessary; takes PMD lock once only. */ - ret = -ENOMEM ; -if (pte_alloc (mm, pmd)) +if (pte_alloc (mm, pmd)) { + err = -ENOMEM ; goto out; + }while (pages_to_write_in_pmd) {int pte_idx =0 ;
@@ -2586,15 +2588,14 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr, start_pte =pte_offset_map_lock (mm, pmd, addr, &pte_lock);if (!start_pte) { - ret = -EFAULT ; + err = -EFAULT ; goto out; }for (pte = start_pte; pte_idx < batch_size; ++pte, ++pte_idx) { -int err =insert_page_in_batch_locked (vma, pte, - addr, pages[curr_page_idx], prot); + err =insert_page_in_batch_locked (vma, pte, addr, + pages[curr_page_idx], prot);if (unlikely (err)) {pte_unmap_unlock (start_pte, pte_lock); - ret = err; remaining_pages_total -= pte_idx; goto out; }
@@ -2607,10 +2608,9 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr, }if (remaining_pages_total) goto more; - ret =0 ; out: *num = remaining_pages_total; -return ret; +return err; }/**
Signature · 1 line
2.43.0
PATCH v3 2/2 mm/memory: return -ENOMEM for page-table allocation failure in insert_pages()
In reply to Avi Weisswalk_to_pmd() returns NULL only when p4d_alloc(), pud_alloc(), or pmd_alloc() fails. These are page-table allocation failures, but insert_pages() currently reports them as -EFAULT.
Return -ENOMEM instead, consistent with the subsequent pte_alloc() failure and with the single-page insert_page() path, which reports failure of the same page-table allocation chain as -ENOMEM.
Address and range validation failures in vm_insert_pages() continue to return -EFAULT. Keep the later -EFAULT return for pte_offset_map_lock(), which is not an allocation failure.
--- mm/memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/memory.c +1 -1
@@ -2569,7 +2569,7 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr, more: pmd =walk_to_pmd (mm, addr);if (!pmd) { - err = -EFAULT ; + err = -ENOMEM ; goto out; }
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerQuoted 11 lines · Avi Weiss
Improve
insert_pages()error handling.The first patch simplifies error handling by initializing the error status to zero and assigning error codes at their respective failure sites.
The second patch returns
-ENOMEMwhenwalk_to_pmd()fails. A NULL return fromwalk_to_pmd()indicates failure to allocate an upper page-table level, so-ENOMEMis more appropriate than-EFAULTand is consistent with the subsequentpte_alloc()failure.
Quoted 6 lines
Changes in v3:
* drop the
walk_to_pmd()rename following review
* use the mm/memory: prefix consistently across the series
* carry review tags from v2
* rebase onto current mm-new
Double-checked, LGTM.
Signature · 3 lines
Cheers, David
Re: [PATCH v3 1/2] mm/memory: simplify error handling in insert_pages()
In reply to Avi WeissQuoted 8 lines · Avi Weiss
Initialize error return status to zero and then set it as needed at each point of failure.
Assign
-ENOMEMexplicitly whenpte_alloc()fails as thepte_alloc()macro returns a boolean.Signed-off-by Avi Weiss <[email protected]>
LGTM so:
Quoted 68 lines
--- mm/memory.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)mm/memory.c +11
-12@@ -2565,20 +2565,22 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,unsigned long curr_page_idx =0 ;unsigned long remaining_pages_total = *num;unsigned long pages_to_write_in_pmd; -int ret; +int err =0 ; more: - ret = -EFAULT ; pmd =walk_to_pmd (mm, addr); -if (!pmd) +if (!pmd) { + err = -EFAULT ; goto out; + } pages_to_write_in_pmd = min_t(unsigned long, remaining_pages_total,PTRS_PER_PTE -pte_index (addr));/* Allocate the PTE if necessary; takes PMD lock once only. */ - ret = -ENOMEM ; -if (pte_alloc (mm, pmd)) +if (pte_alloc (mm, pmd)) { + err = -ENOMEM ; goto out; + }while (pages_to_write_in_pmd) {int pte_idx =0 ;@@ -2586,15 +2588,14 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr, start_pte =pte_offset_map_lock (mm, pmd, addr, &pte_lock);if (!start_pte) { - ret = -EFAULT ; + err = -EFAULT ; goto out; }for (pte = start_pte; pte_idx < batch_size; ++pte, ++pte_idx) { -int err =insert_page_in_batch_locked (vma, pte, - addr, pages[curr_page_idx], prot); + err =insert_page_in_batch_locked (vma, pte, addr, + pages[curr_page_idx], prot);if (unlikely (err)) {pte_unmap_unlock (start_pte, pte_lock); - ret = err; remaining_pages_total -= pte_idx; goto out; }@@ -2607,10 +2608,9 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr, }if (remaining_pages_total) goto more; - ret =0 ; out: *num = remaining_pages_total; -return ret; +return err; }/** --
Quoted 2 lines
2.43.0
--
Cheers, Lorenzo