From d31f0c40a7488d06c0b31a68ce250766c78645ab Mon Sep 17 00:00:00 2001 From: Danny Wood Date: Tue, 26 Feb 2019 08:15:45 +0000 Subject: [PATCH] Revert "android: binder: Disable preemption while holding the global binder lock." This reverts commit 6fd130fe5ba15d200251d27aabab0bf4c94d8f13. --- drivers/android/binder.c | 151 ++++++++++----------------------------- 1 file changed, 38 insertions(+), 113 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 41e6ffac7c7..cfb76f8c1df 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -473,7 +473,6 @@ static inline void binder_lock(struct binder_context *context, const char *tag) { trace_binder_lock(tag); mutex_lock(&context->binder_main_lock); - preempt_disable(); trace_binder_locked(tag); } @@ -482,62 +481,8 @@ static inline void binder_unlock(struct binder_context *context, { trace_binder_unlock(tag); mutex_unlock(&context->binder_main_lock); - preempt_enable(); -} - -static inline void *kzalloc_preempt_disabled(size_t size) -{ - void *ptr; - - ptr = kzalloc(size, GFP_NOWAIT); - if (ptr) - return ptr; - - preempt_enable_no_resched(); - ptr = kzalloc(size, GFP_KERNEL); - preempt_disable(); - - return ptr; -} - -static inline long copy_to_user_preempt_disabled(void __user *to, const void *from, long n) -{ - long ret; - - preempt_enable_no_resched(); - ret = copy_to_user(to, from, n); - preempt_disable(); - return ret; } -static inline long copy_from_user_preempt_disabled(void *to, const void __user *from, long n) -{ - long ret; - - preempt_enable_no_resched(); - ret = copy_from_user(to, from, n); - preempt_disable(); - return ret; -} - -#define get_user_preempt_disabled(x, ptr) \ -({ \ - int __ret; \ - preempt_enable_no_resched(); \ - __ret = get_user(x, ptr); \ - preempt_disable(); \ - __ret; \ -}) - -#define put_user_preempt_disabled(x, ptr) \ -({ \ - int __ret; \ - preempt_enable_no_resched(); \ - __ret = put_user(x, ptr); \ - preempt_disable(); \ - __ret; \ -}) - static void binder_set_nice(long nice) { long min_nice; @@ -671,8 +616,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate, else mm = get_task_mm(proc->tsk); - preempt_enable_no_resched(); - if (mm) { down_write(&mm->mmap_sem); vma = proc->vma; @@ -726,9 +669,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate, up_write(&mm->mmap_sem); mmput(mm); } - - preempt_disable(); - return 0; free_range: @@ -751,9 +691,6 @@ err_no_vma: up_write(&mm->mmap_sem); mmput(mm); } - - preempt_disable(); - return -ENOMEM; } @@ -1022,7 +959,7 @@ static struct binder_node *binder_new_node(struct binder_proc *proc, return NULL; } - node = kzalloc_preempt_disabled(sizeof(*node)); + node = kzalloc(sizeof(*node), GFP_KERNEL); if (node == NULL) return NULL; binder_stats_created(BINDER_STAT_NODE); @@ -1166,7 +1103,7 @@ static struct binder_ref *binder_get_ref_for_node(struct binder_proc *proc, else return ref; } - new_ref = kzalloc_preempt_disabled(sizeof(*ref)); + new_ref = kzalloc(sizeof(*ref), GFP_KERNEL); if (new_ref == NULL) return NULL; binder_stats_created(BINDER_STAT_REF); @@ -2037,14 +1974,14 @@ static void binder_transaction(struct binder_proc *proc, e->to_proc = target_proc->pid; /* TODO: reuse incoming transaction for reply */ - t = kzalloc_preempt_disabled(sizeof(*t)); + t = kzalloc(sizeof(*t), GFP_KERNEL); if (t == NULL) { return_error = BR_FAILED_REPLY; goto err_alloc_t_failed; } binder_stats_created(BINDER_STAT_TRANSACTION); - tcomplete = kzalloc_preempt_disabled(sizeof(*tcomplete)); + tcomplete = kzalloc(sizeof(*tcomplete), GFP_KERNEL); if (tcomplete == NULL) { return_error = BR_FAILED_REPLY; goto err_alloc_tcomplete_failed; @@ -2105,14 +2042,14 @@ static void binder_transaction(struct binder_proc *proc, ALIGN(tr->data_size, sizeof(void *))); offp = off_start; - if (copy_from_user_preempt_disabled(t->buffer->data, (const void __user *)(uintptr_t) + if (copy_from_user(t->buffer->data, (const void __user *)(uintptr_t) tr->data.ptr.buffer, tr->data_size)) { binder_user_error("%d:%d got transaction with invalid data ptr\n", proc->pid, thread->pid); return_error = BR_FAILED_REPLY; goto err_copy_data_failed; } - if (copy_from_user_preempt_disabled(offp, (const void __user *)(uintptr_t) + if (copy_from_user(offp, (const void __user *)(uintptr_t) tr->data.ptr.offsets, tr->offsets_size)) { binder_user_error("%d:%d got transaction with invalid offsets ptr\n", proc->pid, thread->pid); @@ -2346,7 +2283,7 @@ int binder_thread_write(struct binder_proc *proc, void __user *end = buffer + size; while (ptr < end && thread->return_error == BR_OK) { - if (get_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) + if (get_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); trace_binder_command(cmd); @@ -2364,7 +2301,7 @@ int binder_thread_write(struct binder_proc *proc, struct binder_ref *ref; const char *debug_string; - if (get_user_preempt_disabled(target, (uint32_t __user *)ptr)) + if (get_user(target, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); if (target == 0 && context->binder_context_mgr_node && @@ -2416,10 +2353,10 @@ int binder_thread_write(struct binder_proc *proc, binder_uintptr_t cookie; struct binder_node *node; - if (get_user_preempt_disabled(node_ptr, (binder_uintptr_t __user *)ptr)) + if (get_user(node_ptr, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); - if (get_user_preempt_disabled(cookie, (binder_uintptr_t __user *)ptr)) + if (get_user(cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); node = binder_get_node(proc, node_ptr); @@ -2477,7 +2414,7 @@ int binder_thread_write(struct binder_proc *proc, binder_uintptr_t data_ptr; struct binder_buffer *buffer; - if (get_user_preempt_disabled(data_ptr, (binder_uintptr_t __user *)ptr)) + if (get_user(data_ptr, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); @@ -2531,7 +2468,7 @@ int binder_thread_write(struct binder_proc *proc, case BC_REPLY: { struct binder_transaction_data tr; - if (copy_from_user_preempt_disabled(&tr, ptr, sizeof(tr))) + if (copy_from_user(&tr, ptr, sizeof(tr))) return -EFAULT; ptr += sizeof(tr); binder_transaction(proc, thread, &tr, @@ -2582,10 +2519,10 @@ int binder_thread_write(struct binder_proc *proc, struct binder_ref *ref; struct binder_ref_death *death; - if (get_user_preempt_disabled(target, (uint32_t __user *)ptr)) + if (get_user(target, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (get_user_preempt_disabled(cookie, (binder_uintptr_t __user *)ptr)) + if (get_user(cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); ref = binder_get_ref(proc, target, false); @@ -2614,7 +2551,7 @@ int binder_thread_write(struct binder_proc *proc, proc->pid, thread->pid); break; } - death = kzalloc_preempt_disabled(sizeof(*death)); + death = kzalloc(sizeof(*death), GFP_KERNEL); if (death == NULL) { thread->return_error = BR_ERROR; binder_debug(BINDER_DEBUG_FAILED_TRANSACTION, @@ -2669,7 +2606,7 @@ int binder_thread_write(struct binder_proc *proc, binder_uintptr_t cookie; struct binder_ref_death *death = NULL; - if (get_user_preempt_disabled(cookie, (binder_uintptr_t __user *)ptr)) + if (get_user(cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(cookie); @@ -2750,7 +2687,7 @@ static int binder_thread_read(struct binder_proc *proc, int wait_for_proc_work; if (*consumed == 0) { - if (put_user_preempt_disabled(BR_NOOP, (uint32_t __user *)ptr)) + if (put_user(BR_NOOP, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); } @@ -2761,7 +2698,7 @@ retry: if (thread->return_error != BR_OK && ptr < end) { if (thread->return_error2 != BR_OK) { - if (put_user_preempt_disabled(thread->return_error2, (uint32_t __user *)ptr)) + if (put_user(thread->return_error2, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); binder_stat_br(proc, thread, thread->return_error2); @@ -2769,7 +2706,7 @@ retry: goto done; thread->return_error2 = BR_OK; } - if (put_user_preempt_disabled(thread->return_error, (uint32_t __user *)ptr)) + if (put_user(thread->return_error, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); binder_stat_br(proc, thread, thread->return_error); @@ -2847,7 +2784,7 @@ retry: } break; case BINDER_WORK_TRANSACTION_COMPLETE: { cmd = BR_TRANSACTION_COMPLETE; - if (put_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); @@ -2889,14 +2826,14 @@ retry: node->has_weak_ref = 0; } if (cmd != BR_NOOP) { - if (put_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (put_user_preempt_disabled(node->ptr, + if (put_user(node->ptr, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); - if (put_user_preempt_disabled(node->cookie, + if (put_user(node->cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); @@ -2940,10 +2877,10 @@ retry: cmd = BR_CLEAR_DEATH_NOTIFICATION_DONE; else cmd = BR_DEAD_BINDER; - if (put_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (put_user_preempt_disabled(death->cookie, + if (put_user(death->cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); @@ -3011,10 +2948,10 @@ retry: ALIGN(t->buffer->data_size, sizeof(void *)); - if (put_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (copy_to_user_preempt_disabled(ptr, &tr, sizeof(tr))) + if (copy_to_user(ptr, &tr, sizeof(tr))) return -EFAULT; ptr += sizeof(tr); @@ -3056,7 +2993,7 @@ done: binder_debug(BINDER_DEBUG_THREADS, "%d:%d BR_SPAWN_LOOPER\n", proc->pid, thread->pid); - if (put_user_preempt_disabled(BR_SPAWN_LOOPER, (uint32_t __user *)buffer)) + if (put_user(BR_SPAWN_LOOPER, (uint32_t __user *)buffer)) return -EFAULT; binder_stat_br(proc, thread, BR_SPAWN_LOOPER); } @@ -3131,7 +3068,7 @@ static struct binder_thread *binder_get_thread(struct binder_proc *proc) break; } if (*p == NULL) { - thread = kzalloc_preempt_disabled(sizeof(*thread)); + thread = kzalloc(sizeof(*thread), GFP_KERNEL); if (thread == NULL) return NULL; binder_stats_created(BINDER_STAT_THREAD); @@ -3235,7 +3172,7 @@ static int binder_ioctl_write_read(struct file *filp, ret = -EINVAL; goto out; } - if (copy_from_user_preempt_disabled(&bwr, ubuf, sizeof(bwr))) { + if (copy_from_user(&bwr, ubuf, sizeof(bwr))) { ret = -EFAULT; goto out; } @@ -3253,7 +3190,7 @@ static int binder_ioctl_write_read(struct file *filp, trace_binder_write_done(ret); if (ret < 0) { bwr.read_consumed = 0; - if (copy_to_user_preempt_disabled(ubuf, &bwr, sizeof(bwr))) + if (copy_to_user(ubuf, &bwr, sizeof(bwr))) ret = -EFAULT; goto out; } @@ -3267,7 +3204,7 @@ static int binder_ioctl_write_read(struct file *filp, if (!list_empty(&proc->todo)) wake_up_interruptible(&proc->wait); if (ret < 0) { - if (copy_to_user_preempt_disabled(ubuf, &bwr, sizeof(bwr))) + if (copy_to_user(ubuf, &bwr, sizeof(bwr))) ret = -EFAULT; goto out; } @@ -3277,7 +3214,7 @@ static int binder_ioctl_write_read(struct file *filp, proc->pid, thread->pid, (u64)bwr.write_consumed, (u64)bwr.write_size, (u64)bwr.read_consumed, (u64)bwr.read_size); - if (copy_to_user_preempt_disabled(ubuf, &bwr, sizeof(bwr))) { + if (copy_to_user(ubuf, &bwr, sizeof(bwr))) { ret = -EFAULT; goto out; } @@ -3358,7 +3295,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) goto err; break; case BINDER_SET_MAX_THREADS: - if (copy_from_user_preempt_disabled(&proc->max_threads, ubuf, sizeof(proc->max_threads))) { + if (copy_from_user(&proc->max_threads, ubuf, sizeof(proc->max_threads))) { ret = -EINVAL; goto err; } @@ -3381,8 +3318,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) ret = -EINVAL; goto err; } - - if (put_user_preempt_disabled(BINDER_CURRENT_PROTOCOL_VERSION, + if (put_user(BINDER_CURRENT_PROTOCOL_VERSION, &ver->protocol_version)) { ret = -EINVAL; goto err; @@ -3445,7 +3381,6 @@ static struct vm_operations_struct binder_vm_ops = { static int binder_mmap(struct file *filp, struct vm_area_struct *vma) { int ret; - struct vm_struct *area; struct binder_proc *proc = filp->private_data; const char *failure_string; @@ -3506,11 +3441,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) vma->vm_ops = &binder_vm_ops; vma->vm_private_data = proc; - /* binder_update_page_range assumes preemption is disabled */ - preempt_disable(); - ret = binder_update_page_range(proc, 1, proc->buffer, proc->buffer + PAGE_SIZE, vma); - preempt_enable_no_resched(); - if (ret) { + if (binder_update_page_range(proc, 1, proc->buffer, proc->buffer + PAGE_SIZE, vma)) { ret = -ENOMEM; failure_string = "alloc small buf"; goto err_alloc_small_buf_failed; @@ -3799,12 +3730,8 @@ static void binder_deferred_func(struct work_struct *work) int defer; do { - trace_binder_lock(__func__); - mutex_lock(&context->binder_main_lock); - trace_binder_locked(__func__); - + binder_lock(__func__); mutex_lock(&context->binder_deferred_lock); - preempt_disable(); if (!hlist_empty(&context->binder_deferred_list)) { proc = hlist_entry(context->binder_deferred_list.first, struct binder_proc, @@ -3831,9 +3758,7 @@ static void binder_deferred_func(struct work_struct *work) if (defer & BINDER_DEFERRED_RELEASE) binder_deferred_release(proc); /* frees proc */ - trace_binder_unlock(__func__); - mutex_unlock(&context->binder_main_lock); - preempt_enable_no_resched(); + binder_unlock(__func__); if (files) put_files_struct(files); } while (proc); -- 2.20.1