mm/memory.c: Convert to use vmf_error()

PATCH In review Souptick Joarder 2 participants 3 messages Sep 5, 2019 – Sep 6, 2019

Subsystems (2)

Memory management MM core

lore

Share & tools

lore

Souptick Joarder

author

2019-09-05 18:37 UTC #0 lore raw

PATCH 1/1 mm/memory.c: Convert to use vmf_error()

Convert to use vmf_error() here.

Signed-off-by Souptick Joarder <[email protected]>

---
 mm/memory.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

mm/memory.c +4 -7

index e2bb51b..1302be32 100644
@@ -1750,13 +1750,10 @@ static vm_fault_t __vm_insert_mixed(struct vm_area_struct *vma,
 	} else {
 		return insert_pfn(vma, addr, pfn, pgprot, mkwrite);
 	}
-
-	if (err == -ENOMEM)
-		return VM_FAULT_OOM;
-	if (err < 0 && err != -EBUSY)
-		return VM_FAULT_SIGBUS;
-
-	return VM_FAULT_NOPAGE;
+	if (!err || err == -EBUSY)
+		return VM_FAULT_NOPAGE;
+	else
+		return vmf_error(err);
 }
 
 vm_fault_t vmf_insert_mixed(struct vm_area_struct *vma, unsigned long addr,

Signature · 1 line

1.9.1

Matthew Wilcox

2019-09-05 18:59 UTC #1 lore raw

Quoted 7 lines · Souptick Joarder

+++ b/mm/memory.c

mm/memory.c +0 -2

@@ -1750,13 +1750,10 @@ static vm_fault_t __vm_insert_mixed(struct vm_area_struct *vma,
 	} else {
 		return insert_pfn(vma, addr, pfn, pgprot, mkwrite);
 	}
-
-	if (err == -ENOMEM)

mm/memory.c static vm_fault_t __vm_insert_mixed(struct vm_area_struct *vma, · line 1757

Quoted 10 lines

mm/memory.c +4 -5

-		return VM_FAULT_OOM;
-	if (err < 0 && err != -EBUSY)
-		return VM_FAULT_SIGBUS;
-
-	return VM_FAULT_NOPAGE;
+	if (!err || err == -EBUSY)
+		return VM_FAULT_NOPAGE;
+	else
+		return vmf_error(err);
 }

My plan is to convert insert_page() to return a VM_FAULT error code like insert_pfn() does. Need to finish off the vm_insert_page() conversions first ;-)

Souptick Joarder

author

2019-09-06 12:24 UTC #2 lore raw

Quoted 15 lines · Matthew Wilcox

Quoted 13 lines · Souptick Joarder

+++ b/mm/memory.c

mm/memory.c +1 -7

@@ -1750,13 +1750,10 @@ static vm_fault_t __vm_insert_mixed(struct vm_area_struct *vma,
      } else {
              return insert_pfn(vma, addr, pfn, pgprot, mkwrite);
      }
-
-     if (err == -ENOMEM)
-             return VM_FAULT_OOM;
-     if (err < 0 && err != -EBUSY)
-             return VM_FAULT_SIGBUS;
-
-     return VM_FAULT_NOPAGE;
+     if (!err || err == -EBUSY)

Quoted 8 lines

mm/memory.c static vm_fault_t __vm_insert_mixed(struct vm_area_struct *vma, · line 1757
Quoted 4 lines
mm/memory.c +3 -0
+             return VM_FAULT_NOPAGE;
+     else
+             return vmf_error(err);
 }
My plan is to convert insert_page() to return a VM_FAULT error code like insert_pfn() does. Need to finish off the vm_insert_page() conversions first ;-)

Previously we have problem while converting vm_insert_page() to return vm_fault_t.

vm_insert_page() is called from different drivers. Some of them are already converted
to use vm_map_pages()/ vm_map_pages_zero(). But still we left with few users.

drivers/media/usb/usbvision/usbvision-video.c#L1045 mm/vmalloc.c#L2969

These 2 can be converted with something like vm_map_vmalloc_pages(). I am working on it. Will post it in sometime.

drivers/android/binder_alloc.c#L259 (have objection) drivers/infiniband/hw/efa/efa_verbs.c#L1701 drivers/infiniband/hw/mlx5/main.c#L2085 (have objection as using vm_map_pages_zero() doesn't make sense) drivers/xen/gntalloc.c#L548 (have limitation) kernel/kcov.c#L297 (have objection) net/ipv4/tcp.c#L1799
net/packet/af_packet.c#L4453

But these are the places where replacing vm_insert_page() is bit difficult/ have objection.
In some cases, maintainers/ reviewers will not agree to replace vm_insert_page().

In other scenario, if we change return type of vm_insert_page() to vm_fault_t, we end up with adding few lines of conversion code from vm_fault_t to errno in drivers which is not a correct way to go with.

Any suggestion, how to solve this ?