Re: [PATCH] binder_alloc: Fix sleeping function called from invalid context

[Carlos Llamas <cmllamas@google.com>]

On Wed, Jul 24, 2024 at 09:25:55PM +0530, Mukesh Ojha wrote:
> 36c55ce8703c ("binder_alloc: Replace kcalloc with kvcalloc to
nit: I believe checkpatch expects the format "Commit 36c55..."

> mitigate OOM issues") introduce schedule while atomic issue.
nit: past tense "introduced"

> 
> [ 2689.152635][ T4275] BUG: sleeping function called from invalid context at mm/vmalloc.c:2847
> [ 2689.161291][ T4275] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 4275, name: kworker/1:140
> [ 2689.170708][ T4275] preempt_count: 1, expected: 0
> [ 2689.175572][ T4275] RCU nest depth: 0, expected: 0
> [ 2689.180521][ T4275] INFO: lockdep is turned off.
> [ 2689.180523][ T4275] Preemption disabled at:
> [ 2689.180525][ T4275] [<ffffffe031f2a2dc>] binder_alloc_deferred_release+0x2c/0x388
> ..
> ..
> [ 2689.213419][ T4275]  __might_resched+0x174/0x178
> [ 2689.213423][ T4275]  __might_sleep+0x48/0x7c
> [ 2689.213426][ T4275]  vfree+0x4c/0x15c
> [ 2689.213430][ T4275]  kvfree+0x24/0x44
> [ 2689.213433][ T4275]  binder_alloc_deferred_release+0x2c0/0x388
> [ 2689.213436][ T4275]  binder_proc_dec_tmpref+0x15c/0x2a8
> [ 2689.213440][ T4275]  binder_deferred_func+0xa8/0x8ec
> [ 2689.213442][ T4275]  process_one_work+0x254/0x59c
> [ 2689.213447][ T4275]  worker_thread+0x274/0x3ec
> [ 2689.213450][ T4275]  kthread+0x110/0x134
> [ 2689.213453][ T4275]  ret_from_fork+0x10/0x20
> 
> Fix it by moving the place of kvfree outside of spinlock context.
> 
> Fixes: 36c55ce8703c ("binder_alloc: Replace kcalloc with kvcalloc to mitigate OOM issues")
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
>  drivers/android/binder_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index b00961944ab1..b3acbc4174fb 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -939,9 +939,9 @@ void binder_alloc_deferred_release(struct binder_alloc *alloc)
>  			__free_page(alloc->pages[i].page_ptr);
>  			page_count++;
>  		}
> -		kvfree(alloc->pages);
>  	}
>  	spin_unlock(&alloc->lock);
> +	kvfree(alloc->pages);
>  	if (alloc->mm)
>  		mmdrop(alloc->mm);
>  
> -- 
> 2.34.1
> 

Note scripts/get_maintainer.pl asks to Cc the blamed fixes:
  Lei Liu <liulei.rjpt@vivo.com> (blamed_fixes:1/1=100%)

If you care to fix the nits feel free to add for v2:
Acked-by: Carlos Llamas <cmllamas@google.com>

Thanks
--
Carlos Llamas