Re: [PATCH v3 0/6] Android Binder IPC Fixes

From: Serban Constantinescu
Date: Tue Apr 30 2013 - 04:36:53 EST

Hi Arve,

On 30/04/13 00:13, Arve HjÃnnevÃg wrote:
On Mon, Apr 29, 2013 at 9:16 AM, Serban Constantinescu
<Serban.Constantinescu@xxxxxxx> wrote:
Hi all,

Any feedback or comments on this patch set?

You don't seem to have addressed my feedback on the previous patch set.

For v3 I have modified the following according to your review:

Changes for v3:
1: Dropped the patch that was replacing uint32_t types with unsigned int
2: Dropped the patch fixing the IOCTL types(since it has been added to Greg's
staging tree)
3: Split one patch into two: 'modify binder_write_read' and '64bit changes'
4: Modified BINDER_SET_MAX_THREADS ioctl definition accordint to Arve's review
5: Modified the binder command IOCTL declarations according to Arve's review

The following were left out:

On 11/04/13 22:40, Arve HjÃnnevÃg wrote:
OK, relaxing the alignment requirement for *offp to what the hardware
requires makes sense. Is there any macros in the kernel to help with
this, instead of hard-coding it to 4 bytes?

There is no kernel macro that I know which will help here(one that springs to mind is PTR_ALIGN but it aligns to (unsigned long) - we need one that aligns to (u32)). Any ideas?

On 11/04/13 21:38, Arve HjÃnnevÃg wrote:
OK, but if you are using this change let a 64 bit user-space know that
the driver has been fixed, then this patch needs to go after the
patches that change the structures on 64 bit systems.

For 32bit systems nothing has changed so they will continue to work as before. For 64bit systems the size of binder_version was signed long before the patch and __s32 after the patch is applied. Thus a 64bit system using the old interface will fail immediately after opening the binder driver, while cheeking the binder version (since the BINDER_VERSION ioctl will be different pre/post patch - size of binder_version differs).

For 64/32 systems once I will have the userspace wrapper ready I will add another ioctl(as discussed) that will check if the driver is 64bit ready(among the first things to do on binder_open).

Please let me know if there is anything that skipped my review and you would like to integrate in this patch set.

Thanks for your feedback and help,

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at