Thread (25 messages) 25 messages, 6 authors, 2024-08-12

Re: [PATCH 1/4] mm: Add optional close() to struct vm_special_mapping

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2024-08-12 08:22:47
Also in: linux-mm, lkml

"Liam R. Howlett" [off-list ref] writes:
* Michael Ellerman [off-list ref] [240807 08:41]:
quoted
Add an optional close() callback to struct vm_special_mapping. It will
be used, by powerpc at least, to handle unmapping of the VDSO.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 include/linux/mm_types.h | 2 ++
 mm/mmap.c                | 3 +++
 2 files changed, 5 insertions(+)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 485424979254..ef32d87a3adc 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1313,6 +1313,8 @@ struct vm_special_mapping {
 
 	int (*mremap)(const struct vm_special_mapping *sm,
 		     struct vm_area_struct *new_vma);
nit: missing new line?
Ack.
quoted
+	void (*close)(const struct vm_special_mapping *sm,
+		      struct vm_area_struct *vma);
 };
 
 enum tlb_flush_reason {
diff --git a/mm/mmap.c b/mm/mmap.c
index d0dfc85b209b..24bd6aa9155c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3624,6 +3624,9 @@ static vm_fault_t special_mapping_fault(struct vm_fault *vmf);
  */
The above comment should probably be expanded to explain what this is
about, or removed.
I expanded it slightly, happy for others to wordsmith it further.
quoted
 static void special_mapping_close(struct vm_area_struct *vma)
 {
+	const struct vm_special_mapping *sm = vma->vm_private_data;
+	if (sm->close)
+		sm->close(sm, vma);
Right now we have the same sort of situation for mremap calls on
special: we have a call to the specific vma mremap() function.
...
So, are we missing an opportunity to avoid every arch having the same
implementation here (that will evolve into random bugs existing in some
archs for years before someone realises the cloned code wasn't fixed)?
Do we already have a fix in ppc for the size checking that doesn't exist
in the other archs in the case of mremap?
I took this as more of a meta comment/rant :)

Yes I agree the implementation should eventually be generic, but this series
is just about moving the existing powerpc behaviour from arch_unmap()
into this hook. 

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help