[Ce-android-mainline] Binder API, was: Re: Android mainlining interest group

[Arnd Bergmann]

(trimming the Cc list, apparently lists.linuxfoundation.org otherwise
rejects the mail. I hope the ones I removed are already subscribed)

On Thursday 22 December 2011, Brian Swetland wrote:
> > I think we also need to look at other projects that have found the
> > same deficiencies that the Android drivers address, and make sure
> > that the interfaces we come up with work for all of them.
> > E.g. in case of the binder, it would be great if we could come up
> > with an interface that works for both Android and the in-kernel
> > DBUS implementation that has been discussed before.
> 
> Provided that it didn't impact performance, stability, and power...

Absolutely. The goal of course is to make it better in the process
and let you move to the new version. 

> Binder is probably going to be the touchiest thing to modify in some
> ways because it is very central to a lot of the Android userspace
> infrastructure and is both complex and subtle.  This is not
> necessarily a good thing, but it is a reality we live with -- there
> is a LOT of software stacked on top of the functionality and semantics
> that Binder provides and chasing down all the possible breakage if we
> fundamentally change how it works could be quite the project.

Let's talk about the API then. IMHO the biggest problem with this
driver is just the concept of implementing it as a chardev when the
functionality is clearly tied into core kernel code.

The least intrusive way to fix change this would be to translate
the various calls directly into new system calls. My understanding
of the code is still limited, but I can see this become something
like:

- binder_open(...)
- BINDER_SET_MAX_THREADS
- BINDER_SET_CONTEXT_MGR
- BINDER_SET_IDLE_TIMEOUT
+ #define BINDER_FLAG_CONTEX_MGR 1
+ sys_binder_create(unsigned flags, unsigned long max_threads,
			int64_t idle_timeout); /* returns fd */

- BINDER_WRITE_READ
+ sys_binder_write_read(int fd, void __user *write_buffer, size_t *wlength,
+				void __user *read_buffer,  size_t *rlength);

- BINDER_THREAD_EXIT
+ sys_binder_thread_exit(int fd); /* is this really needed? */

- BINDER_VERSION /* no replacement, upstream interfaces may never break */

Doing this would remove one extra level of indirection that you have in the
driver today and should be fairly straightforward to do. Unfortunately, the
binder_write_read() call is a second indirection level that is much harder
to resolve. I assume that changing the format of the write_read buffer is
not an option, but we could in theory turn each BC_*/BR_* pair into an
individual syscall and walk/fill the buffers in user space.

Overhead in terms of cycles should be minimal or negative, since you save
the entire ioctl path and another multiplexer, but I don't know what the
impact on the API would be, in particular on concurrent execution of the
individual commands.

	Arnd