From: Oleg Nesterov <oleg@redhat.com>
To: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@elte.hu>,
Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
Anton Arapov <anton@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] uprobes: install_breakpoint() should fail if is_swbp_insn() == T
Date: Fri, 1 Jun 2012 19:20:55 +0200 [thread overview]
Message-ID: <20120601172055.GA24798@redhat.com> (raw)
In-Reply-To: <20120601163315.GJ24279@linux.vnet.ibm.com>
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;
}
next prev parent reply other threads:[~2012-06-01 18:08 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-30 16:57 [PATCH 0/3] uprobes: misc minor changes Oleg Nesterov
2012-05-30 16:58 ` [PATCH 1/3] uprobes: install_breakpoint() should fail if is_swbp_insn() == T Oleg Nesterov
2012-05-30 17:28 ` Peter Zijlstra
2012-05-30 17:37 ` Srikar Dronamraju
2012-05-30 17:49 ` Peter Zijlstra
2012-05-30 17:54 ` Peter Zijlstra
2012-05-30 18:03 ` Peter Zijlstra
2012-05-30 18:04 ` Srikar Dronamraju
2012-05-30 18:21 ` Peter Zijlstra
2012-05-31 18:53 ` Oleg Nesterov
2012-06-01 15:53 ` Oleg Nesterov
2012-06-01 16:33 ` Srikar Dronamraju
2012-06-01 17:20 ` Oleg Nesterov [this message]
2012-06-01 18:31 ` Oleg Nesterov
2012-06-02 18:21 ` Oleg Nesterov
2012-06-01 16:47 ` Srikar Dronamraju
2012-06-01 18:38 ` Oleg Nesterov
2012-05-31 12:15 ` Peter Zijlstra
2012-05-31 12:54 ` Srikar Dronamraju
2012-05-30 16:58 ` [PATCH 2/3] uprobes: remove the unnecessary initialization in add_utask() Oleg Nesterov
2012-05-30 16:58 ` [PATCH 3/3] uprobes: simplify the usage of uprobe->pending_list Oleg Nesterov
2012-05-30 17:48 ` Srikar Dronamraju
2012-05-30 18:10 ` Oleg Nesterov
2012-05-30 18:23 ` Srikar Dronamraju
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120601172055.GA24798@redhat.com \
--to=oleg@redhat.com \
--cc=ananth@in.ibm.com \
--cc=anton@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=srikar@linux.vnet.ibm.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.