Messages in this thread | | | Date | Fri, 1 Jun 2012 19:20:55 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/3] uprobes: install_breakpoint() should fail if is_swbp_insn() == T |
| |
On 06/01, Srikar Dronamraju wrote: > > * Oleg Nesterov <oleg@redhat.com> [2012-06-01 17:53:12]: > > > But. Doesn't this mean we can greatly simplify register_for_each_vma() > > and make it O(n) ? > > > > Unless I missed something, we can simply create the list of > > mm/vaddr structures under ->i_mmap_mutex (vma_prio_tree_foreach), then > > register_for_each_vma() can process the list and that is all. > > > If I remember correctly, we cannot allocate the list elements under > i_mmap_mutex. We dont know how many list elements to allocate. > > This is what Peter had to say : https://lkml.org/lkml/2011/6/27/72 > > "Because we try to take i_mmap_mutex during reclaim, trying to unmap > pages. So suppose we do an allocation while holding i_mmap_mutex, find > there's no free memory, try and unmap a page in order to free it, and > we're stuck."
Yes, try_to_unmap.
But. What do you think about the pseudo-code below? Only to illustrate the approach, the code is not complete.
In the "likely" case we do vma_prio_tree_foreach() twice, this is better than the current quadratic behaviour.
Oleg.
struct map_info { struct map_info *next; struct mm_struct *mm; loff_t vaddr; };
static struct map_info * build_map_info_list(struct address_space *mapping, loff_t offset, bool is_register) { struct map_info *prev = NULL; struct map_info *curr; int more;
again: more = 0; curr = NULL; vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) { struct map_info *info = prev;
if (!valid_vma(vma, is_register)) continue;
if (!info) { more++; continue; }
if (!atomic_inc_not_zero(&vma->vm_mm->mm_users)) contunue;
prev = info->next;
info->mm = vma->vm_mm; info->vaddr = vma_address(vma, offset);
info->next = curr; curr = info; }
if (!more) { while (prev) { map_info *tmp = prev; prev = prev->next; kfree(tmp); }
return curr; }
prev = curr;
while (curr) { mmput(curr->mm); curr = curr->next; }
while (more--) { struct map_info *info = kmalloc(...); if (!info) return ERR_PTR(-ENOMEM); // MEMORY LEAK info->next = prev; prev = info; }
goto again; }
| |