On Fri, Nov 9, 2018 at 10:27 AM Davidlohr Bueso <dave@xxxxxxxxxxxx> wrote: > > On Thu, 08 Nov 2018, chouryzhou(??????) wrote: > > >+#ifdef CONFIG_ANDROID_BINDER_IPC > >+ /* next fields are for binder */ > >+ struct mutex binder_procs_lock; > >+ struct hlist_head binder_procs; > >+ struct hlist_head binder_contexts; > >+#endif > > Now, I took a look at how the binder_procs list is used; and no, what > follows isn't really related to this patch, but a general observation. > > I think that a mutex is also an overkill and you might wanna replace it > with a spinlock/rwlock. Can anything block while holding the binder_procs_lock? > I don't see anything... you mainly need it for consulting the hlist calling > print_binder_proc[_stat]() - which will take the proc->inner_lock anyway, so > no blocking there. print_binder_proc() drops proc->inner_lock and calls binder_alloc_print_allocated() which acquires proc->alloc->mutex. Likewise, print_binder_stats() calls print_binder_proc_stats() which drops its locks to call binder_alloc_print_pages() which also acquires proc->alloc->mutex. So binder_procs_lock needs to be a mutex since it can block on proc->alloc->mutex. > Also, if this is perhaps because of long hold times, dunno, > the rb_first_cached primitive might reduce some of it, although I don't know > how big the rbtrees in binder can get and if it matters at all. > > Anyway, that said and along with addressing Todd's comments, the ipc/ bits look > good. Feel free to add my: > > Reviewed-by: Davidlohr Bueso <dave@xxxxxxxxxxxx> > > Thanks, > Davidlohr _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
