On Wed, Aug 21, 2024 at 09:33:06AM GMT, Jeff Xu wrote:
On Wed, Aug 21, 2024 at 9:24 AM Pedro Falcato pedro.falcato@gmail.com wrote:
On Wed, Aug 21, 2024 at 5:16 PM Jeff Xu jeffxu@chromium.org wrote:
On Fri, Aug 16, 2024 at 5:18 PM Pedro Falcato pedro.falcato@gmail.com wrote:
We were doing an extra mmap tree traversal just to check if the entire range is modifiable. This can be done when we iterate through the VMAs instead.
Signed-off-by: Pedro Falcato pedro.falcato@gmail.com
mm/mmap.c | 11 +---------- mm/vma.c | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c index 3af256bacef3..30ae4cb5cec9 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1740,16 +1740,7 @@ int do_vma_munmap(struct vma_iterator *vmi, struct vm_area_struct *vma, unsigned long start, unsigned long end, struct list_head *uf, bool unlock) {
struct mm_struct *mm = vma->vm_mm;/** Check if memory is sealed, prevent unmapping a sealed VMA.* can_modify_mm assumes we have acquired the lock on MM.*/if (unlikely(!can_modify_mm(mm, start, end)))return -EPERM;Another approach to improve perf is to clone the vmi (since it already point to the first vma), and pass the cloned vmi/vma into can_modify_mm check, that will remove the cost of re-finding the first VMA.
The can_modify_mm then continues from cloned VMI/vma till the end of address range, there will be some perf cost there. However, most address ranges in the real world are within a single VMA, in practice, the perf cost is the same as checking the single VMA, 99.9% case.
This will help preserve the nice sealing feature (if one of the vma is sealed, the entire address range is not modified)
Please drop it. No one wants to preserve this. Everyone is in sync when it comes to the solution except you.
Still, this is another option that will very likely address the perf issue.
Nack to your approach. Feel free to send a follow up series replacing Pedro's with yours for review if you feel differently, and stop stalling things. Thanks.
-Jeff
-- Pedro