Re: [PATCH] staging: android: binder: move to the "real" part of the kernel
From: Pavel Machek <hidden>
Date: 2014-10-21 20:05:29
Also in:
lkml
On Tue 2014-10-21 16:12:24, Arnd Bergmann wrote:
On Tuesday 21 October 2014 12:36:22 Pavel Machek wrote:quoted
On Fri 2014-10-17 01:12:21, Greg Kroah-Hartman wrote:quoted
On Thu, Oct 16, 2014 at 10:09:04AM -0700, John Stultz wrote:quoted
On Thu, Oct 16, 2014 at 5:47 AM, Greg Kroah-Hartman [off-list ref] wrote:quoted
From: Greg Kroah-Hartman <redacted>quoted
quoted
Are the Android guys comfortable with the ABI stability rules they'll now face?Just because something is in staging, doesn't mean you don't have to follow the same ABI stability rules as the rest of the kernel. If a change had happened to this code that broke userspace in the past, I would have reverted it. So this should not be anything different from what has been happening inthe past.Actually, there's big difference. If Al Viro changes core filesystem in a way that breaks staging/binder, binder is broken, and if it can't be fixed... well it can't be fixed. If Al Viro changes core filesystem in a way that breaks drivers/binder, Al's change is going to be reverted.One might have argued that we'd have to do that already, but the reasons for doing that with binder in the main kernel are certainly stronger.quoted
It is really hard to review without API documentation. Normally, API documentation is required for stuff like this. For example: does it add new files in /proc? Given that it is stable, can we get rid of binder_debug() and especially BINDER_DEBUG_ENTRY stuff?Good point. We require documentation for every single sysfs attribute that gets added to a driver (some escape the review, but that doesn't change the rule), so we should not make an exception for a new procfs file here.
Actually, it looked like it is debugfs file. Code was messy enough that I was not sure.
quoted
Could binder_transcation() be split to smaller functions according to CodingStyle? 17 goto targets at the end of function are not exactly easy to read. ginder_thread_read/write also needs splitting.Yes, in principle, but this is still a detail that would mainly serve to simplify review. The problem is more the lack of review and documentation of the API.
Yes, the problem is that code is impossible to review without API documentation. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html