uprobes: Introduce uprobe->register_rwsem
authorOleg Nesterov <oleg@redhat.com>
Sat, 24 Nov 2012 16:29:40 +0000 (17:29 +0100)
committerOleg Nesterov <oleg@redhat.com>
Fri, 8 Feb 2013 16:47:03 +0000 (17:47 +0100)
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

index d1d1394..61d0fa6 100644 (file)
@@ -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);
 }