Re: [PATCH v3 1/1] binder: return pending info for frozen async txns

From: Carlos Llamas
Date: Wed Nov 23 2022 - 21:25:55 EST

Next message: kernel test robot: "Re: [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs"
Previous message: wangchuanlei: "Re: [PATCH] [openvswitch v4] openvswitch: Add support to count upcall packets"
In reply to: Li Li: "[PATCH v3 1/1] binder: return pending info for frozen async txns"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Nov 23, 2022 at 12:16:54PM -0800, Li Li wrote:
> From: Li Li <dualli@xxxxxxxxxx>
>
> An async transaction to a frozen process will still be successfully
> put in the queue. But this pending async transaction won't be processed
> until the target process is unfrozen at an unspecified time in the
> future. Pass this important information back to the user space caller
> by returning BR_TRANSACTION_PENDING_FROZEN.
>
> Signed-off-by: Li Li <dualli@xxxxxxxxxx>
> ---
> drivers/android/binder.c | 32 +++++++++++++++++++++++------
> drivers/android/binder_internal.h | 3 ++-
> include/uapi/linux/android/binder.h | 7 ++++++-
> 3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 880224ec6abb..acd53147d5d1 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2728,7 +2728,10 @@ binder_find_outdated_transaction_ilocked(struct binder_transaction *t,
> *
> * Return: 0 if the transaction was successfully queued
> * BR_DEAD_REPLY if the target process or thread is dead
> - * BR_FROZEN_REPLY if the target process or thread is frozen
> + * BR_FROZEN_REPLY if the target process or thread is frozen and
> + * the sync transaction was rejected
> + * BR_TRANSACTION_PENDING_FROZEN if the target process is frozen
> + * and the async transaction was successfully queued
> */
> static int binder_proc_transaction(struct binder_transaction *t,
> struct binder_proc *proc,
> @@ -2738,6 +2741,7 @@ static int binder_proc_transaction(struct binder_transaction *t,
> bool oneway = !!(t->flags & TF_ONE_WAY);
> bool pending_async = false;
> struct binder_transaction *t_outdated = NULL;
> + bool frozen = false;
>
> BUG_ON(!node);
> binder_node_lock(node);
> @@ -2751,15 +2755,16 @@ static int binder_proc_transaction(struct binder_transaction *t,
>
> binder_inner_proc_lock(proc);
> if (proc->is_frozen) {
> + frozen = true;
> proc->sync_recv |= !oneway;
> proc->async_recv |= oneway;
> }
>
> - if ((proc->is_frozen && !oneway) || proc->is_dead ||
> + if ((frozen && !oneway) || proc->is_dead ||
> (thread && thread->is_dead)) {
> binder_inner_proc_unlock(proc);
> binder_node_unlock(node);
> - return proc->is_frozen ? BR_FROZEN_REPLY : BR_DEAD_REPLY;
> + return frozen ? BR_FROZEN_REPLY : BR_DEAD_REPLY;
> }
>
> if (!thread && !pending_async)
> @@ -2770,7 +2775,7 @@ static int binder_proc_transaction(struct binder_transaction *t,
> } else if (!pending_async) {
> binder_enqueue_work_ilocked(&t->work, &proc->todo);
> } else {
> - if ((t->flags & TF_UPDATE_TXN) && proc->is_frozen) {
> + if ((t->flags & TF_UPDATE_TXN) && frozen) {
> t_outdated = binder_find_outdated_transaction_ilocked(t,
> &node->async_todo);
> if (t_outdated) {
> @@ -2807,6 +2812,9 @@ static int binder_proc_transaction(struct binder_transaction *t,
> binder_stats_deleted(BINDER_STAT_TRANSACTION);
> }
>
> + if (oneway && frozen)
> + return BR_TRANSACTION_PENDING_FROZEN;
> +
> return 0;
> }
>
> @@ -3607,9 +3615,17 @@ static void binder_transaction(struct binder_proc *proc,
> } else {
> BUG_ON(target_node == NULL);
> BUG_ON(t->buffer->async_transaction != 1);
> - binder_enqueue_thread_work(thread, tcomplete);
> return_error = binder_proc_transaction(t, target_proc, NULL);
> - if (return_error)
> + /*
> + * Let the caller know when async transaction reaches a frozen
> + * process and is put in a pending queue, waiting for the target
> + * process to be unfrozen.
> + */
> + if (return_error == BR_TRANSACTION_PENDING_FROZEN)
> + tcomplete->type = BINDER_WORK_TRANSACTION_PENDING;
> + binder_enqueue_thread_work(thread, tcomplete);
> + if (return_error &&
> + return_error != BR_TRANSACTION_PENDING_FROZEN)
> goto err_dead_proc_or_thread;
> }
> if (target_thread)
> @@ -4440,10 +4456,13 @@ static int binder_thread_read(struct binder_proc *proc,
> binder_stat_br(proc, thread, cmd);
> } break;
> case BINDER_WORK_TRANSACTION_COMPLETE:
> + case BINDER_WORK_TRANSACTION_PENDING:
> case BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT: {
> if (proc->oneway_spam_detection_enabled &&
> w->type == BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT)
> cmd = BR_ONEWAY_SPAM_SUSPECT;
> + else if (w->type == BINDER_WORK_TRANSACTION_PENDING)
> + cmd = BR_TRANSACTION_PENDING_FROZEN;
> else
> cmd = BR_TRANSACTION_COMPLETE;
> binder_inner_proc_unlock(proc);
> @@ -6170,6 +6189,7 @@ static const char * const binder_return_strings[] = {
> "BR_FAILED_REPLY",
> "BR_FROZEN_REPLY",
> "BR_ONEWAY_SPAM_SUSPECT",
> + "BR_TRANSACTION_PENDING_FROZEN"
> };
>
> static const char * const binder_command_strings[] = {
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index abe19d88c6ec..28ef5b3704b1 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -133,7 +133,7 @@ enum binder_stat_types {
> };
>
> struct binder_stats {
> - atomic_t br[_IOC_NR(BR_ONEWAY_SPAM_SUSPECT) + 1];
> + atomic_t br[_IOC_NR(BR_TRANSACTION_PENDING_FROZEN) + 1];
> atomic_t bc[_IOC_NR(BC_REPLY_SG) + 1];
> atomic_t obj_created[BINDER_STAT_COUNT];
> atomic_t obj_deleted[BINDER_STAT_COUNT];
> @@ -152,6 +152,7 @@ struct binder_work {
> enum binder_work_type {
> BINDER_WORK_TRANSACTION = 1,
> BINDER_WORK_TRANSACTION_COMPLETE,
> + BINDER_WORK_TRANSACTION_PENDING,
> BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT,
> BINDER_WORK_RETURN_ERROR,
> BINDER_WORK_NODE,
> diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
> index e72e4de8f452..5f636b5afcd7 100644
> --- a/include/uapi/linux/android/binder.h
> +++ b/include/uapi/linux/android/binder.h
> @@ -450,7 +450,7 @@ enum binder_driver_return_protocol {
>
> BR_FROZEN_REPLY = _IO('r', 18),
> /*
> - * The target of the last transaction (either a bcTRANSACTION or
> + * The target of the last sync transaction (either a bcTRANSACTION or
> * a bcATTEMPT_ACQUIRE) is frozen. No parameters.
> */
>
> @@ -460,6 +460,11 @@ enum binder_driver_return_protocol {
> * asynchronous transaction makes the allocated async buffer size exceed
> * detection threshold. No parameters.
> */
> +
> + BR_TRANSACTION_PENDING_FROZEN = _IO('r', 20),
> + /*
> + * The target of the last async transaction is frozen. No parameters.
> + */
> };
>
> enum binder_driver_command_protocol {
> --
> 2.38.1.584.g0f3c55d4c2-goog
>

Thanks!

Acked-by: Carlos Llamas <cmllamas@xxxxxxxxxx>

Next message: kernel test robot: "Re: [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs"
Previous message: wangchuanlei: "Re: [PATCH] [openvswitch v4] openvswitch: Add support to count upcall packets"
In reply to: Li Li: "[PATCH v3 1/1] binder: return pending info for frozen async txns"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]