* [PATCH 1/4] uprobes: Introduce uprobe->register_rwsem
2012-11-24 18:02 [PATCH 0/4] uprobes: locking changes for filtering Oleg Nesterov
@ 2012-11-24 18:02 ` Oleg Nesterov
2012-12-13 14:09 ` Srikar Dronamraju
2012-11-24 18:02 ` [PATCH 2/4] uprobes: Change filter_chain() to iterate ->consumers list Oleg Nesterov
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2012-11-24 18:02 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Srikar Dronamraju
Cc: Ananth N Mavinakayanahalli, Anton Arapov, linux-kernel
Introduce uprobe->register_rwsem. It is taken for writing around
__uprobe_register/unregister.
Change handler_chain() to use this sem rather than consumer_rwsem.
The main reason for this change is that we have the nasty problem
with mmap_sem/consumer_rwsem dependency. filter_chain() needs to
protect uprobe->consumers like handler_chain(), but they can not
use the same lock. filter_chain() can be called under ->mmap_sem
(currently this is always true), but we want to allow ->handler()
to play with the probed task's memory, and this needs ->mmap_sem.
Alternatively we could use srcu, but synchronize_srcu() is very
slow and ->register_rwsem allows us to do more. In particular, we
can teach handler_chain() to do remove_breakpoint() if this bp is
"nacked" by all consumers, we know that we can't race with the
new consumer which does uprobe_register().
See also the next patches. uprobes_mutex[] is almost ready to die.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/events/uprobes.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index c80507d..03ffbb5 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -91,6 +91,7 @@ static atomic_t uprobe_events = ATOMIC_INIT(0);
struct uprobe {
struct rb_node rb_node; /* node in the rb tree */
atomic_t ref;
+ struct rw_semaphore register_rwsem;
struct rw_semaphore consumer_rwsem;
struct mutex copy_mutex; /* TODO: kill me and UPROBE_COPY_INSN */
struct list_head pending_list;
@@ -449,6 +450,7 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
uprobe->inode = igrab(inode);
uprobe->offset = offset;
+ init_rwsem(&uprobe->register_rwsem);
init_rwsem(&uprobe->consumer_rwsem);
mutex_init(&uprobe->copy_mutex);
/* For now assume that the instruction need not be single-stepped */
@@ -476,10 +478,10 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
if (!test_bit(UPROBE_RUN_HANDLER, &uprobe->flags))
return;
- down_read(&uprobe->consumer_rwsem);
+ down_read(&uprobe->register_rwsem);
for (uc = uprobe->consumers; uc; uc = uc->next)
uc->handler(uc, regs);
- up_read(&uprobe->consumer_rwsem);
+ up_read(&uprobe->register_rwsem);
}
static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
@@ -873,9 +875,11 @@ int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *
mutex_lock(uprobes_hash(inode));
uprobe = alloc_uprobe(inode, offset);
if (uprobe) {
+ down_write(&uprobe->register_rwsem);
ret = __uprobe_register(uprobe, uc);
if (ret)
__uprobe_unregister(uprobe, uc);
+ up_write(&uprobe->register_rwsem);
}
mutex_unlock(uprobes_hash(inode));
if (uprobe)
@@ -899,7 +903,9 @@ void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consume
return;
mutex_lock(uprobes_hash(inode));
+ down_write(&uprobe->register_rwsem);
__uprobe_unregister(uprobe, uc);
+ up_write(&uprobe->register_rwsem);
mutex_unlock(uprobes_hash(inode));
put_uprobe(uprobe);
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/4] uprobes: Introduce uprobe->register_rwsem
2012-11-24 18:02 ` [PATCH 1/4] uprobes: Introduce uprobe->register_rwsem Oleg Nesterov
@ 2012-12-13 14:09 ` Srikar Dronamraju
0 siblings, 0 replies; 9+ messages in thread
From: Srikar Dronamraju @ 2012-12-13 14:09 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Peter Zijlstra, Ananth N Mavinakayanahalli,
Anton Arapov, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2012-11-24 19:02:28]:
> Introduce uprobe->register_rwsem. It is taken for writing around
> __uprobe_register/unregister.
>
> Change handler_chain() to use this sem rather than consumer_rwsem.
>
> The main reason for this change is that we have the nasty problem
> with mmap_sem/consumer_rwsem dependency. filter_chain() needs to
> protect uprobe->consumers like handler_chain(), but they can not
> use the same lock. filter_chain() can be called under ->mmap_sem
> (currently this is always true), but we want to allow ->handler()
> to play with the probed task's memory, and this needs ->mmap_sem.
>
> Alternatively we could use srcu, but synchronize_srcu() is very
> slow and ->register_rwsem allows us to do more. In particular, we
> can teach handler_chain() to do remove_breakpoint() if this bp is
> "nacked" by all consumers, we know that we can't race with the
> new consumer which does uprobe_register().
>
> See also the next patches. uprobes_mutex[] is almost ready to die.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/events/uprobes.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index c80507d..03ffbb5 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -91,6 +91,7 @@ static atomic_t uprobe_events = ATOMIC_INIT(0);
> struct uprobe {
> struct rb_node rb_node; /* node in the rb tree */
> atomic_t ref;
> + struct rw_semaphore register_rwsem;
> struct rw_semaphore consumer_rwsem;
> struct mutex copy_mutex; /* TODO: kill me and UPROBE_COPY_INSN */
> struct list_head pending_list;
> @@ -449,6 +450,7 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
>
> uprobe->inode = igrab(inode);
> uprobe->offset = offset;
> + init_rwsem(&uprobe->register_rwsem);
> init_rwsem(&uprobe->consumer_rwsem);
> mutex_init(&uprobe->copy_mutex);
> /* For now assume that the instruction need not be single-stepped */
> @@ -476,10 +478,10 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
> if (!test_bit(UPROBE_RUN_HANDLER, &uprobe->flags))
> return;
>
> - down_read(&uprobe->consumer_rwsem);
> + down_read(&uprobe->register_rwsem);
> for (uc = uprobe->consumers; uc; uc = uc->next)
> uc->handler(uc, regs);
> - up_read(&uprobe->consumer_rwsem);
> + up_read(&uprobe->register_rwsem);
> }
>
> static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
> @@ -873,9 +875,11 @@ int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *
> mutex_lock(uprobes_hash(inode));
> uprobe = alloc_uprobe(inode, offset);
> if (uprobe) {
> + down_write(&uprobe->register_rwsem);
> ret = __uprobe_register(uprobe, uc);
> if (ret)
> __uprobe_unregister(uprobe, uc);
> + up_write(&uprobe->register_rwsem);
> }
> mutex_unlock(uprobes_hash(inode));
> if (uprobe)
> @@ -899,7 +903,9 @@ void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consume
> return;
>
> mutex_lock(uprobes_hash(inode));
> + down_write(&uprobe->register_rwsem);
> __uprobe_unregister(uprobe, uc);
> + up_write(&uprobe->register_rwsem);
> mutex_unlock(uprobes_hash(inode));
> put_uprobe(uprobe);
> }
> --
> 1.5.5.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] uprobes: Change filter_chain() to iterate ->consumers list
2012-11-24 18:02 [PATCH 0/4] uprobes: locking changes for filtering Oleg Nesterov
2012-11-24 18:02 ` [PATCH 1/4] uprobes: Introduce uprobe->register_rwsem Oleg Nesterov
@ 2012-11-24 18:02 ` Oleg Nesterov
2012-12-13 14:12 ` Srikar Dronamraju
2012-11-24 18:02 ` [PATCH 3/4] uprobes: Kill UPROBE_RUN_HANDLER flag Oleg Nesterov
2012-11-24 18:02 ` [PATCH 4/4] uprobes: Kill uprobe->copy_mutex Oleg Nesterov
3 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2012-11-24 18:02 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Srikar Dronamraju
Cc: Ananth N Mavinakayanahalli, Anton Arapov, linux-kernel
Now that it safe to use ->consumer_rwsem under ->mmap_sem we can
almost finish the implementation of filter_chain(). It still lacks
the actual uc->filter(...) call but othewrwise it is ready, just
it pretends that ->filter() always returns true.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/events/uprobes.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 03ffbb5..873c993 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -614,14 +614,19 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file,
static bool filter_chain(struct uprobe *uprobe)
{
- /*
- * TODO:
- * for_each_consumer(uc)
- * if (uc->filter(...))
- * return true;
- * return false;
- */
- return uprobe->consumers != NULL;
+ struct uprobe_consumer *uc;
+ bool ret = false;
+
+ down_read(&uprobe->consumer_rwsem);
+ for (uc = uprobe->consumers; uc; uc = uc->next) {
+ /* TODO: ret = uc->filter(...) */
+ ret = true;
+ if (ret)
+ break;
+ }
+ up_read(&uprobe->consumer_rwsem);
+
+ return ret;
}
static int
--
1.5.5.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/4] uprobes: Change filter_chain() to iterate ->consumers list
2012-11-24 18:02 ` [PATCH 2/4] uprobes: Change filter_chain() to iterate ->consumers list Oleg Nesterov
@ 2012-12-13 14:12 ` Srikar Dronamraju
0 siblings, 0 replies; 9+ messages in thread
From: Srikar Dronamraju @ 2012-12-13 14:12 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Peter Zijlstra, Ananth N Mavinakayanahalli,
Anton Arapov, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2012-11-24 19:02:31]:
> Now that it safe to use ->consumer_rwsem under ->mmap_sem we can
> almost finish the implementation of filter_chain(). It still lacks
> the actual uc->filter(...) call but othewrwise it is ready, just
> it pretends that ->filter() always returns true.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/events/uprobes.c | 21 +++++++++++++--------
> 1 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 03ffbb5..873c993 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -614,14 +614,19 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file,
>
> static bool filter_chain(struct uprobe *uprobe)
> {
> - /*
> - * TODO:
> - * for_each_consumer(uc)
> - * if (uc->filter(...))
> - * return true;
> - * return false;
> - */
> - return uprobe->consumers != NULL;
> + struct uprobe_consumer *uc;
> + bool ret = false;
> +
> + down_read(&uprobe->consumer_rwsem);
> + for (uc = uprobe->consumers; uc; uc = uc->next) {
> + /* TODO: ret = uc->filter(...) */
> + ret = true;
> + if (ret)
> + break;
> + }
> + up_read(&uprobe->consumer_rwsem);
> +
> + return ret;
> }
>
> static int
> --
> 1.5.5.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] uprobes: Kill UPROBE_RUN_HANDLER flag
2012-11-24 18:02 [PATCH 0/4] uprobes: locking changes for filtering Oleg Nesterov
2012-11-24 18:02 ` [PATCH 1/4] uprobes: Introduce uprobe->register_rwsem Oleg Nesterov
2012-11-24 18:02 ` [PATCH 2/4] uprobes: Change filter_chain() to iterate ->consumers list Oleg Nesterov
@ 2012-11-24 18:02 ` Oleg Nesterov
2012-12-13 14:29 ` Srikar Dronamraju
2012-11-24 18:02 ` [PATCH 4/4] uprobes: Kill uprobe->copy_mutex Oleg Nesterov
3 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2012-11-24 18:02 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Srikar Dronamraju
Cc: Ananth N Mavinakayanahalli, Anton Arapov, linux-kernel
Simply remove UPROBE_RUN_HANDLER and the corresponding code.
It can only help if uprobe has a single consumer, and in fact
it is no longer needed after handler_chain() was changed to use
->register_rwsem, we simply can not race with uprobe_register().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/events/uprobes.c | 23 +++++------------------
1 files changed, 5 insertions(+), 18 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 873c993..97c3874 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -83,10 +83,8 @@ static atomic_t uprobe_events = ATOMIC_INIT(0);
/* Have a copy of original instruction */
#define UPROBE_COPY_INSN 0
-/* Dont run handlers when first register/ last unregister in progress*/
-#define UPROBE_RUN_HANDLER 1
/* Can skip singlestep */
-#define UPROBE_SKIP_SSTEP 2
+#define UPROBE_SKIP_SSTEP 1
struct uprobe {
struct rb_node rb_node; /* node in the rb tree */
@@ -475,9 +473,6 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
{
struct uprobe_consumer *uc;
- if (!test_bit(UPROBE_RUN_HANDLER, &uprobe->flags))
- return;
-
down_read(&uprobe->register_rwsem);
for (uc = uprobe->consumers; uc; uc = uc->next)
uc->handler(uc, regs);
@@ -825,13 +820,8 @@ static int register_for_each_vma(struct uprobe *uprobe, bool is_register)
static int __uprobe_register(struct uprobe *uprobe, struct uprobe_consumer *uc)
{
- int err;
-
consumer_add(uprobe, uc);
- err = register_for_each_vma(uprobe, true);
- if (!err) /* TODO: pointless unless the first consumer */
- set_bit(UPROBE_RUN_HANDLER, &uprobe->flags);
- return err;
+ return register_for_each_vma(uprobe, true);
}
static void __uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *uc)
@@ -842,12 +832,9 @@ static void __uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *u
return;
err = register_for_each_vma(uprobe, false);
- if (!uprobe->consumers) {
- clear_bit(UPROBE_RUN_HANDLER, &uprobe->flags);
- /* TODO : cant unregister? schedule a worker thread */
- if (!err)
- delete_uprobe(uprobe);
- }
+ /* TODO : cant unregister? schedule a worker thread */
+ if (!uprobe->consumers && !err)
+ delete_uprobe(uprobe);
}
/*
--
1.5.5.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] uprobes: Kill UPROBE_RUN_HANDLER flag
2012-11-24 18:02 ` [PATCH 3/4] uprobes: Kill UPROBE_RUN_HANDLER flag Oleg Nesterov
@ 2012-12-13 14:29 ` Srikar Dronamraju
0 siblings, 0 replies; 9+ messages in thread
From: Srikar Dronamraju @ 2012-12-13 14:29 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Peter Zijlstra, Ananth N Mavinakayanahalli,
Anton Arapov, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2012-11-24 19:02:33]:
> Simply remove UPROBE_RUN_HANDLER and the corresponding code.
>
> It can only help if uprobe has a single consumer, and in fact
> it is no longer needed after handler_chain() was changed to use
> ->register_rwsem, we simply can not race with uprobe_register().
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/events/uprobes.c | 23 +++++------------------
> 1 files changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 873c993..97c3874 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -83,10 +83,8 @@ static atomic_t uprobe_events = ATOMIC_INIT(0);
>
> /* Have a copy of original instruction */
> #define UPROBE_COPY_INSN 0
> -/* Dont run handlers when first register/ last unregister in progress*/
> -#define UPROBE_RUN_HANDLER 1
> /* Can skip singlestep */
> -#define UPROBE_SKIP_SSTEP 2
> +#define UPROBE_SKIP_SSTEP 1
>
> struct uprobe {
> struct rb_node rb_node; /* node in the rb tree */
> @@ -475,9 +473,6 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
> {
> struct uprobe_consumer *uc;
>
> - if (!test_bit(UPROBE_RUN_HANDLER, &uprobe->flags))
> - return;
> -
> down_read(&uprobe->register_rwsem);
> for (uc = uprobe->consumers; uc; uc = uc->next)
> uc->handler(uc, regs);
> @@ -825,13 +820,8 @@ static int register_for_each_vma(struct uprobe *uprobe, bool is_register)
>
> static int __uprobe_register(struct uprobe *uprobe, struct uprobe_consumer *uc)
> {
> - int err;
> -
> consumer_add(uprobe, uc);
> - err = register_for_each_vma(uprobe, true);
> - if (!err) /* TODO: pointless unless the first consumer */
> - set_bit(UPROBE_RUN_HANDLER, &uprobe->flags);
> - return err;
> + return register_for_each_vma(uprobe, true);
> }
>
> static void __uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *uc)
> @@ -842,12 +832,9 @@ static void __uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *u
> return;
>
> err = register_for_each_vma(uprobe, false);
> - if (!uprobe->consumers) {
> - clear_bit(UPROBE_RUN_HANDLER, &uprobe->flags);
> - /* TODO : cant unregister? schedule a worker thread */
> - if (!err)
> - delete_uprobe(uprobe);
> - }
> + /* TODO : cant unregister? schedule a worker thread */
> + if (!uprobe->consumers && !err)
> + delete_uprobe(uprobe);
> }
>
> /*
> --
> 1.5.5.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] uprobes: Kill uprobe->copy_mutex
2012-11-24 18:02 [PATCH 0/4] uprobes: locking changes for filtering Oleg Nesterov
` (2 preceding siblings ...)
2012-11-24 18:02 ` [PATCH 3/4] uprobes: Kill UPROBE_RUN_HANDLER flag Oleg Nesterov
@ 2012-11-24 18:02 ` Oleg Nesterov
2012-12-13 14:30 ` Srikar Dronamraju
3 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2012-11-24 18:02 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Srikar Dronamraju
Cc: Ananth N Mavinakayanahalli, Anton Arapov, linux-kernel
Now that ->register_rwsem is safe under ->mmap_sem we can kill
->copy_mutex and abuse down_write(&uprobe->consumer_rwsem).
This makes prepare_uprobe() even more ugly, but we should kill
it anyway.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/events/uprobes.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 97c3874..1e047f8 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -91,7 +91,6 @@ struct uprobe {
atomic_t ref;
struct rw_semaphore register_rwsem;
struct rw_semaphore consumer_rwsem;
- struct mutex copy_mutex; /* TODO: kill me and UPROBE_COPY_INSN */
struct list_head pending_list;
struct uprobe_consumer *consumers;
struct inode *inode; /* Also hold a ref to inode */
@@ -450,7 +449,6 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
uprobe->offset = offset;
init_rwsem(&uprobe->register_rwsem);
init_rwsem(&uprobe->consumer_rwsem);
- mutex_init(&uprobe->copy_mutex);
/* For now assume that the instruction need not be single-stepped */
__set_bit(UPROBE_SKIP_SSTEP, &uprobe->flags);
@@ -578,7 +576,8 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file,
if (test_bit(UPROBE_COPY_INSN, &uprobe->flags))
return ret;
- mutex_lock(&uprobe->copy_mutex);
+ /* TODO: move this into _register, until then we abuse this sem. */
+ down_write(&uprobe->consumer_rwsem);
if (test_bit(UPROBE_COPY_INSN, &uprobe->flags))
goto out;
@@ -602,7 +601,7 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file,
set_bit(UPROBE_COPY_INSN, &uprobe->flags);
out:
- mutex_unlock(&uprobe->copy_mutex);
+ up_write(&uprobe->consumer_rwsem);
return ret;
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] uprobes: Kill uprobe->copy_mutex
2012-11-24 18:02 ` [PATCH 4/4] uprobes: Kill uprobe->copy_mutex Oleg Nesterov
@ 2012-12-13 14:30 ` Srikar Dronamraju
0 siblings, 0 replies; 9+ messages in thread
From: Srikar Dronamraju @ 2012-12-13 14:30 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Peter Zijlstra, Ananth N Mavinakayanahalli,
Anton Arapov, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2012-11-24 19:02:36]:
> Now that ->register_rwsem is safe under ->mmap_sem we can kill
> ->copy_mutex and abuse down_write(&uprobe->consumer_rwsem).
>
> This makes prepare_uprobe() even more ugly, but we should kill
> it anyway.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/events/uprobes.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 97c3874..1e047f8 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -91,7 +91,6 @@ struct uprobe {
> atomic_t ref;
> struct rw_semaphore register_rwsem;
> struct rw_semaphore consumer_rwsem;
> - struct mutex copy_mutex; /* TODO: kill me and UPROBE_COPY_INSN */
> struct list_head pending_list;
> struct uprobe_consumer *consumers;
> struct inode *inode; /* Also hold a ref to inode */
> @@ -450,7 +449,6 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
> uprobe->offset = offset;
> init_rwsem(&uprobe->register_rwsem);
> init_rwsem(&uprobe->consumer_rwsem);
> - mutex_init(&uprobe->copy_mutex);
> /* For now assume that the instruction need not be single-stepped */
> __set_bit(UPROBE_SKIP_SSTEP, &uprobe->flags);
>
> @@ -578,7 +576,8 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file,
> if (test_bit(UPROBE_COPY_INSN, &uprobe->flags))
> return ret;
>
> - mutex_lock(&uprobe->copy_mutex);
> + /* TODO: move this into _register, until then we abuse this sem. */
> + down_write(&uprobe->consumer_rwsem);
> if (test_bit(UPROBE_COPY_INSN, &uprobe->flags))
> goto out;
>
> @@ -602,7 +601,7 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file,
> set_bit(UPROBE_COPY_INSN, &uprobe->flags);
>
> out:
> - mutex_unlock(&uprobe->copy_mutex);
> + up_write(&uprobe->consumer_rwsem);
>
> return ret;
> }
> --
> 1.5.5.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread