uprobes: write_opcode() cleanups

PATCH In review Oleg Nesterov 4 participants 19 messages Jun 24, 2012 – Jul 9, 2012

Subsystems (2)

perf events uprobes

lore

Share & tools

lore

Oleg Nesterov

author maintainer

2012-06-24 15:02 UTC #0 lore raw

v2 cover uprobes: write_opcode() cleanups

Hello,

write_opcode() cleanups resend + new minor fix.

Changes:

- document the new argument in 2/5.

- drop the buggy 5/5, thanks Anton for your quick nack. Probably I'll return to this later, I have another reason for this change.

- so this 5/5 is new.

Srikar, please add your acks unless you have some objections.

Oleg.

Oleg Nesterov

author maintainer

2012-06-24 15:02 UTC #1 lore raw

PATCH v2 1/5 uprobes: don't recheck vma/f_mapping in write_opcode()

write_opcode() rechecks valid_vma() and ->f_mapping, this is pointless. The caller, register_for_each_vma() or uprobe_mmap(), has already done these checks under mmap_sem.

To clarify, uprobe_mmap() checks valid_vma() only, but we can rely on build_probe_list(vm_file->f_mapping->host).

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |   19 +------------------
 1 files changed, 1 insertions(+), 18 deletions(-)

kernel/events/uprobes.c +1 ~~-18~~

index f935327..a2b32a5 100644
@@ -206,33 +206,16 @@ static int write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
 			unsigned long vaddr, uprobe_opcode_t opcode)
 {
 	struct page *old_page, *new_page;
-	struct address_space *mapping;
 	void *vaddr_old, *vaddr_new;
 	struct vm_area_struct *vma;
-	struct uprobe *uprobe;
 	int ret;
+
 retry:
 	/* Read the page with vaddr into memory */
 	ret = get_user_pages(NULL, mm, vaddr, 1, 0, 0, &old_page, &vma);
 	if (ret <= 0)
 		return ret;
 
-	ret = -EINVAL;
-
-	/*
-	 * We are interested in text pages only. Our pages of interest
-	 * should be mapped for read and execute only. We desist from
-	 * adding probes in write mapped pages since the breakpoints
-	 * might end up in the file copy.
-	 */
-	if (!valid_vma(vma, is_swbp_insn(&opcode)))
-		goto put_out;
-
-	uprobe = container_of(auprobe, struct uprobe, arch);
-	mapping = uprobe->inode->i_mapping;
-	if (mapping != vma->vm_file->f_mapping)
-		goto put_out;
-
 	ret = -ENOMEM;
 	new_page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, vaddr);
 	if (!new_page)

Signature · 1 line

1.5.5.1

Srikar Dronamraju

Acked-by on v2 1/5

2012-07-09 10:32 UTC #2 lore raw

Re: [PATCH v2 1/5] uprobes: don't recheck vma/f_mapping in write_opcode()

Quoted 8 lines · Oleg Nesterov

write_opcode() rechecks valid_vma() and ->f_mapping, this is pointless. The caller, register_for_each_vma() or uprobe_mmap(), has already done these checks under mmap_sem.
To clarify, uprobe_mmap() checks valid_vma() only, but we can rely on build_probe_list(vm_file->f_mapping->host).
Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Oleg Nesterov

author maintainer

2012-06-24 15:02 UTC #3 lore raw

PATCH v2 2/5 uprobes: __replace_page() should not use page_address_in_vma()

In reply to Oleg Nesterov

page_address_in_vma(old_page) in __replace_page() is ugly and wrong. The caller already knows the correct virtual address, this page was found by get_user_pages(vaddr).

However, page_address_in_vma() can actually fail if page->mapping was cleared by __delete_from_page_cache() after get_user_pages() returns. But this means the race with page reclaim, write_opcode() should not fail, it should retry and read this page again. Not sure this race is really possible though, page_freeze_refs() logic should prevent it.

We could change __replace_page() to return -EAGAIN in this case, but it would be better to simply use the caller's vaddr and rely on page_check_address().

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

kernel/events/uprobes.c +4 -7

index a2b32a5..6fda799 100644
@@ -127,22 +127,19 @@ static loff_t vma_address(struct vm_area_struct *vma, loff_t offset)
  * based on replace_page in mm/ksm.c
  *
  * @vma:      vma that holds the pte pointing to page
+ * @addr:     address the old @page is mapped at
  * @page:     the cowed page we are replacing by kpage
  * @kpage:    the modified page we replace page by
  *
  * Returns 0 on success, -EFAULT on failure.
  */
-static int __replace_page(struct vm_area_struct *vma, struct page *page, struct page *kpage)
+static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
+				struct page *page, struct page *kpage)
 {
 	struct mm_struct *mm = vma->vm_mm;
-	unsigned long addr;
 	spinlock_t *ptl;
 	pte_t *ptep;
 
-	addr = page_address_in_vma(page, vma);
-	if (addr == -EFAULT)
-		return -EFAULT;
-
 	ptep = page_check_address(page, mm, addr, &ptl, 0);
 	if (!ptep)
 		return -EAGAIN;

@@ -243,7 +240,7 @@ retry:
 		goto unlock_out;
 
 	lock_page(new_page);
-	ret = __replace_page(vma, old_page, new_page);
+	ret = __replace_page(vma, vaddr, old_page, new_page);
 	unlock_page(new_page);
 
 unlock_out:

Signature · 1 line

1.5.5.1

Oleg Nesterov

author maintainer

2012-07-06 16:22 UTC #4 lore raw

Re: [PATCH v2 2/5] uprobes: __replace_page() should not use page_address_in_vma()

Quoted 6 lines · Oleg Nesterov

However, page_address_in_vma() can actually fail if page->mapping was cleared by __delete_from_page_cache() after get_user_pages() returns. But this means the race with page reclaim, write_opcode() should not fail, it should retry and read this page again. Not sure this race is really possible though, page_freeze_refs() logic should prevent it.

This looks a bit confusing, I'll try to to make this more clear... The patch itself was not changed.

------------------------------------------------------------------------------
Subject: [PATCH v2 2/5] uprobes: __replace_page() should not use page_address_in_vma()

page_address_in_vma(old_page) in __replace_page() is ugly and wrong. The caller already knows the correct virtual address, this page was found by get_user_pages(vaddr).

However, page_address_in_vma() can actually fail if page->mapping was cleared by __delete_from_page_cache() after get_user_pages() returns. But this means the race with page reclaim, write_opcode() should not fail, it should retry and read this page again. Probably the race with remove_mapping() is not possible due to page_freeze_refs() logic, but afaics at least shmem_writepage()->shmem_delete_from_page_cache() can clear ->mapping.

We could change __replace_page() to return -EAGAIN in this case, but it would be better to simply use the caller's vaddr and rely on page_check_address().

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

kernel/events/uprobes.c +4 -7

index a2b32a5..6fda799 100644
@@ -127,22 +127,19 @@ static loff_t vma_address(struct vm_area_struct *vma, loff_t offset)
  * based on replace_page in mm/ksm.c
  *
  * @vma:      vma that holds the pte pointing to page
+ * @addr:     address the old @page is mapped at
  * @page:     the cowed page we are replacing by kpage
  * @kpage:    the modified page we replace page by
  *
  * Returns 0 on success, -EFAULT on failure.
  */
-static int __replace_page(struct vm_area_struct *vma, struct page *page, struct page *kpage)
+static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
+				struct page *page, struct page *kpage)
 {
 	struct mm_struct *mm = vma->vm_mm;
-	unsigned long addr;
 	spinlock_t *ptl;
 	pte_t *ptep;
 
-	addr = page_address_in_vma(page, vma);
-	if (addr == -EFAULT)
-		return -EFAULT;
-
 	ptep = page_check_address(page, mm, addr, &ptl, 0);
 	if (!ptep)
 		return -EAGAIN;

@@ -243,7 +240,7 @@ retry:
 		goto unlock_out;
 
 	lock_page(new_page);
-	ret = __replace_page(vma, old_page, new_page);
+	ret = __replace_page(vma, vaddr, old_page, new_page);
 	unlock_page(new_page);
 
 unlock_out:

Signature · 1 line

1.5.5.1

Srikar Dronamraju

Acked-by

2012-07-09 10:29 UTC #5 lore raw

Re: [PATCH v2 2/5] uprobes: __replace_page() should not use page_address_in_vma()

Quoted 15 lines

------------------------------------------------------------------------------
Subject: [PATCH v2 2/5] uprobes: __replace_page() should not use page_address_in_vma()
page_address_in_vma(old_page) in __replace_page() is ugly and wrong. The caller already knows the correct virtual address, this page was found by get_user_pages(vaddr).
However, page_address_in_vma() can actually fail if page->mapping was cleared by __delete_from_page_cache() after get_user_pages() returns. But this means the race with page reclaim, write_opcode() should not fail, it should retry and read this page again. Probably the race with remove_mapping() is not possible due to page_freeze_refs() logic, but afaics at least shmem_writepage()->shmem_delete_from_page_cache() can clear ->mapping.

Quoted 5 lines

We could change __replace_page() to return -EAGAIN in this case, but it would be better to simply use the caller's vaddr and rely on page_check_address().
Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Oleg Nesterov

author maintainer

2012-06-24 15:02 UTC #6 lore raw

PATCH v2 3/5 uprobes: kill write_opcode()->lock_page(new_page)

In reply to Oleg Nesterov

write_opcode() does lock_page(new_page) for no reason. Nobody can see this page until __replace_page() exposes it under ptl lock, and we do nothing with this page after pte_unmap_unlock().

If nothing else, the similar code in do_wp_page() doesn't lock the new page for page_add_new_anon_rmap/set_pte_at_notify.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

kernel/events/uprobes.c +0 -2

index 6fda799..23c562b 100644
@@ -239,9 +239,7 @@ retry:
 	if (ret)
 		goto unlock_out;
 
-	lock_page(new_page);
 	ret = __replace_page(vma, vaddr, old_page, new_page);
-	unlock_page(new_page);
 
 unlock_out:
 	unlock_page(old_page);

Signature · 1 line

1.5.5.1

Srikar Dronamraju

Acked-by on v2 3/5

2012-07-09 10:30 UTC #7 lore raw

Re: [PATCH v2 3/5] uprobes: kill write_opcode()->lock_page(new_page)

Quoted 8 lines · Oleg Nesterov

write_opcode() does lock_page(new_page) for no reason. Nobody can see this page until __replace_page() exposes it under ptl lock, and we do nothing with this page after pte_unmap_unlock().
If nothing else, the similar code in do_wp_page() doesn't lock the new page for page_add_new_anon_rmap/set_pte_at_notify.
Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju<[email protected]>

Oleg Nesterov

author maintainer

2012-06-24 15:03 UTC #8 lore raw

PATCH v2 4/5 uprobes: cleanup and document write_opcode()->lock_page(old_page)

In reply to Oleg Nesterov

The comment above write_opcode()->lock_page(old_page) tells about the race with do_wp_page(). I don't really understand which exactly race it means, but afaics this lock_page() was not enough to close all races with do_wp_page().

Anyway, since 77fc4af1 this code is always called with ->mmap_sem hold for writing so we can forget about do_wp_page().

However, we can't simply remove this lock_page(), and the only (afaics) reason is __replace_page()->try_to_free_swap().

Nothing in write_opcode() needs it, move it into __replace_page() and fix the comment.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)

kernel/events/uprobes.c +14 ~~-13~~

index 23c562b..5db150b 100644
@@ -139,10 +139,15 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
 	struct mm_struct *mm = vma->vm_mm;
 	spinlock_t *ptl;
 	pte_t *ptep;
+	int err;
 
+	/* freeze PageSwapCache() for try_to_free_swap() below */
+	lock_page(page);
+
+	err = -EAGAIN;
 	ptep = page_check_address(page, mm, addr, &ptl, 0);
 	if (!ptep)
-		return -EAGAIN;
+		goto unlock;
 
 	get_page(kpage);
 	page_add_new_anon_rmap(kpage, vma, addr);

@@ -162,7 +167,10 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
 	put_page(page);
 	pte_unmap_unlock(ptep, ptl);
 
-	return 0;
+	err = 0;
+ unlock:
+	unlock_page(page);
+	return err;
 }
 
 /**

@@ -216,15 +224,10 @@ retry:
 	ret = -ENOMEM;
 	new_page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, vaddr);
 	if (!new_page)
-		goto put_out;
+		goto put_old;
 
 	__SetPageUptodate(new_page);
 
-	/*
-	 * lock page will serialize against do_wp_page()'s
-	 * PageAnon() handling
-	 */
-	lock_page(old_page);
 	/* copy the page now that we've got it stable */
 	vaddr_old = kmap_atomic(old_page);
 	vaddr_new = kmap_atomic(new_page);

@@ -237,15 +240,13 @@ retry:
 
 	ret = anon_vma_prepare(vma);
 	if (ret)
-		goto unlock_out;
+		goto put_new;
 
 	ret = __replace_page(vma, vaddr, old_page, new_page);
 
-unlock_out:
-	unlock_page(old_page);
+put_new:
 	page_cache_release(new_page);
-
-put_out:
+put_old:
 	put_page(old_page);
 
 	if (unlikely(ret == -EAGAIN))

Signature · 1 line

1.5.5.1

Srikar Dronamraju

Acked-by on v2 4/5

2012-07-09 10:29 UTC #9 lore raw

Re: [PATCH v2 4/5] uprobes: cleanup and document write_opcode()->lock_page(old_page)

Quoted 8 lines · Oleg Nesterov

The comment above write_opcode()->lock_page(old_page) tells about the race with do_wp_page(). I don't really understand which exactly race it means, but afaics this lock_page() was not enough to close all races with do_wp_page().
Anyway, since 77fc4af1 this code is always called with ->mmap_sem hold for writing so we can forget about do_wp_page().

Quoted 7 lines

However, we can't simply remove this lock_page(), and the only (afaics) reason is __replace_page()->try_to_free_swap().
Nothing in write_opcode() needs it, move it into __replace_page() and fix the comment.
Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Oleg Nesterov

author maintainer

2012-06-24 15:03 UTC #10 lore raw

PATCH v2 5/5 uprobes: __replace_page() needs munlock_vma_page()

In reply to Oleg Nesterov

Like do_wp_page(), __replace_page() should do munlock_vma_page() for the case when the old page still has other !VM_LOCKED mappings. Unfortunately this needs mm/internal.h.

Also, move put_page() outside of ptl lock. This doesn't really matter but looks better.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

kernel/events/uprobes.c +6 -2

index 5db150b..889c62b 100644
@@ -32,6 +32,7 @@
 #include <linux/swap.h>		/* try_to_free_swap */
 #include <linux/ptrace.h>	/* user_enable_single_step */
 #include <linux/kdebug.h>	/* notifier mechanism */
+#include "../../mm/internal.h"	/* munlock_vma_page */
 
 #include <linux/uprobes.h>

@@ -141,7 +142,7 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
 	pte_t *ptep;
 	int err;
 
-	/* freeze PageSwapCache() for try_to_free_swap() below */
+	/* For try_to_free_swap() and munlock_vma_page() below */
 	lock_page(page);
 
 	err = -EAGAIN;

@@ -164,9 +165,12 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
 	page_remove_rmap(page);
 	if (!page_mapped(page))
 		try_to_free_swap(page);
-	put_page(page);
 	pte_unmap_unlock(ptep, ptl);
 
+	if (vma->vm_flags & VM_LOCKED)
+		munlock_vma_page(page);
+	put_page(page);
+
 	err = 0;
  unlock:
 	unlock_page(page);

Signature · 1 line

1.5.5.1

Anton Arapov

2012-06-26 11:56 UTC #11 lore raw

Re: [PATCH v2 5/5] uprobes: __replace_page() needs munlock_vma_page()

Quoted 17 lines · Oleg Nesterov

Like do_wp_page(), __replace_page() should do munlock_vma_page() for the case when the old page still has other !VM_LOCKED mappings. Unfortunately this needs mm/internal.h.
Also, move put_page() outside of ptl lock. This doesn't really matter but looks better.
Signed-off-by Oleg Nesterov <[email protected]>
---
 kernel/events/uprobes.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
kernel/events/uprobes.c +0 -0
index 5db150b..889c62b 100644

kernel/events/uprobes.c line 35

Quoted 4 lines

kernel/events/uprobes.c +1 -0

@@ -32,6 +32,7 @@
 #include <linux/swap.h>		/* try_to_free_swap */
 #include <linux/ptrace.h>	/* user_enable_single_step */
 #include <linux/kdebug.h>	/* notifier mechanism */
+#include "../../mm/internal.h"	/* munlock_vma_page */

We have vma_adress() defined in internal.h, under #ifdef CONFIG_TRANSPARENT_HUGEPAGE .

NAK. :-)

Anton.

Quoted 30 lines

kernel/events/uprobes.c +5 -2

 #include <linux/uprobes.h>

@@ -141,7 +142,7 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
 	pte_t *ptep;
 	int err;
 
-	/* freeze PageSwapCache() for try_to_free_swap() below */
+	/* For try_to_free_swap() and munlock_vma_page() below */
 	lock_page(page);
 
 	err = -EAGAIN;

@@ -164,9 +165,12 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
 	page_remove_rmap(page);
 	if (!page_mapped(page))
 		try_to_free_swap(page);
-	put_page(page);
 	pte_unmap_unlock(ptep, ptl);
 
+	if (vma->vm_flags & VM_LOCKED)
+		munlock_vma_page(page);
+	put_page(page);
+
 	err = 0;
  unlock:
 	unlock_page(page);

Signature · 1 line

1.5.5.1

Oleg Nesterov

author maintainer

2012-06-26 15:49 UTC #12 lore raw

Re: [PATCH v2 5/5] uprobes: __replace_page() needs munlock_vma_page()

Quoted 16 lines · Anton Arapov

kernel/events/uprobes.c line 35
Quoted 11 lines · Oleg Nesterov
Like do_wp_page(), __replace_page() should do munlock_vma_page() for the case when the old page still has other !VM_LOCKED mappings. Unfortunately this needs mm/internal.h.
kernel/events/uprobes.c +1 -0
@@ -32,6 +32,7 @@
 #include <linux/swap.h>		/* try_to_free_swap */
 #include <linux/ptrace.h>	/* user_enable_single_step */
 #include <linux/kdebug.h>	/* notifier mechanism */
+#include "../../mm/internal.h"	/* munlock_vma_page */
We have vma_adress() defined in internal.h, under #ifdef CONFIG_TRANSPARENT_HUGEPAGE .

Quoted 1 line

NAK. :-)

Damn you Anton ;) You seem to dislike number 5...

OK, please ignore this patch.

Can't resist. I swear, I specially checked mm/internal.h to ensure it doesn't export mm/rmap.c:vma_address(), can't understand how I didn't notice this declaration.

Oleg.

Ingo Molnar

2012-07-06 10:54 UTC #13 lore raw

In reply to Oleg Nesterov

Quoted 8 lines · Oleg Nesterov

Hello,
write_opcode() cleanups resend + new minor fix.
Changes:
- document the new argument in 2/5.

Quoted 7 lines

- drop the buggy 5/5, thanks Anton for your quick nack. Probably I'll return to this later, I have another reason for this change.
- so this 5/5 is new.
Srikar, please add your acks unless you have some objections.

Just wondering what's the review status of patches #1-#4?

I'll skip #5 based on Oleg's request, but the others are looking good.

Thanks,

Ingo

Oleg Nesterov

author maintainer

2012-07-06 16:10 UTC #14 lore raw

Quoted 15 lines · Ingo Molnar

Quoted 12 lines · Oleg Nesterov
Hello,
write_opcode() cleanups resend + new minor fix.
Changes:
- document the new argument in 2/5.
- drop the buggy 5/5, thanks Anton for your quick nack. Probably I'll return to this later, I have another reason for this change.

Quoted 5 lines

Quoted 3 lines
- so this 5/5 is new.
Srikar, please add your acks unless you have some objections.
Just wondering what's the review status of patches #1-#4?

I hope Srikar will ack 1-4 soon.

He observed the testing failures, but it turns out this series is innocent.

I'll send more fixes soon.

Quoted 1 line

I'll skip #5 based on Oleg's request,

Yes, thanks. I still think 5/5 makes sense, but we need to do something with uprobe.s:vma_address() first.

Oleg.

Oleg Nesterov

author maintainer

2012-07-06 16:16 UTC #15 lore raw

Quoted 28 lines · Oleg Nesterov

Quoted 15 lines · Ingo Molnar
Quoted 12 lines · Oleg Nesterov
Hello,
write_opcode() cleanups resend + new minor fix.
Changes:
- document the new argument in 2/5.
- drop the buggy 5/5, thanks Anton for your quick nack. Probably I'll return to this later, I have another reason for this change.
Quoted 5 lines
Quoted 3 lines
- so this 5/5 is new.
Srikar, please add your acks unless you have some objections.
Just wondering what's the review status of patches #1-#4?
I hope Srikar will ack 1-4 soon.
He observed the testing failures, but it turns out this series is innocent.

Quoted 6 lines

I'll send more fixes soon.
Quoted 1 line
I'll skip #5 based on Oleg's request,
Yes, thanks. I still think 5/5 makes sense, but we need to do something with uprobe.s:vma_address() first.

Argh.

I wrote this email because I wanted to say that I updated the changelog for 2/5 a little bit, but forgot to mention this. I'll send the updated patch in reply to 2/5 (once again, only the changelog was changed). But please let me know if you want me to resend 1-4.

Oleg.

Ingo Molnar

2012-07-09 09:12 UTC #16 lore raw

Quoted 36 lines · Oleg Nesterov

Quoted 28 lines · Oleg Nesterov
Quoted 15 lines · Ingo Molnar
Quoted 12 lines · Oleg Nesterov
Hello,
write_opcode() cleanups resend + new minor fix.
Changes:
- document the new argument in 2/5.
- drop the buggy 5/5, thanks Anton for your quick nack. Probably I'll return to this later, I have another reason for this change.
Quoted 5 lines
Quoted 3 lines
- so this 5/5 is new.
Srikar, please add your acks unless you have some objections.
Just wondering what's the review status of patches #1-#4?
I hope Srikar will ack 1-4 soon.
He observed the testing failures, but it turns out this series is innocent.
Quoted 6 lines
I'll send more fixes soon.
Quoted 1 line
I'll skip #5 based on Oleg's request,
Yes, thanks. I still think 5/5 makes sense, but we need to do something with uprobe.s:vma_address() first.

Quoted 7 lines

Argh.
I wrote this email because I wanted to say that I updated the changelog for 2/5 a little bit, but forgot to mention this. I'll send the updated patch in reply to 2/5 (once again, only the changelog was changed). But please let me know if you want me to resend 1-4.

Once Srikar's ack is in then it would be nice to update the patches with the ack and resend #1-#4, to make sure I have all the intended patches and nothing more or less than that.

Thanks,

Ingo

Oleg Nesterov

author maintainer

2012-07-09 13:33 UTC #17 lore raw

Quoted 4 lines · Ingo Molnar

Once Srikar's ack is in then it would be nice to update the patches with the ack and resend #1-#4, to make sure I have all the intended patches and nothing more or less than that.

OK, will do.

I'll wait until Srikar reviews the new series, and then I'll send everything acked.

Oleg.

Srikar Dronamraju

2012-07-09 10:28 UTC #18 lore raw

In reply to Oleg Nesterov

Quoted 18 lines · Oleg Nesterov

Quoted 17 lines · Ingo Molnar
Quoted 14 lines · Oleg Nesterov
Hello,
write_opcode() cleanups resend + new minor fix.
Changes:
- document the new argument in 2/5.
- drop the buggy 5/5, thanks Anton for your quick nack. Probably I'll return to this later, I have another reason for this change.
- so this 5/5 is new.

Quoted 9 lines

Quoted 3 lines
Quoted 1 line
Srikar, please add your acks unless you have some objections.
Just wondering what's the review status of patches #1-#4?
I hope Srikar will ack 1-4 soon.
He observed the testing failures, but it turns out this series is innocent.

The 1st patch in your series, seems to have solved the problem. I am not able to reproduce the problem for now .. Will keep running for some time.

I will send out the acks for each of the patches in these series now.

Signature · 2 lines

Thanks and Regards
Srikar