2014-02-17 21:58:53

by John Stultz

[permalink] [raw]
Subject: [PATCH 00/14][RFC] Android updates for staging-next

I recently went through the Android AOSP common tree and
cherry-picked out a number of changes that haven't yet
been submitted upstream.

I wanted to send them out for comment and potential
merging for 3.15 via staging-next.

Few minor notes:
* The first patch in the series (binder: Fix death notifications)
could potentially be merged in 3.14 as a fix.

* The binder ABI change patch had a large number of checkpatch
warnings for columns over 80 chars. I've fixed most of them,
but the remaining are all strings, which I don't have a good
sense of how to best address (splitting the string also give
warnings).

* The only other unaddressed checkpatch issue is:
WARNING: __packed is preferred over __attribute__((packed))
But I'm hesitant to change that as this is a uapi .h file and
I'm not sure if __packed is defined in userspace.

* The last patch isn't yet submitted to AOSP, but it seemed obvious
enough and if there's no objections I'll submit it shortly.

Comments, thoughts, suggestions?

thanks
-john

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Android Kernel Team <[email protected]>

Arve Hjønnevåg (2):
staging: binder: Fix death notifications
staging: binder: Support concurrent 32 bit and 64 bit processes.

Colin Cross (5):
staging: android: Split uapi out of android_alarm.h
staging: android: Split uapi out of ashmem.h
staging: android: split uapi out of sync.h and sw_sync.h
staging: android: Split uapi out of binder.h
staging: ion: Move shrinker out of heaps

John Stultz (1):
staging: binder: Improve Kconfig entry for ANDROID_BINDER_IPC_32BIT

Laura Abbott (1):
staging: ion: Fix debugfs handling of multiple kernel clients

Mitchel Humpherys (4):
staging: ion: Create separate heap and client debugfs directories
staging: ion: Store a copy of the client name on client creation
staging: ion: Make sure all clients are exposed in debugfs
staging: ion: Add private buffer flag to skip page pooling on free

Serban Constantinescu (1):
staging: binder: Fix ABI for 64bit Android

drivers/staging/android/Kconfig | 13 +
drivers/staging/android/android_alarm.h | 44 +---
drivers/staging/android/ashmem.h | 30 +--
drivers/staging/android/binder.c | 271 +++++++++++---------
drivers/staging/android/binder.h | 308 +---------------------
drivers/staging/android/binder_trace.h | 14 +-
drivers/staging/android/ion/ion.c | 121 +++++++--
drivers/staging/android/ion/ion_heap.c | 65 ++++-
drivers/staging/android/ion/ion_page_pool.c | 8 +-
drivers/staging/android/ion/ion_priv.h | 63 ++++-
drivers/staging/android/ion/ion_system_heap.c | 84 ++----
drivers/staging/android/sw_sync.h | 20 +-
drivers/staging/android/sync.h | 86 +------
drivers/staging/android/uapi/android_alarm.h | 62 +++++
drivers/staging/android/uapi/ashmem.h | 47 ++++
drivers/staging/android/uapi/binder.h | 351 ++++++++++++++++++++++++++
drivers/staging/android/uapi/sw_sync.h | 32 +++
drivers/staging/android/uapi/sync.h | 97 +++++++
18 files changed, 1014 insertions(+), 702 deletions(-)
create mode 100644 drivers/staging/android/uapi/android_alarm.h
create mode 100644 drivers/staging/android/uapi/ashmem.h
create mode 100644 drivers/staging/android/uapi/binder.h
create mode 100644 drivers/staging/android/uapi/sw_sync.h
create mode 100644 drivers/staging/android/uapi/sync.h

--
1.8.3.2


2014-02-17 21:59:09

by John Stultz

[permalink] [raw]
Subject: [PATCH 08/14] staging: ion: Store a copy of the client name on client creation

From: Mitchel Humpherys <[email protected]>

Currently, we copy the pointer passed in to ion_client_create without
making a copy of the string itself. This approach is problematic since
it relies on the client keeping the name string in working order.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Mitchel Humpherys <[email protected]>
[jstultz: Minor commit subject tweaks]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/ion/ion.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 4869420..eac4bce 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -735,19 +735,18 @@ struct ion_client *ion_client_create(struct ion_device *dev,
task_unlock(current->group_leader);

client = kzalloc(sizeof(struct ion_client), GFP_KERNEL);
- if (!client) {
- if (task)
- put_task_struct(current->group_leader);
- return ERR_PTR(-ENOMEM);
- }
+ if (!client)
+ goto err_put_task_struct;

client->dev = dev;
client->handles = RB_ROOT;
idr_init(&client->idr);
mutex_init(&client->lock);
- client->name = name;
client->task = task;
client->pid = pid;
+ client->name = kstrdup(name, GFP_KERNEL);
+ if (!client->name)
+ goto err_free_client;

down_write(&dev->lock);
p = &dev->clients.rb_node;
@@ -776,6 +775,13 @@ struct ion_client *ion_client_create(struct ion_device *dev,
up_write(&dev->lock);

return client;
+
+err_free_client:
+ kfree(client);
+err_put_task_struct:
+ if (task)
+ put_task_struct(current->group_leader);
+ return ERR_PTR(-ENOMEM);
}
EXPORT_SYMBOL(ion_client_create);

@@ -800,6 +806,7 @@ void ion_client_destroy(struct ion_client *client)
debugfs_remove_recursive(client->debug_root);
up_write(&dev->lock);

+ kfree(client->name);
kfree(client);
}
EXPORT_SYMBOL(ion_client_destroy);
--
1.8.3.2

2014-02-17 21:59:18

by John Stultz

[permalink] [raw]
Subject: [PATCH 13/14] staging: binder: Support concurrent 32 bit and 64 bit processes.

From: Arve Hjønnevåg <[email protected]>

Add binder_size_t and binder_uintptr_t that is used instead of size_t and
void __user * in the user-space interface.

Use 64 bit pointers on all systems unless CONFIG_ANDROID_BINDER_IPC_32BIT
is set (which enables the old protocol on 32 bit systems).

Change BINDER_CURRENT_PROTOCOL_VERSION to 8 if
CONFIG_ANDROID_BINDER_IPC_32BIT is not set.

Add compat ioctl.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Arve Hjønnevåg <[email protected]>
[jstultz: Merged with upstream type changes. Tweaked commit message.
Various whitespace fixes and longer Kconfig description for checkpatch]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/Kconfig | 12 ++
drivers/staging/android/binder.c | 268 ++++++++++++++++++---------------
drivers/staging/android/binder.h | 4 +
drivers/staging/android/binder_trace.h | 14 +-
drivers/staging/android/uapi/binder.h | 64 +++++---
5 files changed, 213 insertions(+), 149 deletions(-)

diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
index b91c758..78e82f5 100644
--- a/drivers/staging/android/Kconfig
+++ b/drivers/staging/android/Kconfig
@@ -20,6 +20,18 @@ config ANDROID_BINDER_IPC
Android process, using Binder to identify, invoke and pass arguments
between said processes.

+config ANDROID_BINDER_IPC_32BIT
+ bool "Use old 32-bit binder api"
+ depends on !64BIT
+ ---help---
+ The Binder API has been changed to support both 32 and 64bit
+ applications in a mixed environment.
+
+ Enable this to support an old 32-bit Android user-space (v4.4 and
+ earlier).
+
+ Note that eabling this will break newer Android user-space.
+
config ASHMEM
bool "Enable the Anonymous Shared Memory Subsystem"
default n
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 1432d95..cfe4bc8 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -228,8 +228,8 @@ struct binder_node {
int internal_strong_refs;
int local_weak_refs;
int local_strong_refs;
- void __user *ptr;
- void __user *cookie;
+ binder_uintptr_t ptr;
+ binder_uintptr_t cookie;
unsigned has_strong_ref:1;
unsigned pending_strong_ref:1;
unsigned has_weak_ref:1;
@@ -242,7 +242,7 @@ struct binder_node {

struct binder_ref_death {
struct binder_work work;
- void __user *cookie;
+ binder_uintptr_t cookie;
};

struct binder_ref {
@@ -515,14 +515,14 @@ static void binder_insert_allocated_buffer(struct binder_proc *proc,
}

static struct binder_buffer *binder_buffer_lookup(struct binder_proc *proc,
- void __user *user_ptr)
+ uintptr_t user_ptr)
{
struct rb_node *n = proc->allocated_buffers.rb_node;
struct binder_buffer *buffer;
struct binder_buffer *kern_ptr;

- kern_ptr = user_ptr - proc->user_buffer_offset
- - offsetof(struct binder_buffer, data);
+ kern_ptr = (struct binder_buffer *)(user_ptr - proc->user_buffer_offset
+ - offsetof(struct binder_buffer, data));

while (n) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
@@ -856,7 +856,7 @@ static void binder_free_buf(struct binder_proc *proc,
}

static struct binder_node *binder_get_node(struct binder_proc *proc,
- void __user *ptr)
+ binder_uintptr_t ptr)
{
struct rb_node *n = proc->nodes.rb_node;
struct binder_node *node;
@@ -875,8 +875,8 @@ static struct binder_node *binder_get_node(struct binder_proc *proc,
}

static struct binder_node *binder_new_node(struct binder_proc *proc,
- void __user *ptr,
- void __user *cookie)
+ binder_uintptr_t ptr,
+ binder_uintptr_t cookie)
{
struct rb_node **p = &proc->nodes.rb_node;
struct rb_node *parent = NULL;
@@ -908,9 +908,9 @@ static struct binder_node *binder_new_node(struct binder_proc *proc,
INIT_LIST_HEAD(&node->work.entry);
INIT_LIST_HEAD(&node->async_todo);
binder_debug(BINDER_DEBUG_INTERNAL_REFS,
- "%d:%d node %d u%p c%p created\n",
+ "%d:%d node %d u%016llx c%016llx created\n",
proc->pid, current->pid, node->debug_id,
- node->ptr, node->cookie);
+ (u64)node->ptr, (u64)node->cookie);
return node;
}

@@ -1226,9 +1226,9 @@ static void binder_send_failed_reply(struct binder_transaction *t,

static void binder_transaction_buffer_release(struct binder_proc *proc,
struct binder_buffer *buffer,
- size_t *failed_at)
+ binder_size_t *failed_at)
{
- size_t *offp, *off_end;
+ binder_size_t *offp, *off_end;
int debug_id = buffer->debug_id;

binder_debug(BINDER_DEBUG_TRANSACTION,
@@ -1239,7 +1239,8 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
if (buffer->target_node)
binder_dec_node(buffer->target_node, 1, 0);

- offp = (size_t *)(buffer->data + ALIGN(buffer->data_size, sizeof(void *)));
+ offp = (binder_size_t *)(buffer->data +
+ ALIGN(buffer->data_size, sizeof(void *)));
if (failed_at)
off_end = failed_at;
else
@@ -1249,8 +1250,8 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
if (*offp > buffer->data_size - sizeof(*fp) ||
buffer->data_size < sizeof(*fp) ||
!IS_ALIGNED(*offp, sizeof(u32))) {
- pr_err("transaction release %d bad offset %zd, size %zd\n",
- debug_id, *offp, buffer->data_size);
+ pr_err("transaction release %d bad offset %lld, size %zd\n",
+ debug_id, (u64)*offp, buffer->data_size);
continue;
}
fp = (struct flat_binder_object *)(buffer->data + *offp);
@@ -1259,13 +1260,13 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
case BINDER_TYPE_WEAK_BINDER: {
struct binder_node *node = binder_get_node(proc, fp->binder);
if (node == NULL) {
- pr_err("transaction release %d bad node %p\n",
- debug_id, fp->binder);
+ pr_err("transaction release %d bad node %016llx\n",
+ debug_id, (u64)fp->binder);
break;
}
binder_debug(BINDER_DEBUG_TRANSACTION,
- " node %d u%p\n",
- node->debug_id, node->ptr);
+ " node %d u%016llx\n",
+ node->debug_id, (u64)node->ptr);
binder_dec_node(node, fp->type == BINDER_TYPE_BINDER, 0);
} break;
case BINDER_TYPE_HANDLE:
@@ -1303,7 +1304,7 @@ static void binder_transaction(struct binder_proc *proc,
{
struct binder_transaction *t;
struct binder_work *tcomplete;
- size_t *offp, *off_end;
+ binder_size_t *offp, *off_end;
struct binder_proc *target_proc;
struct binder_thread *target_thread = NULL;
struct binder_node *target_node = NULL;
@@ -1432,18 +1433,20 @@ static void binder_transaction(struct binder_proc *proc,

if (reply)
binder_debug(BINDER_DEBUG_TRANSACTION,
- "%d:%d BC_REPLY %d -> %d:%d, data %p-%p size %zd-%zd\n",
+ "%d:%d BC_REPLY %d -> %d:%d, data %016llx-%016llx size %lld-%lld\n",
proc->pid, thread->pid, t->debug_id,
target_proc->pid, target_thread->pid,
- tr->data.ptr.buffer, tr->data.ptr.offsets,
- tr->data_size, tr->offsets_size);
+ (u64)tr->data.ptr.buffer,
+ (u64)tr->data.ptr.offsets,
+ (u64)tr->data_size, (u64)tr->offsets_size);
else
binder_debug(BINDER_DEBUG_TRANSACTION,
- "%d:%d BC_TRANSACTION %d -> %d - node %d, data %p-%p size %zd-%zd\n",
+ "%d:%d BC_TRANSACTION %d -> %d - node %d, data %016llx-%016llx size %lld-%lld\n",
proc->pid, thread->pid, t->debug_id,
target_proc->pid, target_node->debug_id,
- tr->data.ptr.buffer, tr->data.ptr.offsets,
- tr->data_size, tr->offsets_size);
+ (u64)tr->data.ptr.buffer,
+ (u64)tr->data.ptr.offsets,
+ (u64)tr->data_size, (u64)tr->offsets_size);

if (!reply && !(tr->flags & TF_ONE_WAY))
t->from = thread;
@@ -1472,23 +1475,26 @@ static void binder_transaction(struct binder_proc *proc,
if (target_node)
binder_inc_node(target_node, 1, 0, NULL);

- offp = (size_t *)(t->buffer->data + ALIGN(tr->data_size, sizeof(void *)));
+ offp = (binder_size_t *)(t->buffer->data +
+ ALIGN(tr->data_size, sizeof(void *)));

- if (copy_from_user(t->buffer->data, tr->data.ptr.buffer, tr->data_size)) {
+ if (copy_from_user(t->buffer->data, (const void __user *)(uintptr_t)
+ tr->data.ptr.buffer, tr->data_size)) {
binder_user_error("%d:%d got transaction with invalid data ptr\n",
proc->pid, thread->pid);
return_error = BR_FAILED_REPLY;
goto err_copy_data_failed;
}
- if (copy_from_user(offp, tr->data.ptr.offsets, tr->offsets_size)) {
+ if (copy_from_user(offp, (const void __user *)(uintptr_t)
+ tr->data.ptr.offsets, tr->offsets_size)) {
binder_user_error("%d:%d got transaction with invalid offsets ptr\n",
proc->pid, thread->pid);
return_error = BR_FAILED_REPLY;
goto err_copy_data_failed;
}
- if (!IS_ALIGNED(tr->offsets_size, sizeof(size_t))) {
- binder_user_error("%d:%d got transaction with invalid offsets size, %zd\n",
- proc->pid, thread->pid, tr->offsets_size);
+ if (!IS_ALIGNED(tr->offsets_size, sizeof(binder_size_t))) {
+ binder_user_error("%d:%d got transaction with invalid offsets size, %lld\n",
+ proc->pid, thread->pid, (u64)tr->offsets_size);
return_error = BR_FAILED_REPLY;
goto err_bad_offset;
}
@@ -1498,8 +1504,8 @@ static void binder_transaction(struct binder_proc *proc,
if (*offp > t->buffer->data_size - sizeof(*fp) ||
t->buffer->data_size < sizeof(*fp) ||
!IS_ALIGNED(*offp, sizeof(u32))) {
- binder_user_error("%d:%d got transaction with invalid offset, %zd\n",
- proc->pid, thread->pid, *offp);
+ binder_user_error("%d:%d got transaction with invalid offset, %lld\n",
+ proc->pid, thread->pid, (u64)*offp);
return_error = BR_FAILED_REPLY;
goto err_bad_offset;
}
@@ -1519,10 +1525,10 @@ static void binder_transaction(struct binder_proc *proc,
node->accept_fds = !!(fp->flags & FLAT_BINDER_FLAG_ACCEPTS_FDS);
}
if (fp->cookie != node->cookie) {
- binder_user_error("%d:%d sending u%p node %d, cookie mismatch %p != %p\n",
+ binder_user_error("%d:%d sending u%016llx node %d, cookie mismatch %016llx != %016llx\n",
proc->pid, thread->pid,
- fp->binder, node->debug_id,
- fp->cookie, node->cookie);
+ (u64)fp->binder, node->debug_id,
+ (u64)fp->cookie, (u64)node->cookie);
goto err_binder_get_ref_for_node_failed;
}
ref = binder_get_ref_for_node(target_proc, node);
@@ -1540,9 +1546,9 @@ static void binder_transaction(struct binder_proc *proc,

trace_binder_transaction_node_to_ref(t, node, ref);
binder_debug(BINDER_DEBUG_TRANSACTION,
- " node %d u%p -> ref %d desc %d\n",
- node->debug_id, node->ptr, ref->debug_id,
- ref->desc);
+ " node %d u%016llx -> ref %d desc %d\n",
+ node->debug_id, (u64)node->ptr,
+ ref->debug_id, ref->desc);
} break;
case BINDER_TYPE_HANDLE:
case BINDER_TYPE_WEAK_HANDLE: {
@@ -1564,9 +1570,9 @@ static void binder_transaction(struct binder_proc *proc,
binder_inc_node(ref->node, fp->type == BINDER_TYPE_BINDER, 0, NULL);
trace_binder_transaction_ref_to_node(t, ref);
binder_debug(BINDER_DEBUG_TRANSACTION,
- " ref %d desc %d -> node %d u%p\n",
+ " ref %d desc %d -> node %d u%016llx\n",
ref->debug_id, ref->desc, ref->node->debug_id,
- ref->node->ptr);
+ (u64)ref->node->ptr);
} else {
struct binder_ref *new_ref;
new_ref = binder_get_ref_for_node(target_proc, ref->node);
@@ -1682,9 +1688,9 @@ err_dead_binder:
err_invalid_target_handle:
err_no_context_mgr_node:
binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
- "%d:%d transaction failed %d, size %zd-%zd\n",
+ "%d:%d transaction failed %d, size %lld-%lld\n",
proc->pid, thread->pid, return_error,
- tr->data_size, tr->offsets_size);
+ (u64)tr->data_size, (u64)tr->offsets_size);

{
struct binder_transaction_log_entry *fe;
@@ -1702,9 +1708,11 @@ err_no_context_mgr_node:

static int binder_thread_write(struct binder_proc *proc,
struct binder_thread *thread,
- void __user *buffer, size_t size, size_t *consumed)
+ binder_uintptr_t binder_buffer, size_t size,
+ binder_size_t *consumed)
{
uint32_t cmd;
+ void __user *buffer = (void __user *)(uintptr_t)binder_buffer;
void __user *ptr = buffer + *consumed;
void __user *end = buffer + size;

@@ -1773,33 +1781,33 @@ static int binder_thread_write(struct binder_proc *proc,
}
case BC_INCREFS_DONE:
case BC_ACQUIRE_DONE: {
- void __user *node_ptr;
- void __user *cookie;
+ binder_uintptr_t node_ptr;
+ binder_uintptr_t cookie;
struct binder_node *node;

- if (get_user(node_ptr, (void * __user *)ptr))
+ if (get_user(node_ptr, (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
- if (get_user(cookie, (void * __user *)ptr))
+ ptr += sizeof(binder_uintptr_t);
+ if (get_user(cookie, (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
+ ptr += sizeof(binder_uintptr_t);
node = binder_get_node(proc, node_ptr);
if (node == NULL) {
- binder_user_error("%d:%d %s u%p no match\n",
+ binder_user_error("%d:%d %s u%016llx no match\n",
proc->pid, thread->pid,
cmd == BC_INCREFS_DONE ?
"BC_INCREFS_DONE" :
"BC_ACQUIRE_DONE",
- node_ptr);
+ (u64)node_ptr);
break;
}
if (cookie != node->cookie) {
- binder_user_error("%d:%d %s u%p node %d cookie mismatch %p != %p\n",
+ binder_user_error("%d:%d %s u%016llx node %d cookie mismatch %016llx != %016llx\n",
proc->pid, thread->pid,
cmd == BC_INCREFS_DONE ?
"BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
- node_ptr, node->debug_id,
- cookie, node->cookie);
+ (u64)node_ptr, node->debug_id,
+ (u64)cookie, (u64)node->cookie);
break;
}
if (cmd == BC_ACQUIRE_DONE) {
@@ -1835,27 +1843,28 @@ static int binder_thread_write(struct binder_proc *proc,
return -EINVAL;

case BC_FREE_BUFFER: {
- void __user *data_ptr;
+ binder_uintptr_t data_ptr;
struct binder_buffer *buffer;

- if (get_user(data_ptr, (void * __user *)ptr))
+ if (get_user(data_ptr, (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
+ ptr += sizeof(binder_uintptr_t);

buffer = binder_buffer_lookup(proc, data_ptr);
if (buffer == NULL) {
- binder_user_error("%d:%d BC_FREE_BUFFER u%p no match\n",
- proc->pid, thread->pid, data_ptr);
+ binder_user_error("%d:%d BC_FREE_BUFFER u%016llx no match\n",
+ proc->pid, thread->pid, (u64)data_ptr);
break;
}
if (!buffer->allow_user_free) {
- binder_user_error("%d:%d BC_FREE_BUFFER u%p matched unreturned buffer\n",
- proc->pid, thread->pid, data_ptr);
+ binder_user_error("%d:%d BC_FREE_BUFFER u%016llx matched unreturned buffer\n",
+ proc->pid, thread->pid, (u64)data_ptr);
break;
}
binder_debug(BINDER_DEBUG_FREE_BUFFER,
- "%d:%d BC_FREE_BUFFER u%p found buffer %d for %s transaction\n",
- proc->pid, thread->pid, data_ptr, buffer->debug_id,
+ "%d:%d BC_FREE_BUFFER u%016llx found buffer %d for %s transaction\n",
+ proc->pid, thread->pid, (u64)data_ptr,
+ buffer->debug_id,
buffer->transaction ? "active" : "finished");

if (buffer->transaction) {
@@ -1925,16 +1934,16 @@ static int binder_thread_write(struct binder_proc *proc,
case BC_REQUEST_DEATH_NOTIFICATION:
case BC_CLEAR_DEATH_NOTIFICATION: {
uint32_t target;
- void __user *cookie;
+ binder_uintptr_t cookie;
struct binder_ref *ref;
struct binder_ref_death *death;

if (get_user(target, (uint32_t __user *)ptr))
return -EFAULT;
ptr += sizeof(uint32_t);
- if (get_user(cookie, (void __user * __user *)ptr))
+ if (get_user(cookie, (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
+ ptr += sizeof(binder_uintptr_t);
ref = binder_get_ref(proc, target);
if (ref == NULL) {
binder_user_error("%d:%d %s invalid ref %d\n",
@@ -1947,12 +1956,12 @@ static int binder_thread_write(struct binder_proc *proc,
}

binder_debug(BINDER_DEBUG_DEATH_NOTIFICATION,
- "%d:%d %s %p ref %d desc %d s %d w %d for node %d\n",
+ "%d:%d %s %016llx ref %d desc %d s %d w %d for node %d\n",
proc->pid, thread->pid,
cmd == BC_REQUEST_DEATH_NOTIFICATION ?
"BC_REQUEST_DEATH_NOTIFICATION" :
"BC_CLEAR_DEATH_NOTIFICATION",
- cookie, ref->debug_id, ref->desc,
+ (u64)cookie, ref->debug_id, ref->desc,
ref->strong, ref->weak, ref->node->debug_id);

if (cmd == BC_REQUEST_DEATH_NOTIFICATION) {
@@ -1990,9 +1999,10 @@ static int binder_thread_write(struct binder_proc *proc,
}
death = ref->death;
if (death->cookie != cookie) {
- binder_user_error("%d:%d BC_CLEAR_DEATH_NOTIFICATION death notification cookie mismatch %p != %p\n",
+ binder_user_error("%d:%d BC_CLEAR_DEATH_NOTIFICATION death notification cookie mismatch %016llx != %016llx\n",
proc->pid, thread->pid,
- death->cookie, cookie);
+ (u64)death->cookie,
+ (u64)cookie);
break;
}
ref->death = NULL;
@@ -2012,9 +2022,9 @@ static int binder_thread_write(struct binder_proc *proc,
} break;
case BC_DEAD_BINDER_DONE: {
struct binder_work *w;
- void __user *cookie;
+ binder_uintptr_t cookie;
struct binder_ref_death *death = NULL;
- if (get_user(cookie, (void __user * __user *)ptr))
+ if (get_user(cookie, (binder_uintptr_t __user *)ptr))
return -EFAULT;

ptr += sizeof(void *);
@@ -2026,11 +2036,12 @@ static int binder_thread_write(struct binder_proc *proc,
}
}
binder_debug(BINDER_DEBUG_DEAD_BINDER,
- "%d:%d BC_DEAD_BINDER_DONE %p found %p\n",
- proc->pid, thread->pid, cookie, death);
+ "%d:%d BC_DEAD_BINDER_DONE %016llx found %p\n",
+ proc->pid, thread->pid, (u64)cookie,
+ death);
if (death == NULL) {
- binder_user_error("%d:%d BC_DEAD_BINDER_DONE %p not found\n",
- proc->pid, thread->pid, cookie);
+ binder_user_error("%d:%d BC_DEAD_BINDER_DONE %016llx not found\n",
+ proc->pid, thread->pid, (u64)cookie);
break;
}

@@ -2082,9 +2093,10 @@ static int binder_has_thread_work(struct binder_thread *thread)

static int binder_thread_read(struct binder_proc *proc,
struct binder_thread *thread,
- void __user *buffer, size_t size,
- size_t *consumed, int non_block)
+ binder_uintptr_t binder_buffer, size_t size,
+ binder_size_t *consumed, int non_block)
{
+ void __user *buffer = (void __user *)(uintptr_t)binder_buffer;
void __user *ptr = buffer + *consumed;
void __user *end = buffer + size;

@@ -2229,32 +2241,40 @@ retry:
if (put_user(cmd, (uint32_t __user *)ptr))
return -EFAULT;
ptr += sizeof(uint32_t);
- if (put_user(node->ptr, (void * __user *)ptr))
+ if (put_user(node->ptr,
+ (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
- if (put_user(node->cookie, (void * __user *)ptr))
+ ptr += sizeof(binder_uintptr_t);
+ if (put_user(node->cookie,
+ (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
+ ptr += sizeof(binder_uintptr_t);

binder_stat_br(proc, thread, cmd);
binder_debug(BINDER_DEBUG_USER_REFS,
- "%d:%d %s %d u%p c%p\n",
- proc->pid, thread->pid, cmd_name, node->debug_id, node->ptr, node->cookie);
+ "%d:%d %s %d u%016llx c%016llx\n",
+ proc->pid, thread->pid, cmd_name,
+ node->debug_id,
+ (u64)node->ptr, (u64)node->cookie);
} else {
list_del_init(&w->entry);
if (!weak && !strong) {
binder_debug(BINDER_DEBUG_INTERNAL_REFS,
- "%d:%d node %d u%p c%p deleted\n",
- proc->pid, thread->pid, node->debug_id,
- node->ptr, node->cookie);
+ "%d:%d node %d u%016llx c%016llx deleted\n",
+ proc->pid, thread->pid,
+ node->debug_id,
+ (u64)node->ptr,
+ (u64)node->cookie);
rb_erase(&node->rb_node, &proc->nodes);
kfree(node);
binder_stats_deleted(BINDER_STAT_NODE);
} else {
binder_debug(BINDER_DEBUG_INTERNAL_REFS,
- "%d:%d node %d u%p c%p state unchanged\n",
- proc->pid, thread->pid, node->debug_id, node->ptr,
- node->cookie);
+ "%d:%d node %d u%016llx c%016llx state unchanged\n",
+ proc->pid, thread->pid,
+ node->debug_id,
+ (u64)node->ptr,
+ (u64)node->cookie);
}
}
} break;
@@ -2272,17 +2292,18 @@ retry:
if (put_user(cmd, (uint32_t __user *)ptr))
return -EFAULT;
ptr += sizeof(uint32_t);
- if (put_user(death->cookie, (void * __user *)ptr))
+ if (put_user(death->cookie,
+ (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
+ ptr += sizeof(binder_uintptr_t);
binder_stat_br(proc, thread, cmd);
binder_debug(BINDER_DEBUG_DEATH_NOTIFICATION,
- "%d:%d %s %p\n",
+ "%d:%d %s %016llx\n",
proc->pid, thread->pid,
cmd == BR_DEAD_BINDER ?
"BR_DEAD_BINDER" :
"BR_CLEAR_DEATH_NOTIFICATION_DONE",
- death->cookie);
+ (u64)death->cookie);

if (w->type == BINDER_WORK_CLEAR_DEATH_NOTIFICATION) {
list_del(&w->entry);
@@ -2312,8 +2333,8 @@ retry:
binder_set_nice(target_node->min_priority);
cmd = BR_TRANSACTION;
} else {
- tr.target.ptr = NULL;
- tr.cookie = NULL;
+ tr.target.ptr = 0;
+ tr.cookie = 0;
cmd = BR_REPLY;
}
tr.code = t->code;
@@ -2330,8 +2351,9 @@ retry:

tr.data_size = t->buffer->data_size;
tr.offsets_size = t->buffer->offsets_size;
- tr.data.ptr.buffer = (void *)t->buffer->data +
- proc->user_buffer_offset;
+ tr.data.ptr.buffer = (binder_uintptr_t)(
+ (uintptr_t)t->buffer->data +
+ proc->user_buffer_offset);
tr.data.ptr.offsets = tr.data.ptr.buffer +
ALIGN(t->buffer->data_size,
sizeof(void *));
@@ -2346,14 +2368,14 @@ retry:
trace_binder_transaction_received(t);
binder_stat_br(proc, thread, cmd);
binder_debug(BINDER_DEBUG_TRANSACTION,
- "%d:%d %s %d %d:%d, cmd %d size %zd-%zd ptr %p-%p\n",
+ "%d:%d %s %d %d:%d, cmd %d size %zd-%zd ptr %016llx-%016llx\n",
proc->pid, thread->pid,
(cmd == BR_TRANSACTION) ? "BR_TRANSACTION" :
"BR_REPLY",
t->debug_id, t->from ? t->from->proc->pid : 0,
t->from ? t->from->pid : 0, cmd,
t->buffer->data_size, t->buffer->offsets_size,
- tr.data.ptr.buffer, tr.data.ptr.offsets);
+ (u64)tr.data.ptr.buffer, (u64)tr.data.ptr.offsets);

list_del(&t->work.entry);
t->buffer->allow_user_free = 1;
@@ -2423,8 +2445,8 @@ static void binder_release_work(struct list_head *list)

death = container_of(w, struct binder_ref_death, work);
binder_debug(BINDER_DEBUG_DEAD_TRANSACTION,
- "undelivered death notification, %p\n",
- death->cookie);
+ "undelivered death notification, %016llx\n",
+ (u64)death->cookie);
kfree(death);
binder_stats_deleted(BINDER_STAT_DEATH);
} break;
@@ -2580,12 +2602,16 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
goto err;
}
binder_debug(BINDER_DEBUG_READ_WRITE,
- "%d:%d write %zd at %016lx, read %zd at %016lx\n",
- proc->pid, thread->pid, bwr.write_size,
- bwr.write_buffer, bwr.read_size, bwr.read_buffer);
+ "%d:%d write %lld at %016llx, read %lld at %016llx\n",
+ proc->pid, thread->pid,
+ (u64)bwr.write_size, (u64)bwr.write_buffer,
+ (u64)bwr.read_size, (u64)bwr.read_buffer);

if (bwr.write_size > 0) {
- ret = binder_thread_write(proc, thread, (void __user *)bwr.write_buffer, bwr.write_size, &bwr.write_consumed);
+ ret = binder_thread_write(proc, thread,
+ bwr.write_buffer,
+ bwr.write_size,
+ &bwr.write_consumed);
trace_binder_write_done(ret);
if (ret < 0) {
bwr.read_consumed = 0;
@@ -2595,7 +2621,10 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
}
}
if (bwr.read_size > 0) {
- ret = binder_thread_read(proc, thread, (void __user *)bwr.read_buffer, bwr.read_size, &bwr.read_consumed, filp->f_flags & O_NONBLOCK);
+ ret = binder_thread_read(proc, thread, bwr.read_buffer,
+ bwr.read_size,
+ &bwr.read_consumed,
+ filp->f_flags & O_NONBLOCK);
trace_binder_read_done(ret);
if (!list_empty(&proc->todo))
wake_up_interruptible(&proc->wait);
@@ -2606,9 +2635,10 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
}
}
binder_debug(BINDER_DEBUG_READ_WRITE,
- "%d:%d wrote %zd of %zd, read return %zd of %zd\n",
- proc->pid, thread->pid, bwr.write_consumed, bwr.write_size,
- bwr.read_consumed, bwr.read_size);
+ "%d:%d wrote %lld of %lld, read return %lld of %lld\n",
+ proc->pid, thread->pid,
+ (u64)bwr.write_consumed, (u64)bwr.write_size,
+ (u64)bwr.read_consumed, (u64)bwr.read_size);
if (copy_to_user(ubuf, &bwr, sizeof(bwr))) {
ret = -EFAULT;
goto err;
@@ -2637,7 +2667,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
}
} else
binder_context_mgr_uid = current->cred->euid;
- binder_context_mgr_node = binder_new_node(proc, NULL, NULL);
+ binder_context_mgr_node = binder_new_node(proc, 0, 0);
if (binder_context_mgr_node == NULL) {
ret = -ENOMEM;
goto err;
@@ -3132,8 +3162,9 @@ static void print_binder_work(struct seq_file *m, const char *prefix,
break;
case BINDER_WORK_NODE:
node = container_of(w, struct binder_node, work);
- seq_printf(m, "%snode work %d: u%p c%p\n",
- prefix, node->debug_id, node->ptr, node->cookie);
+ seq_printf(m, "%snode work %d: u%016llx c%016llx\n",
+ prefix, node->debug_id,
+ (u64)node->ptr, (u64)node->cookie);
break;
case BINDER_WORK_DEAD_BINDER:
seq_printf(m, "%shas dead binder\n", prefix);
@@ -3193,8 +3224,8 @@ static void print_binder_node(struct seq_file *m, struct binder_node *node)
hlist_for_each_entry(ref, &node->refs, node_entry)
count++;

- seq_printf(m, " node %d: u%p c%p hs %d hw %d ls %d lw %d is %d iw %d",
- node->debug_id, node->ptr, node->cookie,
+ seq_printf(m, " node %d: u%016llx c%016llx hs %d hw %d ls %d lw %d is %d iw %d",
+ node->debug_id, (u64)node->ptr, (u64)node->cookie,
node->has_strong_ref, node->has_weak_ref,
node->local_strong_refs, node->local_weak_refs,
node->internal_strong_refs, count);
@@ -3496,6 +3527,7 @@ static const struct file_operations binder_fops = {
.owner = THIS_MODULE,
.poll = binder_poll,
.unlocked_ioctl = binder_ioctl,
+ .compat_ioctl = binder_ioctl,
.mmap = binder_mmap,
.open = binder_open,
.flush = binder_flush,
diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index d4101a6..eb08346 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/binder.h
@@ -20,6 +20,10 @@
#ifndef _LINUX_BINDER_H
#define _LINUX_BINDER_H

+#ifdef CONFIG_ANDROID_BINDER_IPC_32BIT
+#define BINDER_IPC_32BIT 1
+#endif
+
#include "uapi/binder.h"

#endif /* _LINUX_BINDER_H */
diff --git a/drivers/staging/android/binder_trace.h b/drivers/staging/android/binder_trace.h
index 82a567c..7f20f3d 100644
--- a/drivers/staging/android/binder_trace.h
+++ b/drivers/staging/android/binder_trace.h
@@ -152,7 +152,7 @@ TRACE_EVENT(binder_transaction_node_to_ref,
TP_STRUCT__entry(
__field(int, debug_id)
__field(int, node_debug_id)
- __field(void __user *, node_ptr)
+ __field(binder_uintptr_t, node_ptr)
__field(int, ref_debug_id)
__field(uint32_t, ref_desc)
),
@@ -163,8 +163,9 @@ TRACE_EVENT(binder_transaction_node_to_ref,
__entry->ref_debug_id = ref->debug_id;
__entry->ref_desc = ref->desc;
),
- TP_printk("transaction=%d node=%d src_ptr=0x%p ==> dest_ref=%d dest_desc=%d",
- __entry->debug_id, __entry->node_debug_id, __entry->node_ptr,
+ TP_printk("transaction=%d node=%d src_ptr=0x%016llx ==> dest_ref=%d dest_desc=%d",
+ __entry->debug_id, __entry->node_debug_id,
+ (u64)__entry->node_ptr,
__entry->ref_debug_id, __entry->ref_desc)
);

@@ -177,7 +178,7 @@ TRACE_EVENT(binder_transaction_ref_to_node,
__field(int, ref_debug_id)
__field(uint32_t, ref_desc)
__field(int, node_debug_id)
- __field(void __user *, node_ptr)
+ __field(binder_uintptr_t, node_ptr)
),
TP_fast_assign(
__entry->debug_id = t->debug_id;
@@ -186,9 +187,10 @@ TRACE_EVENT(binder_transaction_ref_to_node,
__entry->node_debug_id = ref->node->debug_id;
__entry->node_ptr = ref->node->ptr;
),
- TP_printk("transaction=%d node=%d src_ref=%d src_desc=%d ==> dest_ptr=0x%p",
+ TP_printk("transaction=%d node=%d src_ref=%d src_desc=%d ==> dest_ptr=0x%016llx",
__entry->debug_id, __entry->node_debug_id,
- __entry->ref_debug_id, __entry->ref_desc, __entry->node_ptr)
+ __entry->ref_debug_id, __entry->ref_desc,
+ (u64)__entry->node_ptr)
);

TRACE_EVENT(binder_transaction_ref_to_ref,
diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
index 4071fcf..904adb7 100644
--- a/drivers/staging/android/uapi/binder.h
+++ b/drivers/staging/android/uapi/binder.h
@@ -39,6 +39,14 @@ enum {
FLAT_BINDER_FLAG_ACCEPTS_FDS = 0x100,
};

+#ifdef BINDER_IPC_32BIT
+typedef __u32 binder_size_t;
+typedef __u32 binder_uintptr_t;
+#else
+typedef __u64 binder_size_t;
+typedef __u64 binder_uintptr_t;
+#endif
+
/*
* This is the flattened representation of a Binder object for transfer
* between processes. The 'offsets' supplied as part of a binder transaction
@@ -53,12 +61,12 @@ struct flat_binder_object {

/* 8 bytes of data. */
union {
- void __user *binder; /* local object */
- __u32 handle; /* remote object */
+ binder_uintptr_t binder; /* local object */
+ __u32 handle; /* remote object */
};

/* extra data associated with local object */
- void __user *cookie;
+ binder_uintptr_t cookie;
};

/*
@@ -67,12 +75,12 @@ struct flat_binder_object {
*/

struct binder_write_read {
- size_t write_size; /* bytes to write */
- size_t write_consumed; /* bytes consumed by driver */
- unsigned long write_buffer;
- size_t read_size; /* bytes to read */
- size_t read_consumed; /* bytes consumed by driver */
- unsigned long read_buffer;
+ binder_size_t write_size; /* bytes to write */
+ binder_size_t write_consumed; /* bytes consumed by driver */
+ binder_uintptr_t write_buffer;
+ binder_size_t read_size; /* bytes to read */
+ binder_size_t read_consumed; /* bytes consumed by driver */
+ binder_uintptr_t read_buffer;
};

/* Use with BINDER_VERSION, driver fills in fields. */
@@ -82,7 +90,11 @@ struct binder_version {
};

/* This is the current protocol version. */
+#ifdef BINDER_IPC_32BIT
#define BINDER_CURRENT_PROTOCOL_VERSION 7
+#else
+#define BINDER_CURRENT_PROTOCOL_VERSION 8
+#endif

#define BINDER_WRITE_READ _IOWR('b', 1, struct binder_write_read)
#define BINDER_SET_IDLE_TIMEOUT _IOW('b', 3, __s64)
@@ -119,18 +131,20 @@ struct binder_transaction_data {
* identifying the target and contents of the transaction.
*/
union {
- __u32 handle; /* target descriptor of command transaction */
- void *ptr; /* target descriptor of return transaction */
+ /* target descriptor of command transaction */
+ __u32 handle;
+ /* target descriptor of return transaction */
+ binder_uintptr_t ptr;
} target;
- void *cookie; /* target object cookie */
+ binder_uintptr_t cookie; /* target object cookie */
__u32 code; /* transaction command */

/* General information about the transaction. */
__u32 flags;
pid_t sender_pid;
uid_t sender_euid;
- size_t data_size; /* number of bytes of data */
- size_t offsets_size; /* number of bytes of offsets */
+ binder_size_t data_size; /* number of bytes of data */
+ binder_size_t offsets_size; /* number of bytes of offsets */

/* If this transaction is inline, the data immediately
* follows here; otherwise, it ends with a pointer to
@@ -139,22 +153,22 @@ struct binder_transaction_data {
union {
struct {
/* transaction data */
- const void __user *buffer;
+ binder_uintptr_t buffer;
/* offsets from buffer to flat_binder_object structs */
- const void __user *offsets;
+ binder_uintptr_t offsets;
} ptr;
__u8 buf[8];
} data;
};

struct binder_ptr_cookie {
- void *ptr;
- void *cookie;
+ binder_uintptr_t ptr;
+ binder_uintptr_t cookie;
};

struct binder_handle_cookie {
__u32 handle;
- void *cookie;
+ binder_uintptr_t cookie;
} __attribute__((packed));

struct binder_pri_desc {
@@ -164,8 +178,8 @@ struct binder_pri_desc {

struct binder_pri_ptr_cookie {
__s32 priority;
- void *ptr;
- void *cookie;
+ binder_uintptr_t ptr;
+ binder_uintptr_t cookie;
};

enum binder_driver_return_protocol {
@@ -240,11 +254,11 @@ enum binder_driver_return_protocol {
* stop threadpool thread
*/

- BR_DEAD_BINDER = _IOR('r', 15, void *),
+ BR_DEAD_BINDER = _IOR('r', 15, binder_uintptr_t),
/*
* void *: cookie
*/
- BR_CLEAR_DEATH_NOTIFICATION_DONE = _IOR('r', 16, void *),
+ BR_CLEAR_DEATH_NOTIFICATION_DONE = _IOR('r', 16, binder_uintptr_t),
/*
* void *: cookie
*/
@@ -270,7 +284,7 @@ enum binder_driver_command_protocol {
* Else you have acquired a primary reference on the object.
*/

- BC_FREE_BUFFER = _IOW('c', 3, void *),
+ BC_FREE_BUFFER = _IOW('c', 3, binder_uintptr_t),
/*
* void *: ptr to transaction data received on a read
*/
@@ -327,7 +341,7 @@ enum binder_driver_command_protocol {
* void *: cookie
*/

- BC_DEAD_BINDER_DONE = _IOW('c', 16, void *),
+ BC_DEAD_BINDER_DONE = _IOW('c', 16, binder_uintptr_t),
/*
* void *: cookie
*/
--
1.8.3.2

2014-02-17 21:59:16

by John Stultz

[permalink] [raw]
Subject: [PATCH 14/14] staging: binder: Improve Kconfig entry for ANDROID_BINDER_IPC_32BIT

Add a more clear explanation of the option in the prompt, and
make the config depend on ANDROID_BINDER_IPC being selected.

Also sets the default to y, which matches AOSP.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/Kconfig | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
index 78e82f5..0dc8ed7 100644
--- a/drivers/staging/android/Kconfig
+++ b/drivers/staging/android/Kconfig
@@ -21,8 +21,9 @@ config ANDROID_BINDER_IPC
between said processes.

config ANDROID_BINDER_IPC_32BIT
- bool "Use old 32-bit binder api"
- depends on !64BIT
+ bool "Use old (Android 4.4 and earlier) 32-bit binder API"
+ depends on !64BIT && ANDROID_BINDER_IPC
+ default y
---help---
The Binder API has been changed to support both 32 and 64bit
applications in a mixed environment.
--
1.8.3.2

2014-02-17 21:59:54

by John Stultz

[permalink] [raw]
Subject: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

From: Serban Constantinescu <[email protected]>

This patch fixes the ABI for 64bit Android userspace.
BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
to be using struct binder_ptr_cookie, but they are using a 32bit handle
and a pointer.

On 32bit systems the payload size is the same as the size of struct
binder_ptr_cookie, however for 64bit systems this will differ. This
patch adds struct binder_handle_cookie that fixes this issue for 64bit
Android.

Since there are no 64bit users of this interface that we know of this
change should not affect any existing systems.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Serban Constantinescu <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Serban Constantinescu <[email protected]>
[jstultz: Minor commit tweaks, few 80+ col fixes for checkpatch]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/uapi/binder.h | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
index 2b1eb81..4071fcf 100644
--- a/drivers/staging/android/uapi/binder.h
+++ b/drivers/staging/android/uapi/binder.h
@@ -152,6 +152,11 @@ struct binder_ptr_cookie {
void *cookie;
};

+struct binder_handle_cookie {
+ __u32 handle;
+ void *cookie;
+} __attribute__((packed));
+
struct binder_pri_desc {
__s32 priority;
__u32 desc;
@@ -308,15 +313,17 @@ enum binder_driver_command_protocol {
* of looping threads it has available.
*/

- BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14, struct binder_ptr_cookie),
+ BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14,
+ struct binder_handle_cookie),
/*
- * void *: ptr to binder
+ * int: handle
* void *: cookie
*/

- BC_CLEAR_DEATH_NOTIFICATION = _IOW('c', 15, struct binder_ptr_cookie),
+ BC_CLEAR_DEATH_NOTIFICATION = _IOW('c', 15,
+ struct binder_handle_cookie),
/*
- * void *: ptr to binder
+ * int: handle
* void *: cookie
*/

--
1.8.3.2

2014-02-17 22:00:20

by John Stultz

[permalink] [raw]
Subject: [PATCH 11/14] staging: ion: Add private buffer flag to skip page pooling on free

From: Mitchel Humpherys <[email protected]>

Currently, when we free a buffer it might actually just go back into a
heap-specific page pool rather than going back to the system. This poses
a problem because sometimes (like when we're running a shrinker in low
memory conditions) we need to force the memory associated with the
buffer to truly be relinquished to the system rather than just going
back into a page pool.

There isn't a use case for this flag by Ion clients, so make it a
private flag. The main use case right now is to provide a mechanism for
the deferred free code to force stale buffers to bypass page pooling.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Mitchel Humpherys <[email protected]>
[jstultz: Minor commit subject tweak]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/ion/ion_heap.c | 19 ++++++++++--
drivers/staging/android/ion/ion_priv.h | 42 ++++++++++++++++++++++++++-
drivers/staging/android/ion/ion_system_heap.c | 4 +--
3 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
index 49ace13..bdc6a28 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -178,7 +178,8 @@ size_t ion_heap_freelist_size(struct ion_heap *heap)
return size;
}

-size_t ion_heap_freelist_drain(struct ion_heap *heap, size_t size)
+static size_t _ion_heap_freelist_drain(struct ion_heap *heap, size_t size,
+ bool skip_pools)
{
struct ion_buffer *buffer;
size_t total_drained = 0;
@@ -197,6 +198,8 @@ size_t ion_heap_freelist_drain(struct ion_heap *heap, size_t size)
list);
list_del(&buffer->list);
heap->free_list_size -= buffer->size;
+ if (skip_pools)
+ buffer->private_flags |= ION_PRIV_FLAG_SHRINKER_FREE;
total_drained += buffer->size;
spin_unlock(&heap->free_lock);
ion_buffer_destroy(buffer);
@@ -207,6 +210,16 @@ size_t ion_heap_freelist_drain(struct ion_heap *heap, size_t size)
return total_drained;
}

+size_t ion_heap_freelist_drain(struct ion_heap *heap, size_t size)
+{
+ return _ion_heap_freelist_drain(heap, size, false);
+}
+
+size_t ion_heap_freelist_shrink(struct ion_heap *heap, size_t size)
+{
+ return _ion_heap_freelist_drain(heap, size, true);
+}
+
static int ion_heap_deferred_free(void *data)
{
struct ion_heap *heap = data;
@@ -278,10 +291,10 @@ static unsigned long ion_heap_shrink_scan(struct shrinker *shrinker,

/*
* shrink the free list first, no point in zeroing the memory if we're
- * just going to reclaim it
+ * just going to reclaim it. Also, skip any possible page pooling.
*/
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
- freed = ion_heap_freelist_drain(heap, to_scan * PAGE_SIZE) /
+ freed = ion_heap_freelist_shrink(heap, to_scan * PAGE_SIZE) /
PAGE_SIZE;

to_scan -= freed;
diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index bcf9d19..1eba3f2 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -38,6 +38,7 @@ struct ion_buffer *ion_handle_buffer(struct ion_handle *handle);
* @dev: back pointer to the ion_device
* @heap: back pointer to the heap the buffer came from
* @flags: buffer specific flags
+ * @private_flags: internal buffer specific flags
* @size: size of the buffer
* @priv_virt: private data to the buffer representable as
* a void *
@@ -66,6 +67,7 @@ struct ion_buffer {
struct ion_device *dev;
struct ion_heap *heap;
unsigned long flags;
+ unsigned long private_flags;
size_t size;
union {
void *priv_virt;
@@ -98,7 +100,11 @@ void ion_buffer_destroy(struct ion_buffer *buffer);
* @map_user map memory to userspace
*
* allocate, phys, and map_user return 0 on success, -errno on error.
- * map_dma and map_kernel return pointer on success, ERR_PTR on error.
+ * map_dma and map_kernel return pointer on success, ERR_PTR on
+ * error. @free will be called with ION_PRIV_FLAG_SHRINKER_FREE set in
+ * the buffer's private_flags when called from a shrinker. In that
+ * case, the pages being free'd must be truly free'd back to the
+ * system, not put in a page pool or otherwise cached.
*/
struct ion_heap_ops {
int (*allocate)(struct ion_heap *heap,
@@ -123,6 +129,17 @@ struct ion_heap_ops {
#define ION_HEAP_FLAG_DEFER_FREE (1 << 0)

/**
+ * private flags - flags internal to ion
+ */
+/*
+ * Buffer is being freed from a shrinker function. Skip any possible
+ * heap-specific caching mechanism (e.g. page pools). Guarantees that
+ * any buffer storage that came from the system allocator will be
+ * returned to the system allocator.
+ */
+#define ION_PRIV_FLAG_SHRINKER_FREE (1 << 0)
+
+/**
* struct ion_heap - represents a heap in the system
* @node: rb node to put the heap on the device's tree of heaps
* @dev: back pointer to the ion_device
@@ -258,6 +275,29 @@ void ion_heap_freelist_add(struct ion_heap *heap, struct ion_buffer *buffer);
size_t ion_heap_freelist_drain(struct ion_heap *heap, size_t size);

/**
+ * ion_heap_freelist_shrink - drain the deferred free
+ * list, skipping any heap-specific
+ * pooling or caching mechanisms
+ *
+ * @heap: the heap
+ * @size: amount of memory to drain in bytes
+ *
+ * Drains the indicated amount of memory from the deferred freelist immediately.
+ * Returns the total amount freed. The total freed may be higher depending
+ * on the size of the items in the list, or lower if there is insufficient
+ * total memory on the freelist.
+ *
+ * Unlike with @ion_heap_freelist_drain, don't put any pages back into
+ * page pools or otherwise cache the pages. Everything must be
+ * genuinely free'd back to the system. If you're free'ing from a
+ * shrinker you probably want to use this. Note that this relies on
+ * the heap.ops.free callback honoring the ION_PRIV_FLAG_SHRINKER_FREE
+ * flag.
+ */
+size_t ion_heap_freelist_shrink(struct ion_heap *heap,
+ size_t size);
+
+/**
* ion_heap_freelist_size - returns the size of the freelist in bytes
* @heap: the heap
*/
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index f453d97..c923633 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -90,7 +90,7 @@ static void free_buffer_page(struct ion_system_heap *heap,
{
bool cached = ion_buffer_cached(buffer);

- if (!cached) {
+ if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
struct ion_page_pool *pool = heap->pools[order_to_index(order)];
ion_page_pool_free(pool, page);
} else {
@@ -209,7 +209,7 @@ static void ion_system_heap_free(struct ion_buffer *buffer)

/* uncached pages come from the page pools, zero them before returning
for security purposes (other allocations are zerod at alloc time */
- if (!cached)
+ if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
ion_heap_buffer_zero(buffer);

for_each_sg(table->sgl, sg, table->nents, i)
--
1.8.3.2

2014-02-17 21:59:07

by John Stultz

[permalink] [raw]
Subject: [PATCH 09/14] staging: ion: Make sure all clients are exposed in debugfs

From: Mitchel Humpherys <[email protected]>

Currently, if multiple Ion clients are created with the same name, only
the first one shows up in debugfs. Rectify this by adding a
monotonically-increasing serial number to the debug names of Ion
clients.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Mitchel Humpherys <[email protected]>
[jstultz: Minor commit subject tweaks]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/ion/ion.c | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index eac4bce..5776697 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -72,6 +72,8 @@ struct ion_device {
* @idr: an idr space for allocating handle ids
* @lock: lock protecting the tree of handles
* @name: used for debugging
+ * @display_name: used for debugging (unique version of @name)
+ * @display_serial: used for debugging (to make display_name unique)
* @task: used for debugging
*
* A client represents a list of buffers this client may access.
@@ -85,6 +87,8 @@ struct ion_client {
struct idr idr;
struct mutex lock;
const char *name;
+ char *display_name;
+ int display_serial;
struct task_struct *task;
pid_t pid;
struct dentry *debug_root;
@@ -711,6 +715,21 @@ static const struct file_operations debug_client_fops = {
.release = single_release,
};

+static int ion_get_client_serial(const struct rb_root *root,
+ const unsigned char *name)
+{
+ int serial = -1;
+ struct rb_node *node;
+ for (node = rb_first(root); node; node = rb_next(node)) {
+ struct ion_client *client = rb_entry(node, struct ion_client,
+ node);
+ if (strcmp(client->name, name))
+ continue;
+ serial = max(serial, client->display_serial);
+ }
+ return serial + 1;
+}
+
struct ion_client *ion_client_create(struct ion_device *dev,
const char *name)
{
@@ -721,6 +740,11 @@ struct ion_client *ion_client_create(struct ion_device *dev,
struct ion_client *entry;
pid_t pid;

+ if (!name) {
+ pr_err("%s: Name cannot be null\n", __func__);
+ return ERR_PTR(-EINVAL);
+ }
+
get_task_struct(current->group_leader);
task_lock(current->group_leader);
pid = task_pid_nr(current->group_leader);
@@ -749,6 +773,13 @@ struct ion_client *ion_client_create(struct ion_device *dev,
goto err_free_client;

down_write(&dev->lock);
+ client->display_serial = ion_get_client_serial(&dev->clients, name);
+ client->display_name = kasprintf(
+ GFP_KERNEL, "%s-%d", name, client->display_serial);
+ if (!client->display_name) {
+ up_write(&dev->lock);
+ goto err_free_client_name;
+ }
p = &dev->clients.rb_node;
while (*p) {
parent = *p;
@@ -762,20 +793,22 @@ struct ion_client *ion_client_create(struct ion_device *dev,
rb_link_node(&client->node, parent, p);
rb_insert_color(&client->node, &dev->clients);

- client->debug_root = debugfs_create_file(name, 0664,
+ client->debug_root = debugfs_create_file(client->display_name, 0664,
dev->clients_debug_root,
client, &debug_client_fops);
if (!client->debug_root) {
char buf[256], *path;
path = dentry_path(dev->clients_debug_root, buf, 256);
pr_err("Failed to create client debugfs at %s/%s\n",
- path, name);
+ path, client->display_name);
}

up_write(&dev->lock);

return client;

+err_free_client_name:
+ kfree(client->name);
err_free_client:
kfree(client);
err_put_task_struct:
@@ -806,6 +839,7 @@ void ion_client_destroy(struct ion_client *client)
debugfs_remove_recursive(client->debug_root);
up_write(&dev->lock);

+ kfree(client->display_name);
kfree(client->name);
kfree(client);
}
--
1.8.3.2

2014-02-17 22:01:11

by John Stultz

[permalink] [raw]
Subject: [PATCH 10/14] staging: ion: Move shrinker out of heaps

From: Colin Cross <[email protected]>

Every heap that uses deferred frees is going to need a shrinker
to shrink the freelist under memory pressure. Rather than
requiring each heap to implement a shrinker, automatically
register a shrinker if the deferred free flag is set.
The system heap also needs to shrink its page pools, so add
a shrink function to the heap ops that will be called after
shrinking the freelists.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Colin Cross <[email protected]>
[jstultz: Resolved big conflicts with the shrinker api change.
Also minor commit subject tweak.]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/ion/ion.c | 3 +
drivers/staging/android/ion/ion_heap.c | 50 +++++++++++++++++
drivers/staging/android/ion/ion_page_pool.c | 8 +--
drivers/staging/android/ion/ion_priv.h | 21 ++++---
drivers/staging/android/ion/ion_system_heap.c | 80 ++++++---------------------
5 files changed, 85 insertions(+), 77 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 5776697..0836717 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1502,6 +1502,9 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
ion_heap_init_deferred_free(heap);

+ if ((heap->flags & ION_HEAP_FLAG_DEFER_FREE) || heap->ops->shrink)
+ ion_heap_init_shrinker(heap);
+
heap->dev = dev;
down_write(&dev->lock);
/* use negative heap->id to reverse the priority -- when traversing
diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
index 305b75e..49ace13 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -252,6 +252,56 @@ int ion_heap_init_deferred_free(struct ion_heap *heap)
return 0;
}

+static unsigned long ion_heap_shrink_count(struct shrinker *shrinker,
+ struct shrink_control *sc)
+{
+ struct ion_heap *heap = container_of(shrinker, struct ion_heap,
+ shrinker);
+ int total = 0;
+
+ total = ion_heap_freelist_size(heap) / PAGE_SIZE;
+ if (heap->ops->shrink)
+ total += heap->ops->shrink(heap, sc->gfp_mask, 0);
+ return total;
+}
+
+static unsigned long ion_heap_shrink_scan(struct shrinker *shrinker,
+ struct shrink_control *sc)
+{
+ struct ion_heap *heap = container_of(shrinker, struct ion_heap,
+ shrinker);
+ int freed = 0;
+ int to_scan = sc->nr_to_scan;
+
+ if (to_scan == 0)
+ return 0;
+
+ /*
+ * shrink the free list first, no point in zeroing the memory if we're
+ * just going to reclaim it
+ */
+ if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
+ freed = ion_heap_freelist_drain(heap, to_scan * PAGE_SIZE) /
+ PAGE_SIZE;
+
+ to_scan -= freed;
+ if (to_scan <= 0)
+ return freed;
+
+ if (heap->ops->shrink)
+ freed += heap->ops->shrink(heap, sc->gfp_mask, to_scan);
+ return freed;
+}
+
+void ion_heap_init_shrinker(struct ion_heap *heap)
+{
+ heap->shrinker.count_objects = ion_heap_shrink_count;
+ heap->shrinker.scan_objects = ion_heap_shrink_scan;
+ heap->shrinker.seeks = DEFAULT_SEEKS;
+ heap->shrinker.batch = 0;
+ register_shrinker(&heap->shrinker);
+}
+
struct ion_heap *ion_heap_create(struct ion_platform_heap *heap_data)
{
struct ion_heap *heap = NULL;
diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index fa693c2..ecb5fc3 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -130,8 +130,7 @@ static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
int nr_to_scan)
{
- int nr_freed = 0;
- int i;
+ int freed;
bool high;

high = !!(gfp_mask & __GFP_HIGHMEM);
@@ -139,7 +138,7 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
if (nr_to_scan == 0)
return ion_page_pool_total(pool, high);

- for (i = 0; i < nr_to_scan; i++) {
+ for (freed = 0; freed < nr_to_scan; freed++) {
struct page *page;

mutex_lock(&pool->mutex);
@@ -153,10 +152,9 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
}
mutex_unlock(&pool->mutex);
ion_page_pool_free_pages(pool, page);
- nr_freed += (1 << pool->order);
}

- return nr_freed;
+ return freed;
}

struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order)
diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index 0942a7f..bcf9d19 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -114,6 +114,7 @@ struct ion_heap_ops {
void (*unmap_kernel)(struct ion_heap *heap, struct ion_buffer *buffer);
int (*map_user)(struct ion_heap *mapper, struct ion_buffer *buffer,
struct vm_area_struct *vma);
+ int (*shrink)(struct ion_heap *heap, gfp_t gfp_mask, int nr_to_scan);
};

/**
@@ -132,10 +133,7 @@ struct ion_heap_ops {
* allocating. These are specified by platform data and
* MUST be unique
* @name: used for debugging
- * @shrinker: a shrinker for the heap, if the heap caches system
- * memory, it must define a shrinker to return it on low
- * memory conditions, this includes system memory cached
- * in the deferred free lists for heaps that support it
+ * @shrinker: a shrinker for the heap
* @free_list: free list head if deferred free is used
* @free_list_size size of the deferred free list in bytes
* @lock: protects the free list
@@ -219,6 +217,16 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer);
int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);

/**
+ * ion_heap_init_shrinker
+ * @heap: the heap
+ *
+ * If a heap sets the ION_HEAP_FLAG_DEFER_FREE flag or defines the shrink op
+ * this function will be called to setup a shrinker to shrink the freelists
+ * and call the heap's shrink op.
+ */
+void ion_heap_init_shrinker(struct ion_heap *heap);
+
+/**
* ion_heap_init_deferred_free -- initialize deferred free functionality
* @heap: the heap
*
@@ -305,13 +313,8 @@ void ion_carveout_free(struct ion_heap *heap, ion_phys_addr_t addr,
* @low_count: number of lowmem items in the pool
* @high_items: list of highmem items
* @low_items: list of lowmem items
- * @shrinker: a shrinker for the items
* @mutex: lock protecting this struct and especially the count
* item list
- * @alloc: function to be used to allocate pageory when the pool
- * is empty
- * @free: function to be used to free pageory back to the system
- * when the shrinker fires
* @gfp_mask: gfp_mask to use from alloc
* @order: order of pages in the pool
* @list: plist node for list of pools
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index 9849f39..f453d97 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -231,75 +231,34 @@ static void ion_system_heap_unmap_dma(struct ion_heap *heap,
return;
}

-static struct ion_heap_ops system_heap_ops = {
- .allocate = ion_system_heap_allocate,
- .free = ion_system_heap_free,
- .map_dma = ion_system_heap_map_dma,
- .unmap_dma = ion_system_heap_unmap_dma,
- .map_kernel = ion_heap_map_kernel,
- .unmap_kernel = ion_heap_unmap_kernel,
- .map_user = ion_heap_map_user,
-};
-
-static unsigned long ion_system_heap_shrink_count(struct shrinker *shrinker,
- struct shrink_control *sc)
+static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask,
+ int nr_to_scan)
{
- struct ion_heap *heap = container_of(shrinker, struct ion_heap,
- shrinker);
- struct ion_system_heap *sys_heap = container_of(heap,
- struct ion_system_heap,
- heap);
+ struct ion_system_heap *sys_heap;
int nr_total = 0;
int i;

- /* total number of items is whatever the page pools are holding
- plus whatever's in the freelist */
- for (i = 0; i < num_orders; i++) {
- struct ion_page_pool *pool = sys_heap->pools[i];
- nr_total += ion_page_pool_shrink(pool, sc->gfp_mask, 0);
- }
- nr_total += ion_heap_freelist_size(heap) / PAGE_SIZE;
- return nr_total;
-
-}
-
-static unsigned long ion_system_heap_shrink_scan(struct shrinker *shrinker,
- struct shrink_control *sc)
-{
-
- struct ion_heap *heap = container_of(shrinker, struct ion_heap,
- shrinker);
- struct ion_system_heap *sys_heap = container_of(heap,
- struct ion_system_heap,
- heap);
- int nr_freed = 0;
- int i;
-
- if (sc->nr_to_scan == 0)
- goto end;
-
- /* shrink the free list first, no point in zeroing the memory if
- we're just going to reclaim it */
- nr_freed += ion_heap_freelist_drain(heap, sc->nr_to_scan * PAGE_SIZE) /
- PAGE_SIZE;
-
- if (nr_freed >= sc->nr_to_scan)
- goto end;
+ sys_heap = container_of(heap, struct ion_system_heap, heap);

for (i = 0; i < num_orders; i++) {
struct ion_page_pool *pool = sys_heap->pools[i];
-
- nr_freed += ion_page_pool_shrink(pool, sc->gfp_mask,
- sc->nr_to_scan);
- if (nr_freed >= sc->nr_to_scan)
- break;
+ nr_total += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
}

-end:
- return nr_freed;
-
+ return nr_total;
}

+static struct ion_heap_ops system_heap_ops = {
+ .allocate = ion_system_heap_allocate,
+ .free = ion_system_heap_free,
+ .map_dma = ion_system_heap_map_dma,
+ .unmap_dma = ion_system_heap_unmap_dma,
+ .map_kernel = ion_heap_map_kernel,
+ .unmap_kernel = ion_heap_unmap_kernel,
+ .map_user = ion_heap_map_user,
+ .shrink = ion_system_heap_shrink,
+};
+
static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file *s,
void *unused)
{
@@ -347,11 +306,6 @@ struct ion_heap *ion_system_heap_create(struct ion_platform_heap *unused)
heap->pools[i] = pool;
}

- heap->heap.shrinker.scan_objects = ion_system_heap_shrink_scan;
- heap->heap.shrinker.count_objects = ion_system_heap_shrink_count;
- heap->heap.shrinker.seeks = DEFAULT_SEEKS;
- heap->heap.shrinker.batch = 0;
- register_shrinker(&heap->heap.shrinker);
heap->heap.debug_show = ion_system_heap_debug_show;
return &heap->heap;
err_create_pool:
--
1.8.3.2

2014-02-17 21:59:02

by John Stultz

[permalink] [raw]
Subject: [PATCH 06/14] staging: ion: Create separate heap and client debugfs directories

From: Mitchel Humpherys <[email protected]>

It can be slightly annoying to figure out which files under the ion
debugfs directory are heap debug files and which ones are client debug
files. Create separate subdirectories under ion to hold the different
types of debug files.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Mitchel Humpherys <[email protected]>
[jstultz: Minor commit subject tweaks]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/ion/ion.c | 57 +++++++++++++++++++++++++++++++++------
1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 14cb6c6..8f6bdb7 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -60,6 +60,8 @@ struct ion_device {
unsigned long arg);
struct rb_root clients;
struct dentry *debug_root;
+ struct dentry *heaps_debug_root;
+ struct dentry *clients_debug_root;
};

/**
@@ -764,8 +766,15 @@ struct ion_client *ion_client_create(struct ion_device *dev,

snprintf(debug_name, 64, "%u", client->pid);
client->debug_root = debugfs_create_file(debug_name, 0664,
- dev->debug_root, client,
- &debug_client_fops);
+ dev->clients_debug_root,
+ client, &debug_client_fops);
+ if (!client->debug_root) {
+ char buf[256], *path;
+ path = dentry_path(dev->clients_debug_root, buf, 256);
+ pr_err("Failed to create client debugfs at %s/%s\n",
+ path, debug_name);
+ }
+
up_write(&dev->lock);

return client;
@@ -1442,6 +1451,8 @@ DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,

void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
{
+ struct dentry *debug_file;
+
if (!heap->ops->allocate || !heap->ops->free || !heap->ops->map_dma ||
!heap->ops->unmap_dma)
pr_err("%s: can not add heap with invalid ops struct.\n",
@@ -1456,15 +1467,31 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
the list later attempt higher id numbers first */
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- debugfs_create_file(heap->name, 0664, dev->debug_root, heap,
- &debug_heap_fops);
+ debug_file = debugfs_create_file(heap->name, 0664,
+ dev->heaps_debug_root, heap,
+ &debug_heap_fops);
+
+ if (!debug_file) {
+ char buf[256], *path;
+ path = dentry_path(dev->heaps_debug_root, buf, 256);
+ pr_err("Failed to create heap debugfs at %s/%s\n",
+ path, heap->name);
+ }
+
#ifdef DEBUG_HEAP_SHRINKER
if (heap->shrinker.shrink) {
char debug_name[64];

snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root, heap,
- &debug_shrink_fops);
+ debug_file = debugfs_create_file(
+ debug_name, 0644, dev->heaps_debug_root, heap,
+ &debug_shrink_fops);
+ if (!debug_file) {
+ char buf[256], *path;
+ path = dentry_path(dev->heaps_debug_root, buf, 256);
+ pr_err("Failed to create heap shrinker debugfs at %s/%s\n",
+ path, debug_name);
+ }
}
#endif
up_write(&dev->lock);
@@ -1493,8 +1520,21 @@ struct ion_device *ion_device_create(long (*custom_ioctl)
}

idev->debug_root = debugfs_create_dir("ion", NULL);
- if (!idev->debug_root)
- pr_err("ion: failed to create debug files.\n");
+ if (!idev->debug_root) {
+ pr_err("ion: failed to create debugfs root directory.\n");
+ goto debugfs_done;
+ }
+ idev->heaps_debug_root = debugfs_create_dir("heaps", idev->debug_root);
+ if (!idev->heaps_debug_root) {
+ pr_err("ion: failed to create debugfs heaps directory.\n");
+ goto debugfs_done;
+ }
+ idev->clients_debug_root = debugfs_create_dir("clients",
+ idev->debug_root);
+ if (!idev->clients_debug_root)
+ pr_err("ion: failed to create debugfs clients directory.\n");
+
+debugfs_done:

idev->custom_ioctl = custom_ioctl;
idev->buffers = RB_ROOT;
@@ -1508,6 +1548,7 @@ struct ion_device *ion_device_create(long (*custom_ioctl)
void ion_device_destroy(struct ion_device *dev)
{
misc_deregister(&dev->dev);
+ debugfs_remove_recursive(dev->debug_root);
/* XXX need to free the heaps and clients ? */
kfree(dev);
}
--
1.8.3.2

2014-02-17 22:01:37

by John Stultz

[permalink] [raw]
Subject: [PATCH 07/14] staging: ion: Fix debugfs handling of multiple kernel clients

From: Laura Abbott <[email protected]>

Currently, Ion registers all debugfs entries for clients
via pid. If there are multiple kernel clients, this means
the debugfs entry only gets created for the first one. Fix
this by creating debugfs entries by name always. When
creating user clients, specify the name via the pid.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Laura Abbott <[email protected]>
Signed-off-by: Mitchel Humpherys <[email protected]>
[jstultz: Minor commit subject tweaks]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/ion/ion.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 8f6bdb7..4869420 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -719,7 +719,6 @@ struct ion_client *ion_client_create(struct ion_device *dev,
struct rb_node **p;
struct rb_node *parent = NULL;
struct ion_client *entry;
- char debug_name[64];
pid_t pid;

get_task_struct(current->group_leader);
@@ -764,15 +763,14 @@ struct ion_client *ion_client_create(struct ion_device *dev,
rb_link_node(&client->node, parent, p);
rb_insert_color(&client->node, &dev->clients);

- snprintf(debug_name, 64, "%u", client->pid);
- client->debug_root = debugfs_create_file(debug_name, 0664,
+ client->debug_root = debugfs_create_file(name, 0664,
dev->clients_debug_root,
client, &debug_client_fops);
if (!client->debug_root) {
char buf[256], *path;
path = dentry_path(dev->clients_debug_root, buf, 256);
pr_err("Failed to create client debugfs at %s/%s\n",
- path, debug_name);
+ path, name);
}

up_write(&dev->lock);
@@ -1301,9 +1299,11 @@ static int ion_open(struct inode *inode, struct file *file)
struct miscdevice *miscdev = file->private_data;
struct ion_device *dev = container_of(miscdev, struct ion_device, dev);
struct ion_client *client;
+ char debug_name[64];

pr_debug("%s: %d\n", __func__, __LINE__);
- client = ion_client_create(dev, "user");
+ snprintf(debug_name, 64, "%u", task_pid_nr(current->group_leader));
+ client = ion_client_create(dev, debug_name);
if (IS_ERR(client))
return PTR_ERR(client);
file->private_data = client;
--
1.8.3.2

2014-02-17 21:58:59

by John Stultz

[permalink] [raw]
Subject: [PATCH 01/14] staging: binder: Fix death notifications

From: Arve Hjønnevåg <[email protected]>

The change (008fa749e0fe5b2fffd20b7fe4891bb80d072c6a) that moved the
node release code to a separate function broke death notifications in
some cases. When it encountered a reference without a death
notification request, it would skip looking at the remaining
references, and therefore fail to send death notifications for them.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Arve Hjønnevåg <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/binder.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index eaec1da..1432d95 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -2904,7 +2904,7 @@ static int binder_node_release(struct binder_node *node, int refs)
refs++;

if (!ref->death)
- goto out;
+ continue;

death++;

@@ -2917,7 +2917,6 @@ static int binder_node_release(struct binder_node *node, int refs)
BUG();
}

-out:
binder_debug(BINDER_DEBUG_DEAD_BINDER,
"node %d now dead, refs %d, death %d\n",
node->debug_id, refs, death);
--
1.8.3.2

2014-02-17 22:02:35

by John Stultz

[permalink] [raw]
Subject: [PATCH 05/14] staging: android: Split uapi out of binder.h

From: Colin Cross <[email protected]>

Move the userspace interface of binder.h to
drivers/staging/android/uapi/binder.h.

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Colin Cross <[email protected]>
[jstultz: Worked out the collisions from some of the type changes
made upstream. Also minor commit subject tweak]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/binder.h | 306 +------------------------------
drivers/staging/android/uapi/binder.h | 330 ++++++++++++++++++++++++++++++++++
2 files changed, 331 insertions(+), 305 deletions(-)
create mode 100644 drivers/staging/android/uapi/binder.h

diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index c4c1ed6..d4101a6 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/binder.h
@@ -20,311 +20,7 @@
#ifndef _LINUX_BINDER_H
#define _LINUX_BINDER_H

-#include <linux/ioctl.h>
-
-#define B_PACK_CHARS(c1, c2, c3, c4) \
- ((((c1)<<24)) | (((c2)<<16)) | (((c3)<<8)) | (c4))
-#define B_TYPE_LARGE 0x85
-
-enum {
- BINDER_TYPE_BINDER = B_PACK_CHARS('s', 'b', '*', B_TYPE_LARGE),
- BINDER_TYPE_WEAK_BINDER = B_PACK_CHARS('w', 'b', '*', B_TYPE_LARGE),
- BINDER_TYPE_HANDLE = B_PACK_CHARS('s', 'h', '*', B_TYPE_LARGE),
- BINDER_TYPE_WEAK_HANDLE = B_PACK_CHARS('w', 'h', '*', B_TYPE_LARGE),
- BINDER_TYPE_FD = B_PACK_CHARS('f', 'd', '*', B_TYPE_LARGE),
-};
-
-enum {
- FLAT_BINDER_FLAG_PRIORITY_MASK = 0xff,
- FLAT_BINDER_FLAG_ACCEPTS_FDS = 0x100,
-};
-
-/*
- * This is the flattened representation of a Binder object for transfer
- * between processes. The 'offsets' supplied as part of a binder transaction
- * contains offsets into the data where these structures occur. The Binder
- * driver takes care of re-writing the structure type and data as it moves
- * between processes.
- */
-struct flat_binder_object {
- /* 8 bytes for large_flat_header. */
- __u32 type;
- __u32 flags;
-
- /* 8 bytes of data. */
- union {
- void __user *binder; /* local object */
- __u32 handle; /* remote object */
- };
-
- /* extra data associated with local object */
- void __user *cookie;
-};
-
-/*
- * On 64-bit platforms where user code may run in 32-bits the driver must
- * translate the buffer (and local binder) addresses appropriately.
- */
-
-struct binder_write_read {
- size_t write_size; /* bytes to write */
- size_t write_consumed; /* bytes consumed by driver */
- unsigned long write_buffer;
- size_t read_size; /* bytes to read */
- size_t read_consumed; /* bytes consumed by driver */
- unsigned long read_buffer;
-};
-
-/* Use with BINDER_VERSION, driver fills in fields. */
-struct binder_version {
- /* driver protocol version -- increment with incompatible change */
- __s32 protocol_version;
-};
-
-/* This is the current protocol version. */
-#define BINDER_CURRENT_PROTOCOL_VERSION 7
-
-#define BINDER_WRITE_READ _IOWR('b', 1, struct binder_write_read)
-#define BINDER_SET_IDLE_TIMEOUT _IOW('b', 3, __s64)
-#define BINDER_SET_MAX_THREADS _IOW('b', 5, __u32)
-#define BINDER_SET_IDLE_PRIORITY _IOW('b', 6, __s32)
-#define BINDER_SET_CONTEXT_MGR _IOW('b', 7, __s32)
-#define BINDER_THREAD_EXIT _IOW('b', 8, __s32)
-#define BINDER_VERSION _IOWR('b', 9, struct binder_version)
-
-/*
- * NOTE: Two special error codes you should check for when calling
- * in to the driver are:
- *
- * EINTR -- The operation has been interupted. This should be
- * handled by retrying the ioctl() until a different error code
- * is returned.
- *
- * ECONNREFUSED -- The driver is no longer accepting operations
- * from your process. That is, the process is being destroyed.
- * You should handle this by exiting from your process. Note
- * that once this error code is returned, all further calls to
- * the driver from any thread will return this same code.
- */
-
-enum transaction_flags {
- TF_ONE_WAY = 0x01, /* this is a one-way call: async, no return */
- TF_ROOT_OBJECT = 0x04, /* contents are the component's root object */
- TF_STATUS_CODE = 0x08, /* contents are a 32-bit status code */
- TF_ACCEPT_FDS = 0x10, /* allow replies with file descriptors */
-};
-
-struct binder_transaction_data {
- /* The first two are only used for bcTRANSACTION and brTRANSACTION,
- * identifying the target and contents of the transaction.
- */
- union {
- __u32 handle; /* target descriptor of command transaction */
- void *ptr; /* target descriptor of return transaction */
- } target;
- void *cookie; /* target object cookie */
- __u32 code; /* transaction command */
-
- /* General information about the transaction. */
- __u32 flags;
- pid_t sender_pid;
- uid_t sender_euid;
- size_t data_size; /* number of bytes of data */
- size_t offsets_size; /* number of bytes of offsets */
-
- /* If this transaction is inline, the data immediately
- * follows here; otherwise, it ends with a pointer to
- * the data buffer.
- */
- union {
- struct {
- /* transaction data */
- const void __user *buffer;
- /* offsets from buffer to flat_binder_object structs */
- const void __user *offsets;
- } ptr;
- __u8 buf[8];
- } data;
-};
-
-struct binder_ptr_cookie {
- void *ptr;
- void *cookie;
-};
-
-struct binder_pri_desc {
- __s32 priority;
- __u32 desc;
-};
-
-struct binder_pri_ptr_cookie {
- __s32 priority;
- void *ptr;
- void *cookie;
-};
-
-enum binder_driver_return_protocol {
- BR_ERROR = _IOR('r', 0, __s32),
- /*
- * int: error code
- */
-
- BR_OK = _IO('r', 1),
- /* No parameters! */
-
- BR_TRANSACTION = _IOR('r', 2, struct binder_transaction_data),
- BR_REPLY = _IOR('r', 3, struct binder_transaction_data),
- /*
- * binder_transaction_data: the received command.
- */
-
- BR_ACQUIRE_RESULT = _IOR('r', 4, __s32),
- /*
- * not currently supported
- * int: 0 if the last bcATTEMPT_ACQUIRE was not successful.
- * Else the remote object has acquired a primary reference.
- */
-
- BR_DEAD_REPLY = _IO('r', 5),
- /*
- * The target of the last transaction (either a bcTRANSACTION or
- * a bcATTEMPT_ACQUIRE) is no longer with us. No parameters.
- */
-
- BR_TRANSACTION_COMPLETE = _IO('r', 6),
- /*
- * No parameters... always refers to the last transaction requested
- * (including replies). Note that this will be sent even for
- * asynchronous transactions.
- */
-
- BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie),
- BR_ACQUIRE = _IOR('r', 8, struct binder_ptr_cookie),
- BR_RELEASE = _IOR('r', 9, struct binder_ptr_cookie),
- BR_DECREFS = _IOR('r', 10, struct binder_ptr_cookie),
- /*
- * void *: ptr to binder
- * void *: cookie for binder
- */
-
- BR_ATTEMPT_ACQUIRE = _IOR('r', 11, struct binder_pri_ptr_cookie),
- /*
- * not currently supported
- * int: priority
- * void *: ptr to binder
- * void *: cookie for binder
- */
-
- BR_NOOP = _IO('r', 12),
- /*
- * No parameters. Do nothing and examine the next command. It exists
- * primarily so that we can replace it with a BR_SPAWN_LOOPER command.
- */
-
- BR_SPAWN_LOOPER = _IO('r', 13),
- /*
- * No parameters. The driver has determined that a process has no
- * threads waiting to service incoming transactions. When a process
- * receives this command, it must spawn a new service thread and
- * register it via bcENTER_LOOPER.
- */
-
- BR_FINISHED = _IO('r', 14),
- /*
- * not currently supported
- * stop threadpool thread
- */
-
- BR_DEAD_BINDER = _IOR('r', 15, void *),
- /*
- * void *: cookie
- */
- BR_CLEAR_DEATH_NOTIFICATION_DONE = _IOR('r', 16, void *),
- /*
- * void *: cookie
- */
-
- BR_FAILED_REPLY = _IO('r', 17),
- /*
- * The the last transaction (either a bcTRANSACTION or
- * a bcATTEMPT_ACQUIRE) failed (e.g. out of memory). No parameters.
- */
-};
-
-enum binder_driver_command_protocol {
- BC_TRANSACTION = _IOW('c', 0, struct binder_transaction_data),
- BC_REPLY = _IOW('c', 1, struct binder_transaction_data),
- /*
- * binder_transaction_data: the sent command.
- */
-
- BC_ACQUIRE_RESULT = _IOW('c', 2, __s32),
- /*
- * not currently supported
- * int: 0 if the last BR_ATTEMPT_ACQUIRE was not successful.
- * Else you have acquired a primary reference on the object.
- */
-
- BC_FREE_BUFFER = _IOW('c', 3, void *),
- /*
- * void *: ptr to transaction data received on a read
- */
-
- BC_INCREFS = _IOW('c', 4, __u32),
- BC_ACQUIRE = _IOW('c', 5, __u32),
- BC_RELEASE = _IOW('c', 6, __u32),
- BC_DECREFS = _IOW('c', 7, __u32),
- /*
- * int: descriptor
- */
-
- BC_INCREFS_DONE = _IOW('c', 8, struct binder_ptr_cookie),
- BC_ACQUIRE_DONE = _IOW('c', 9, struct binder_ptr_cookie),
- /*
- * void *: ptr to binder
- * void *: cookie for binder
- */
-
- BC_ATTEMPT_ACQUIRE = _IOW('c', 10, struct binder_pri_desc),
- /*
- * not currently supported
- * int: priority
- * int: descriptor
- */
-
- BC_REGISTER_LOOPER = _IO('c', 11),
- /*
- * No parameters.
- * Register a spawned looper thread with the device.
- */
-
- BC_ENTER_LOOPER = _IO('c', 12),
- BC_EXIT_LOOPER = _IO('c', 13),
- /*
- * No parameters.
- * These two commands are sent as an application-level thread
- * enters and exits the binder loop, respectively. They are
- * used so the binder can have an accurate count of the number
- * of looping threads it has available.
- */
-
- BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14, struct binder_ptr_cookie),
- /*
- * void *: ptr to binder
- * void *: cookie
- */
-
- BC_CLEAR_DEATH_NOTIFICATION = _IOW('c', 15, struct binder_ptr_cookie),
- /*
- * void *: ptr to binder
- * void *: cookie
- */
-
- BC_DEAD_BINDER_DONE = _IOW('c', 16, void *),
- /*
- * void *: cookie
- */
-};
+#include "uapi/binder.h"

#endif /* _LINUX_BINDER_H */

diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
new file mode 100644
index 0000000..2b1eb81
--- /dev/null
+++ b/drivers/staging/android/uapi/binder.h
@@ -0,0 +1,330 @@
+/*
+ * Copyright (C) 2008 Google, Inc.
+ *
+ * Based on, but no longer compatible with, the original
+ * OpenBinder.org binder driver interface, which is:
+ *
+ * Copyright (c) 2005 Palmsource, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _UAPI_LINUX_BINDER_H
+#define _UAPI_LINUX_BINDER_H
+
+#include <linux/ioctl.h>
+
+#define B_PACK_CHARS(c1, c2, c3, c4) \
+ ((((c1)<<24)) | (((c2)<<16)) | (((c3)<<8)) | (c4))
+#define B_TYPE_LARGE 0x85
+
+enum {
+ BINDER_TYPE_BINDER = B_PACK_CHARS('s', 'b', '*', B_TYPE_LARGE),
+ BINDER_TYPE_WEAK_BINDER = B_PACK_CHARS('w', 'b', '*', B_TYPE_LARGE),
+ BINDER_TYPE_HANDLE = B_PACK_CHARS('s', 'h', '*', B_TYPE_LARGE),
+ BINDER_TYPE_WEAK_HANDLE = B_PACK_CHARS('w', 'h', '*', B_TYPE_LARGE),
+ BINDER_TYPE_FD = B_PACK_CHARS('f', 'd', '*', B_TYPE_LARGE),
+};
+
+enum {
+ FLAT_BINDER_FLAG_PRIORITY_MASK = 0xff,
+ FLAT_BINDER_FLAG_ACCEPTS_FDS = 0x100,
+};
+
+/*
+ * This is the flattened representation of a Binder object for transfer
+ * between processes. The 'offsets' supplied as part of a binder transaction
+ * contains offsets into the data where these structures occur. The Binder
+ * driver takes care of re-writing the structure type and data as it moves
+ * between processes.
+ */
+struct flat_binder_object {
+ /* 8 bytes for large_flat_header. */
+ __u32 type;
+ __u32 flags;
+
+ /* 8 bytes of data. */
+ union {
+ void __user *binder; /* local object */
+ __u32 handle; /* remote object */
+ };
+
+ /* extra data associated with local object */
+ void __user *cookie;
+};
+
+/*
+ * On 64-bit platforms where user code may run in 32-bits the driver must
+ * translate the buffer (and local binder) addresses appropriately.
+ */
+
+struct binder_write_read {
+ size_t write_size; /* bytes to write */
+ size_t write_consumed; /* bytes consumed by driver */
+ unsigned long write_buffer;
+ size_t read_size; /* bytes to read */
+ size_t read_consumed; /* bytes consumed by driver */
+ unsigned long read_buffer;
+};
+
+/* Use with BINDER_VERSION, driver fills in fields. */
+struct binder_version {
+ /* driver protocol version -- increment with incompatible change */
+ __s32 protocol_version;
+};
+
+/* This is the current protocol version. */
+#define BINDER_CURRENT_PROTOCOL_VERSION 7
+
+#define BINDER_WRITE_READ _IOWR('b', 1, struct binder_write_read)
+#define BINDER_SET_IDLE_TIMEOUT _IOW('b', 3, __s64)
+#define BINDER_SET_MAX_THREADS _IOW('b', 5, __u32)
+#define BINDER_SET_IDLE_PRIORITY _IOW('b', 6, __s32)
+#define BINDER_SET_CONTEXT_MGR _IOW('b', 7, __s32)
+#define BINDER_THREAD_EXIT _IOW('b', 8, __s32)
+#define BINDER_VERSION _IOWR('b', 9, struct binder_version)
+
+/*
+ * NOTE: Two special error codes you should check for when calling
+ * in to the driver are:
+ *
+ * EINTR -- The operation has been interupted. This should be
+ * handled by retrying the ioctl() until a different error code
+ * is returned.
+ *
+ * ECONNREFUSED -- The driver is no longer accepting operations
+ * from your process. That is, the process is being destroyed.
+ * You should handle this by exiting from your process. Note
+ * that once this error code is returned, all further calls to
+ * the driver from any thread will return this same code.
+ */
+
+enum transaction_flags {
+ TF_ONE_WAY = 0x01, /* this is a one-way call: async, no return */
+ TF_ROOT_OBJECT = 0x04, /* contents are the component's root object */
+ TF_STATUS_CODE = 0x08, /* contents are a 32-bit status code */
+ TF_ACCEPT_FDS = 0x10, /* allow replies with file descriptors */
+};
+
+struct binder_transaction_data {
+ /* The first two are only used for bcTRANSACTION and brTRANSACTION,
+ * identifying the target and contents of the transaction.
+ */
+ union {
+ __u32 handle; /* target descriptor of command transaction */
+ void *ptr; /* target descriptor of return transaction */
+ } target;
+ void *cookie; /* target object cookie */
+ __u32 code; /* transaction command */
+
+ /* General information about the transaction. */
+ __u32 flags;
+ pid_t sender_pid;
+ uid_t sender_euid;
+ size_t data_size; /* number of bytes of data */
+ size_t offsets_size; /* number of bytes of offsets */
+
+ /* If this transaction is inline, the data immediately
+ * follows here; otherwise, it ends with a pointer to
+ * the data buffer.
+ */
+ union {
+ struct {
+ /* transaction data */
+ const void __user *buffer;
+ /* offsets from buffer to flat_binder_object structs */
+ const void __user *offsets;
+ } ptr;
+ __u8 buf[8];
+ } data;
+};
+
+struct binder_ptr_cookie {
+ void *ptr;
+ void *cookie;
+};
+
+struct binder_pri_desc {
+ __s32 priority;
+ __u32 desc;
+};
+
+struct binder_pri_ptr_cookie {
+ __s32 priority;
+ void *ptr;
+ void *cookie;
+};
+
+enum binder_driver_return_protocol {
+ BR_ERROR = _IOR('r', 0, __s32),
+ /*
+ * int: error code
+ */
+
+ BR_OK = _IO('r', 1),
+ /* No parameters! */
+
+ BR_TRANSACTION = _IOR('r', 2, struct binder_transaction_data),
+ BR_REPLY = _IOR('r', 3, struct binder_transaction_data),
+ /*
+ * binder_transaction_data: the received command.
+ */
+
+ BR_ACQUIRE_RESULT = _IOR('r', 4, __s32),
+ /*
+ * not currently supported
+ * int: 0 if the last bcATTEMPT_ACQUIRE was not successful.
+ * Else the remote object has acquired a primary reference.
+ */
+
+ BR_DEAD_REPLY = _IO('r', 5),
+ /*
+ * The target of the last transaction (either a bcTRANSACTION or
+ * a bcATTEMPT_ACQUIRE) is no longer with us. No parameters.
+ */
+
+ BR_TRANSACTION_COMPLETE = _IO('r', 6),
+ /*
+ * No parameters... always refers to the last transaction requested
+ * (including replies). Note that this will be sent even for
+ * asynchronous transactions.
+ */
+
+ BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie),
+ BR_ACQUIRE = _IOR('r', 8, struct binder_ptr_cookie),
+ BR_RELEASE = _IOR('r', 9, struct binder_ptr_cookie),
+ BR_DECREFS = _IOR('r', 10, struct binder_ptr_cookie),
+ /*
+ * void *: ptr to binder
+ * void *: cookie for binder
+ */
+
+ BR_ATTEMPT_ACQUIRE = _IOR('r', 11, struct binder_pri_ptr_cookie),
+ /*
+ * not currently supported
+ * int: priority
+ * void *: ptr to binder
+ * void *: cookie for binder
+ */
+
+ BR_NOOP = _IO('r', 12),
+ /*
+ * No parameters. Do nothing and examine the next command. It exists
+ * primarily so that we can replace it with a BR_SPAWN_LOOPER command.
+ */
+
+ BR_SPAWN_LOOPER = _IO('r', 13),
+ /*
+ * No parameters. The driver has determined that a process has no
+ * threads waiting to service incoming transactions. When a process
+ * receives this command, it must spawn a new service thread and
+ * register it via bcENTER_LOOPER.
+ */
+
+ BR_FINISHED = _IO('r', 14),
+ /*
+ * not currently supported
+ * stop threadpool thread
+ */
+
+ BR_DEAD_BINDER = _IOR('r', 15, void *),
+ /*
+ * void *: cookie
+ */
+ BR_CLEAR_DEATH_NOTIFICATION_DONE = _IOR('r', 16, void *),
+ /*
+ * void *: cookie
+ */
+
+ BR_FAILED_REPLY = _IO('r', 17),
+ /*
+ * The the last transaction (either a bcTRANSACTION or
+ * a bcATTEMPT_ACQUIRE) failed (e.g. out of memory). No parameters.
+ */
+};
+
+enum binder_driver_command_protocol {
+ BC_TRANSACTION = _IOW('c', 0, struct binder_transaction_data),
+ BC_REPLY = _IOW('c', 1, struct binder_transaction_data),
+ /*
+ * binder_transaction_data: the sent command.
+ */
+
+ BC_ACQUIRE_RESULT = _IOW('c', 2, __s32),
+ /*
+ * not currently supported
+ * int: 0 if the last BR_ATTEMPT_ACQUIRE was not successful.
+ * Else you have acquired a primary reference on the object.
+ */
+
+ BC_FREE_BUFFER = _IOW('c', 3, void *),
+ /*
+ * void *: ptr to transaction data received on a read
+ */
+
+ BC_INCREFS = _IOW('c', 4, __u32),
+ BC_ACQUIRE = _IOW('c', 5, __u32),
+ BC_RELEASE = _IOW('c', 6, __u32),
+ BC_DECREFS = _IOW('c', 7, __u32),
+ /*
+ * int: descriptor
+ */
+
+ BC_INCREFS_DONE = _IOW('c', 8, struct binder_ptr_cookie),
+ BC_ACQUIRE_DONE = _IOW('c', 9, struct binder_ptr_cookie),
+ /*
+ * void *: ptr to binder
+ * void *: cookie for binder
+ */
+
+ BC_ATTEMPT_ACQUIRE = _IOW('c', 10, struct binder_pri_desc),
+ /*
+ * not currently supported
+ * int: priority
+ * int: descriptor
+ */
+
+ BC_REGISTER_LOOPER = _IO('c', 11),
+ /*
+ * No parameters.
+ * Register a spawned looper thread with the device.
+ */
+
+ BC_ENTER_LOOPER = _IO('c', 12),
+ BC_EXIT_LOOPER = _IO('c', 13),
+ /*
+ * No parameters.
+ * These two commands are sent as an application-level thread
+ * enters and exits the binder loop, respectively. They are
+ * used so the binder can have an accurate count of the number
+ * of looping threads it has available.
+ */
+
+ BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14, struct binder_ptr_cookie),
+ /*
+ * void *: ptr to binder
+ * void *: cookie
+ */
+
+ BC_CLEAR_DEATH_NOTIFICATION = _IOW('c', 15, struct binder_ptr_cookie),
+ /*
+ * void *: ptr to binder
+ * void *: cookie
+ */
+
+ BC_DEAD_BINDER_DONE = _IOW('c', 16, void *),
+ /*
+ * void *: cookie
+ */
+};
+
+#endif /* _UAPI_LINUX_BINDER_H */
+
--
1.8.3.2

2014-02-17 21:58:57

by John Stultz

[permalink] [raw]
Subject: [PATCH 02/14] staging: android: Split uapi out of android_alarm.h

From: Colin Cross <[email protected]>

Move the userspace interface of android_alarm.h to
drivers/staging/android/uapi/android_alarm.h

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Colin Cross <[email protected]>
[jstultz: minor commit msg tweak]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/android_alarm.h | 44 +-------------------
drivers/staging/android/uapi/android_alarm.h | 62 ++++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 42 deletions(-)
create mode 100644 drivers/staging/android/uapi/android_alarm.h

diff --git a/drivers/staging/android/android_alarm.h b/drivers/staging/android/android_alarm.h
index 4fd32f3..495b20c 100644
--- a/drivers/staging/android/android_alarm.h
+++ b/drivers/staging/android/android_alarm.h
@@ -16,50 +16,10 @@
#ifndef _LINUX_ANDROID_ALARM_H
#define _LINUX_ANDROID_ALARM_H

-#include <linux/ioctl.h>
-#include <linux/time.h>
#include <linux/compat.h>
+#include <linux/ioctl.h>

-enum android_alarm_type {
- /* return code bit numbers or set alarm arg */
- ANDROID_ALARM_RTC_WAKEUP,
- ANDROID_ALARM_RTC,
- ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP,
- ANDROID_ALARM_ELAPSED_REALTIME,
- ANDROID_ALARM_SYSTEMTIME,
-
- ANDROID_ALARM_TYPE_COUNT,
-
- /* return code bit numbers */
- /* ANDROID_ALARM_TIME_CHANGE = 16 */
-};
-
-enum android_alarm_return_flags {
- ANDROID_ALARM_RTC_WAKEUP_MASK = 1U << ANDROID_ALARM_RTC_WAKEUP,
- ANDROID_ALARM_RTC_MASK = 1U << ANDROID_ALARM_RTC,
- ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP_MASK =
- 1U << ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP,
- ANDROID_ALARM_ELAPSED_REALTIME_MASK =
- 1U << ANDROID_ALARM_ELAPSED_REALTIME,
- ANDROID_ALARM_SYSTEMTIME_MASK = 1U << ANDROID_ALARM_SYSTEMTIME,
- ANDROID_ALARM_TIME_CHANGE_MASK = 1U << 16
-};
-
-/* Disable alarm */
-#define ANDROID_ALARM_CLEAR(type) _IO('a', 0 | ((type) << 4))
-
-/* Ack last alarm and wait for next */
-#define ANDROID_ALARM_WAIT _IO('a', 1)
-
-#define ALARM_IOW(c, type, size) _IOW('a', (c) | ((type) << 4), size)
-/* Set alarm */
-#define ANDROID_ALARM_SET(type) ALARM_IOW(2, type, struct timespec)
-#define ANDROID_ALARM_SET_AND_WAIT(type) ALARM_IOW(3, type, struct timespec)
-#define ANDROID_ALARM_GET_TIME(type) ALARM_IOW(4, type, struct timespec)
-#define ANDROID_ALARM_SET_RTC _IOW('a', 5, struct timespec)
-#define ANDROID_ALARM_BASE_CMD(cmd) (cmd & ~(_IOC(0, 0, 0xf0, 0)))
-#define ANDROID_ALARM_IOCTL_TO_TYPE(cmd) (_IOC_NR(cmd) >> 4)
-
+#include "uapi/android_alarm.h"

#ifdef CONFIG_COMPAT
#define ANDROID_ALARM_SET_COMPAT(type) ALARM_IOW(2, type, \
diff --git a/drivers/staging/android/uapi/android_alarm.h b/drivers/staging/android/uapi/android_alarm.h
new file mode 100644
index 0000000..aa013f6
--- /dev/null
+++ b/drivers/staging/android/uapi/android_alarm.h
@@ -0,0 +1,62 @@
+/* drivers/staging/android/uapi/android_alarm.h
+ *
+ * Copyright (C) 2006-2007 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _UAPI_LINUX_ANDROID_ALARM_H
+#define _UAPI_LINUX_ANDROID_ALARM_H
+
+#include <linux/ioctl.h>
+#include <linux/time.h>
+
+enum android_alarm_type {
+ /* return code bit numbers or set alarm arg */
+ ANDROID_ALARM_RTC_WAKEUP,
+ ANDROID_ALARM_RTC,
+ ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP,
+ ANDROID_ALARM_ELAPSED_REALTIME,
+ ANDROID_ALARM_SYSTEMTIME,
+
+ ANDROID_ALARM_TYPE_COUNT,
+
+ /* return code bit numbers */
+ /* ANDROID_ALARM_TIME_CHANGE = 16 */
+};
+
+enum android_alarm_return_flags {
+ ANDROID_ALARM_RTC_WAKEUP_MASK = 1U << ANDROID_ALARM_RTC_WAKEUP,
+ ANDROID_ALARM_RTC_MASK = 1U << ANDROID_ALARM_RTC,
+ ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP_MASK =
+ 1U << ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP,
+ ANDROID_ALARM_ELAPSED_REALTIME_MASK =
+ 1U << ANDROID_ALARM_ELAPSED_REALTIME,
+ ANDROID_ALARM_SYSTEMTIME_MASK = 1U << ANDROID_ALARM_SYSTEMTIME,
+ ANDROID_ALARM_TIME_CHANGE_MASK = 1U << 16
+};
+
+/* Disable alarm */
+#define ANDROID_ALARM_CLEAR(type) _IO('a', 0 | ((type) << 4))
+
+/* Ack last alarm and wait for next */
+#define ANDROID_ALARM_WAIT _IO('a', 1)
+
+#define ALARM_IOW(c, type, size) _IOW('a', (c) | ((type) << 4), size)
+/* Set alarm */
+#define ANDROID_ALARM_SET(type) ALARM_IOW(2, type, struct timespec)
+#define ANDROID_ALARM_SET_AND_WAIT(type) ALARM_IOW(3, type, struct timespec)
+#define ANDROID_ALARM_GET_TIME(type) ALARM_IOW(4, type, struct timespec)
+#define ANDROID_ALARM_SET_RTC _IOW('a', 5, struct timespec)
+#define ANDROID_ALARM_BASE_CMD(cmd) (cmd & ~(_IOC(0, 0, 0xf0, 0)))
+#define ANDROID_ALARM_IOCTL_TO_TYPE(cmd) (_IOC_NR(cmd) >> 4)
+
+#endif
--
1.8.3.2

2014-02-17 22:03:25

by John Stultz

[permalink] [raw]
Subject: [PATCH 04/14] staging: android: split uapi out of sync.h and sw_sync.h

From: Colin Cross <[email protected]>

Move the userspace interfaces of sync.h and sw_sync.h to
drivers/staging/android/uapi/

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Colin Cross <[email protected]>
[jstultz: Fixed up some conflicts from upstream spelling fixes]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/sw_sync.h | 20 +------
drivers/staging/android/sync.h | 86 +-----------------------------
drivers/staging/android/uapi/sw_sync.h | 32 +++++++++++
drivers/staging/android/uapi/sync.h | 97 ++++++++++++++++++++++++++++++++++
4 files changed, 133 insertions(+), 102 deletions(-)
create mode 100644 drivers/staging/android/uapi/sw_sync.h
create mode 100644 drivers/staging/android/uapi/sync.h

diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h
index 5aaf71d..1a50669 100644
--- a/drivers/staging/android/sw_sync.h
+++ b/drivers/staging/android/sw_sync.h
@@ -18,10 +18,9 @@
#define _LINUX_SW_SYNC_H

#include <linux/types.h>
-
-#ifdef __KERNEL__
-
+#include <linux/kconfig.h>
#include "sync.h"
+#include "uapi/sw_sync.h"

struct sw_sync_timeline {
struct sync_timeline obj;
@@ -57,19 +56,4 @@ static inline struct sync_pt *sw_sync_pt_create(struct sw_sync_timeline *obj,
}
#endif /* IS_ENABLED(CONFIG_SW_SYNC) */

-#endif /* __KERNEL __ */
-
-struct sw_sync_create_fence_data {
- __u32 value;
- char name[32];
- __s32 fence; /* fd of new fence */
-};
-
-#define SW_SYNC_IOC_MAGIC 'W'
-
-#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\
- struct sw_sync_create_fence_data)
-#define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
-
-
#endif /* _LINUX_SW_SYNC_H */
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 6ee8d69..eaf57ccc 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -14,14 +14,14 @@
#define _LINUX_SYNC_H

#include <linux/types.h>
-#ifdef __KERNEL__
-
#include <linux/kref.h>
#include <linux/ktime.h>
#include <linux/list.h>
#include <linux/spinlock.h>
#include <linux/wait.h>

+#include "uapi/sync.h"
+
struct sync_timeline;
struct sync_pt;
struct sync_fence;
@@ -341,86 +341,4 @@ int sync_fence_cancel_async(struct sync_fence *fence,
*/
int sync_fence_wait(struct sync_fence *fence, long timeout);

-#endif /* __KERNEL__ */
-
-/**
- * struct sync_merge_data - data passed to merge ioctl
- * @fd2: file descriptor of second fence
- * @name: name of new fence
- * @fence: returns the fd of the new fence to userspace
- */
-struct sync_merge_data {
- __s32 fd2; /* fd of second fence */
- char name[32]; /* name of new fence */
- __s32 fence; /* fd on newly created fence */
-};
-
-/**
- * struct sync_pt_info - detailed sync_pt information
- * @len: length of sync_pt_info including any driver_data
- * @obj_name: name of parent sync_timeline
- * @driver_name: name of driver implementing the parent
- * @status: status of the sync_pt 0:active 1:signaled <0:error
- * @timestamp_ns: timestamp of status change in nanoseconds
- * @driver_data: any driver dependent data
- */
-struct sync_pt_info {
- __u32 len;
- char obj_name[32];
- char driver_name[32];
- __s32 status;
- __u64 timestamp_ns;
-
- __u8 driver_data[0];
-};
-
-/**
- * struct sync_fence_info_data - data returned from fence info ioctl
- * @len: ioctl caller writes the size of the buffer its passing in.
- * ioctl returns length of sync_fence_data returned to userspace
- * including pt_info.
- * @name: name of fence
- * @status: status of fence. 1: signaled 0:active <0:error
- * @pt_info: a sync_pt_info struct for every sync_pt in the fence
- */
-struct sync_fence_info_data {
- __u32 len;
- char name[32];
- __s32 status;
-
- __u8 pt_info[0];
-};
-
-#define SYNC_IOC_MAGIC '>'
-
-/**
- * DOC: SYNC_IOC_WAIT - wait for a fence to signal
- *
- * pass timeout in milliseconds. Waits indefinitely timeout < 0.
- */
-#define SYNC_IOC_WAIT _IOW(SYNC_IOC_MAGIC, 0, __s32)
-
-/**
- * DOC: SYNC_IOC_MERGE - merge two fences
- *
- * Takes a struct sync_merge_data. Creates a new fence containing copies of
- * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the
- * new fence's fd in sync_merge_data.fence
- */
-#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data)
-
-/**
- * DOC: SYNC_IOC_FENCE_INFO - get detailed information on a fence
- *
- * Takes a struct sync_fence_info_data with extra space allocated for pt_info.
- * Caller should write the size of the buffer into len. On return, len is
- * updated to reflect the total size of the sync_fence_info_data including
- * pt_info.
- *
- * pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence.
- * To iterate over the sync_pt_infos, use the sync_pt_info.len field.
- */
-#define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2,\
- struct sync_fence_info_data)
-
#endif /* _LINUX_SYNC_H */
diff --git a/drivers/staging/android/uapi/sw_sync.h b/drivers/staging/android/uapi/sw_sync.h
new file mode 100644
index 0000000..9b5d486
--- /dev/null
+++ b/drivers/staging/android/uapi/sw_sync.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2012 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _UAPI_LINUX_SW_SYNC_H
+#define _UAPI_LINUX_SW_SYNC_H
+
+#include <linux/types.h>
+
+struct sw_sync_create_fence_data {
+ __u32 value;
+ char name[32];
+ __s32 fence; /* fd of new fence */
+};
+
+#define SW_SYNC_IOC_MAGIC 'W'
+
+#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\
+ struct sw_sync_create_fence_data)
+#define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
+
+#endif /* _UAPI_LINUX_SW_SYNC_H */
diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h
new file mode 100644
index 0000000..e964c75
--- /dev/null
+++ b/drivers/staging/android/uapi/sync.h
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2012 Google, Inc.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _UAPI_LINUX_SYNC_H
+#define _UAPI_LINUX_SYNC_H
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+/**
+ * struct sync_merge_data - data passed to merge ioctl
+ * @fd2: file descriptor of second fence
+ * @name: name of new fence
+ * @fence: returns the fd of the new fence to userspace
+ */
+struct sync_merge_data {
+ __s32 fd2; /* fd of second fence */
+ char name[32]; /* name of new fence */
+ __s32 fence; /* fd on newly created fence */
+};
+
+/**
+ * struct sync_pt_info - detailed sync_pt information
+ * @len: length of sync_pt_info including any driver_data
+ * @obj_name: name of parent sync_timeline
+ * @driver_name: name of driver implementing the parent
+ * @status: status of the sync_pt 0:active 1:signaled <0:error
+ * @timestamp_ns: timestamp of status change in nanoseconds
+ * @driver_data: any driver dependent data
+ */
+struct sync_pt_info {
+ __u32 len;
+ char obj_name[32];
+ char driver_name[32];
+ __s32 status;
+ __u64 timestamp_ns;
+
+ __u8 driver_data[0];
+};
+
+/**
+ * struct sync_fence_info_data - data returned from fence info ioctl
+ * @len: ioctl caller writes the size of the buffer its passing in.
+ * ioctl returns length of sync_fence_data returned to userspace
+ * including pt_info.
+ * @name: name of fence
+ * @status: status of fence. 1: signaled 0:active <0:error
+ * @pt_info: a sync_pt_info struct for every sync_pt in the fence
+ */
+struct sync_fence_info_data {
+ __u32 len;
+ char name[32];
+ __s32 status;
+
+ __u8 pt_info[0];
+};
+
+#define SYNC_IOC_MAGIC '>'
+
+/**
+ * DOC: SYNC_IOC_WAIT - wait for a fence to signal
+ *
+ * pass timeout in milliseconds. Waits indefinitely timeout < 0.
+ */
+#define SYNC_IOC_WAIT _IOW(SYNC_IOC_MAGIC, 0, __s32)
+
+/**
+ * DOC: SYNC_IOC_MERGE - merge two fences
+ *
+ * Takes a struct sync_merge_data. Creates a new fence containing copies of
+ * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the
+ * new fence's fd in sync_merge_data.fence
+ */
+#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data)
+
+/**
+ * DOC: SYNC_IOC_FENCE_INFO - get detailed information on a fence
+ *
+ * Takes a struct sync_fence_info_data with extra space allocated for pt_info.
+ * Caller should write the size of the buffer into len. On return, len is
+ * updated to reflect the total size of the sync_fence_info_data including
+ * pt_info.
+ *
+ * pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence.
+ * To iterate over the sync_pt_infos, use the sync_pt_info.len field.
+ */
+#define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2,\
+ struct sync_fence_info_data)
+
+#endif /* _UAPI_LINUX_SYNC_H */
--
1.8.3.2

2014-02-17 22:03:44

by John Stultz

[permalink] [raw]
Subject: [PATCH 03/14] staging: android: Split uapi out of ashmem.h

From: Colin Cross <[email protected]>

Move the userspace interface of ashmem.h to
drivers/staging/android/uapi/ashmem.h

Cc: Greg KH <[email protected]>
Cc: Colin Cross <[email protected]>
Cc: Android Kernel Team <[email protected]>
Signed-off-by: Colin Cross <[email protected]>
[jstultz: Minor commit message tweak]
Signed-off-by: John Stultz <[email protected]>
---
drivers/staging/android/ashmem.h | 30 +---------------------
drivers/staging/android/uapi/ashmem.h | 47 +++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 29 deletions(-)
create mode 100644 drivers/staging/android/uapi/ashmem.h

diff --git a/drivers/staging/android/ashmem.h b/drivers/staging/android/ashmem.h
index 8dc0f0d..5abcfd7 100644
--- a/drivers/staging/android/ashmem.h
+++ b/drivers/staging/android/ashmem.h
@@ -16,35 +16,7 @@
#include <linux/ioctl.h>
#include <linux/compat.h>

-#define ASHMEM_NAME_LEN 256
-
-#define ASHMEM_NAME_DEF "dev/ashmem"
-
-/* Return values from ASHMEM_PIN: Was the mapping purged while unpinned? */
-#define ASHMEM_NOT_PURGED 0
-#define ASHMEM_WAS_PURGED 1
-
-/* Return values from ASHMEM_GET_PIN_STATUS: Is the mapping pinned? */
-#define ASHMEM_IS_UNPINNED 0
-#define ASHMEM_IS_PINNED 1
-
-struct ashmem_pin {
- __u32 offset; /* offset into region, in bytes, page-aligned */
- __u32 len; /* length forward from offset, in bytes, page-aligned */
-};
-
-#define __ASHMEMIOC 0x77
-
-#define ASHMEM_SET_NAME _IOW(__ASHMEMIOC, 1, char[ASHMEM_NAME_LEN])
-#define ASHMEM_GET_NAME _IOR(__ASHMEMIOC, 2, char[ASHMEM_NAME_LEN])
-#define ASHMEM_SET_SIZE _IOW(__ASHMEMIOC, 3, size_t)
-#define ASHMEM_GET_SIZE _IO(__ASHMEMIOC, 4)
-#define ASHMEM_SET_PROT_MASK _IOW(__ASHMEMIOC, 5, unsigned long)
-#define ASHMEM_GET_PROT_MASK _IO(__ASHMEMIOC, 6)
-#define ASHMEM_PIN _IOW(__ASHMEMIOC, 7, struct ashmem_pin)
-#define ASHMEM_UNPIN _IOW(__ASHMEMIOC, 8, struct ashmem_pin)
-#define ASHMEM_GET_PIN_STATUS _IO(__ASHMEMIOC, 9)
-#define ASHMEM_PURGE_ALL_CACHES _IO(__ASHMEMIOC, 10)
+#include "uapi/ashmem.h"

/* support of 32bit userspace on 64bit platforms */
#ifdef CONFIG_COMPAT
diff --git a/drivers/staging/android/uapi/ashmem.h b/drivers/staging/android/uapi/ashmem.h
new file mode 100644
index 0000000..ba4743c
--- /dev/null
+++ b/drivers/staging/android/uapi/ashmem.h
@@ -0,0 +1,47 @@
+/*
+ * drivers/staging/android/uapi/ashmem.h
+ *
+ * Copyright 2008 Google Inc.
+ * Author: Robert Love
+ *
+ * This file is dual licensed. It may be redistributed and/or modified
+ * under the terms of the Apache 2.0 License OR version 2 of the GNU
+ * General Public License.
+ */
+
+#ifndef _UAPI_LINUX_ASHMEM_H
+#define _UAPI_LINUX_ASHMEM_H
+
+#include <linux/ioctl.h>
+
+#define ASHMEM_NAME_LEN 256
+
+#define ASHMEM_NAME_DEF "dev/ashmem"
+
+/* Return values from ASHMEM_PIN: Was the mapping purged while unpinned? */
+#define ASHMEM_NOT_PURGED 0
+#define ASHMEM_WAS_PURGED 1
+
+/* Return values from ASHMEM_GET_PIN_STATUS: Is the mapping pinned? */
+#define ASHMEM_IS_UNPINNED 0
+#define ASHMEM_IS_PINNED 1
+
+struct ashmem_pin {
+ __u32 offset; /* offset into region, in bytes, page-aligned */
+ __u32 len; /* length forward from offset, in bytes, page-aligned */
+};
+
+#define __ASHMEMIOC 0x77
+
+#define ASHMEM_SET_NAME _IOW(__ASHMEMIOC, 1, char[ASHMEM_NAME_LEN])
+#define ASHMEM_GET_NAME _IOR(__ASHMEMIOC, 2, char[ASHMEM_NAME_LEN])
+#define ASHMEM_SET_SIZE _IOW(__ASHMEMIOC, 3, size_t)
+#define ASHMEM_GET_SIZE _IO(__ASHMEMIOC, 4)
+#define ASHMEM_SET_PROT_MASK _IOW(__ASHMEMIOC, 5, unsigned long)
+#define ASHMEM_GET_PROT_MASK _IO(__ASHMEMIOC, 6)
+#define ASHMEM_PIN _IOW(__ASHMEMIOC, 7, struct ashmem_pin)
+#define ASHMEM_UNPIN _IOW(__ASHMEMIOC, 8, struct ashmem_pin)
+#define ASHMEM_GET_PIN_STATUS _IO(__ASHMEMIOC, 9)
+#define ASHMEM_PURGE_ALL_CACHES _IO(__ASHMEMIOC, 10)
+
+#endif /* _UAPI_LINUX_ASHMEM_H */
--
1.8.3.2

2014-02-18 19:01:21

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 01/14] staging: binder: Fix death notifications

On Mon, Feb 17, 2014 at 01:58:29PM -0800, John Stultz wrote:
> From: Arve Hj?nnev?g <[email protected]>
>
> The change (008fa749e0fe5b2fffd20b7fe4891bb80d072c6a) that moved the
> node release code to a separate function broke death notifications in
> some cases. When it encountered a reference without a death
> notification request, it would skip looking at the remaining
> references, and therefore fail to send death notifications for them.
>
> Cc: Greg KH <[email protected]>
> Cc: Colin Cross <[email protected]>
> Cc: Arve Hj?nnev?g <[email protected]>
> Cc: Android Kernel Team <[email protected]>
> Signed-off-by: Arve Hj?nnev?g <[email protected]>
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/staging/android/binder.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)

As this broke in 3.10, shouldn't this go to staging-linus and get
backported to the stable trees too?

thanks,

greg k-h

2014-02-18 19:07:05

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

On Mon, Feb 17, 2014 at 01:58:40PM -0800, John Stultz wrote:
> From: Serban Constantinescu <[email protected]>
>
> This patch fixes the ABI for 64bit Android userspace.
> BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
> to be using struct binder_ptr_cookie, but they are using a 32bit handle
> and a pointer.
>
> On 32bit systems the payload size is the same as the size of struct
> binder_ptr_cookie, however for 64bit systems this will differ. This
> patch adds struct binder_handle_cookie that fixes this issue for 64bit
> Android.
>
> Since there are no 64bit users of this interface that we know of this
> change should not affect any existing systems.

But you are changing the ioctl structures here, what is that going to
cause with old programs?

>
> Cc: Colin Cross <[email protected]>
> Cc: Arve Hj?nnev?g <[email protected]>
> Cc: Serban Constantinescu <[email protected]>
> Cc: Android Kernel Team <[email protected]>

I am going to require Acks from someone on the Android team to accept
this, or any other 64bit binder patch, given all the back-and-forth that
has happened with the different patch sets here over the past year or
so.

Until then, I can't take this (and I think this patch is still
broken...)

> Signed-off-by: Serban Constantinescu <[email protected]>
> [jstultz: Minor commit tweaks, few 80+ col fixes for checkpatch]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/staging/android/uapi/binder.h | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
> index 2b1eb81..4071fcf 100644
> --- a/drivers/staging/android/uapi/binder.h
> +++ b/drivers/staging/android/uapi/binder.h
> @@ -152,6 +152,11 @@ struct binder_ptr_cookie {
> void *cookie;
> };
>
> +struct binder_handle_cookie {
> + __u32 handle;
> + void *cookie;
> +} __attribute__((packed));

Are you sure this isn't supposed to be a union?

> +
> struct binder_pri_desc {
> __s32 priority;
> __u32 desc;
> @@ -308,15 +313,17 @@ enum binder_driver_command_protocol {
> * of looping threads it has available.
> */
>
> - BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14, struct binder_ptr_cookie),
> + BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14,
> + struct binder_handle_cookie),
> /*
> - * void *: ptr to binder
> + * int: handle
> * void *: cookie

How does this not break existing user/kernel code if only one of them
gets rebuilt with this new header file? You just changed the ABI here,
not nice...

greg k-h

2014-02-18 19:08:11

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 13/14] staging: binder: Support concurrent 32 bit and 64 bit processes.

On Mon, Feb 17, 2014 at 01:58:41PM -0800, John Stultz wrote:
> From: Arve Hj?nnev?g <[email protected]>
>
> Add binder_size_t and binder_uintptr_t that is used instead of size_t and
> void __user * in the user-space interface.
>
> Use 64 bit pointers on all systems unless CONFIG_ANDROID_BINDER_IPC_32BIT
> is set (which enables the old protocol on 32 bit systems).
>
> Change BINDER_CURRENT_PROTOCOL_VERSION to 8 if
> CONFIG_ANDROID_BINDER_IPC_32BIT is not set.
>
> Add compat ioctl.
>
> Cc: Colin Cross <[email protected]>
> Cc: Arve Hj?nnev?g <[email protected]>
> Cc: Android Kernel Team <[email protected]>
> Signed-off-by: Arve Hj?nnev?g <[email protected]>
> [jstultz: Merged with upstream type changes. Tweaked commit message.
> Various whitespace fixes and longer Kconfig description for checkpatch]
> Signed-off-by: John Stultz <[email protected]>

Same as the previous patch, I'm not going to take this until the Android
team says it's ok, and given the borked-ness of the previous patch, I'm
doubting that's going to happen...

So consider these 3 patches dropped, feel free to resend them when fixed
up and acked.

thanks,

greg k-h

2014-02-18 19:09:19

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 13/14] staging: binder: Support concurrent 32 bit and 64 bit processes.

On Mon, Feb 17, 2014 at 01:58:41PM -0800, John Stultz wrote:
> From: Arve Hj?nnev?g <[email protected]>
>
> Add binder_size_t and binder_uintptr_t that is used instead of size_t and
> void __user * in the user-space interface.
>
> Use 64 bit pointers on all systems unless CONFIG_ANDROID_BINDER_IPC_32BIT
> is set (which enables the old protocol on 32 bit systems).
>
> Change BINDER_CURRENT_PROTOCOL_VERSION to 8 if
> CONFIG_ANDROID_BINDER_IPC_32BIT is not set.
>
> Add compat ioctl.
>
> Cc: Colin Cross <[email protected]>
> Cc: Arve Hj?nnev?g <[email protected]>
> Cc: Android Kernel Team <[email protected]>
> Signed-off-by: Arve Hj?nnev?g <[email protected]>
> [jstultz: Merged with upstream type changes. Tweaked commit message.
> Various whitespace fixes and longer Kconfig description for checkpatch]
> Signed-off-by: John Stultz <[email protected]>

Ok, I see that Arve has signed off on this, why the Cc: above to confuse
me? :)

Anyway, what about the ARM patches that this was based on? And why the
previous patch that breaks things on its own?

totally confused...

greg k-h

2014-02-18 19:21:26

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 01/14] staging: binder: Fix death notifications

On Tue, Feb 18, 2014 at 11:02 AM, Greg KH <[email protected]> wrote:
> On Mon, Feb 17, 2014 at 01:58:29PM -0800, John Stultz wrote:
>> From: Arve Hj?nnev?g <[email protected]>
>>
>> The change (008fa749e0fe5b2fffd20b7fe4891bb80d072c6a) that moved the
>> node release code to a separate function broke death notifications in
>> some cases. When it encountered a reference without a death
>> notification request, it would skip looking at the remaining
>> references, and therefore fail to send death notifications for them.
>>
>> Cc: Greg KH <[email protected]>
>> Cc: Colin Cross <[email protected]>
>> Cc: Arve Hj?nnev?g <[email protected]>
>> Cc: Android Kernel Team <[email protected]>
>> Signed-off-by: Arve Hj?nnev?g <[email protected]>
>> Signed-off-by: John Stultz <[email protected]>
>> ---
>> drivers/staging/android/binder.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> As this broke in 3.10, shouldn't this go to staging-linus and get
> backported to the stable trees too?

Right. I rearranged this fix to be in the front of this series because
of this, and mentioned in the cover letter that it might be considered
for 3.14. If that's considered ok, then it can be pushed to stable as
well.

thanks
-john

2014-02-18 19:30:32

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

On Tue, Feb 18, 2014 at 11:08 AM, Greg KH <[email protected]> wrote:
> On Mon, Feb 17, 2014 at 01:58:40PM -0800, John Stultz wrote:
>> From: Serban Constantinescu <[email protected]>
>>
>> This patch fixes the ABI for 64bit Android userspace.
>> BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
>> to be using struct binder_ptr_cookie, but they are using a 32bit handle
>> and a pointer.
>>
>> On 32bit systems the payload size is the same as the size of struct
>> binder_ptr_cookie, however for 64bit systems this will differ. This
>> patch adds struct binder_handle_cookie that fixes this issue for 64bit
>> Android.
>>
>> Since there are no 64bit users of this interface that we know of this
>> change should not affect any existing systems.
>
> But you are changing the ioctl structures here, what is that going to
> cause with old programs?

So I'd be glad for Serban or Arve to clarify, but my understanding
(and as is described in the commit message) is that the assumption is
there are no 64bit binder users at this point, and the ioctl structure
changes are made such that existing 32bit applications are unaffected.

>> Cc: Colin Cross <[email protected]>
>> Cc: Arve Hj?nnev?g <[email protected]>
>> Cc: Serban Constantinescu <[email protected]>
>> Cc: Android Kernel Team <[email protected]>
>
> I am going to require Acks from someone on the Android team to accept
> this, or any other 64bit binder patch, given all the back-and-forth that
> has happened with the different patch sets here over the past year or
> so.

Certainly reasonable given the earlier back and forth. For extra
context, these have been merged into the 3.10 AOSP by Arve:
https://android-review.googlesource.com/#/c/79228/

thanks
-john

2014-02-18 19:31:49

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 01/14] staging: binder: Fix death notifications

On Tue, Feb 18, 2014 at 11:21:24AM -0800, John Stultz wrote:
> On Tue, Feb 18, 2014 at 11:02 AM, Greg KH <[email protected]> wrote:
> > On Mon, Feb 17, 2014 at 01:58:29PM -0800, John Stultz wrote:
> >> From: Arve Hj?nnev?g <[email protected]>
> >>
> >> The change (008fa749e0fe5b2fffd20b7fe4891bb80d072c6a) that moved the
> >> node release code to a separate function broke death notifications in
> >> some cases. When it encountered a reference without a death
> >> notification request, it would skip looking at the remaining
> >> references, and therefore fail to send death notifications for them.
> >>
> >> Cc: Greg KH <[email protected]>
> >> Cc: Colin Cross <[email protected]>
> >> Cc: Arve Hj?nnev?g <[email protected]>
> >> Cc: Android Kernel Team <[email protected]>
> >> Signed-off-by: Arve Hj?nnev?g <[email protected]>
> >> Signed-off-by: John Stultz <[email protected]>
> >> ---
> >> drivers/staging/android/binder.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > As this broke in 3.10, shouldn't this go to staging-linus and get
> > backported to the stable trees too?
>
> Right. I rearranged this fix to be in the front of this series because
> of this, and mentioned in the cover letter that it might be considered
> for 3.14. If that's considered ok, then it can be pushed to stable as
> well.

I never read 00/ emails because they will never show up in the kernel
changelog :)

I'll go queue this up for 3.14-final and the stable trees.

thanks,

greg k-h

2014-02-18 19:43:18

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 13/14] staging: binder: Support concurrent 32 bit and 64 bit processes.

On Tue, Feb 18, 2014 at 11:10 AM, Greg KH <[email protected]> wrote:
> On Mon, Feb 17, 2014 at 01:58:41PM -0800, John Stultz wrote:
>> From: Arve Hj?nnev?g <[email protected]>
>>
>> Add binder_size_t and binder_uintptr_t that is used instead of size_t and
>> void __user * in the user-space interface.
>>
>> Use 64 bit pointers on all systems unless CONFIG_ANDROID_BINDER_IPC_32BIT
>> is set (which enables the old protocol on 32 bit systems).
>>
>> Change BINDER_CURRENT_PROTOCOL_VERSION to 8 if
>> CONFIG_ANDROID_BINDER_IPC_32BIT is not set.
>>
>> Add compat ioctl.
>>
>> Cc: Colin Cross <[email protected]>
>> Cc: Arve Hj?nnev?g <[email protected]>
>> Cc: Android Kernel Team <[email protected]>
>> Signed-off-by: Arve Hj?nnev?g <[email protected]>
>> [jstultz: Merged with upstream type changes. Tweaked commit message.
>> Various whitespace fixes and longer Kconfig description for checkpatch]
>> Signed-off-by: John Stultz <[email protected]>
>
> Ok, I see that Arve has signed off on this, why the Cc: above to confuse
> me? :)

Apologies for the confusion. These are definitely big changes that
landed recently.

> Anyway, what about the ARM patches that this was based on? And why the
> previous patch that breaks things on its own?

So, the previous change was making the now-old 32bit binder api
consistent on 64bit, and then this change converts binder api to the
new 64bit API, which will break existing 32bit binder users. Because
of this, they provide a CONFIG_ANDROID_BINDER_IPC_32BIT option which
switches back to the old ABI for current userspace users.

But yea, I'm fine resubmitting these independently. I'll also work
with Serban to see if we can expand the commit message to be more
clear.

thanks
-john

2014-02-18 19:48:13

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

On Tue, Feb 18, 2014 at 11:30:26AM -0800, John Stultz wrote:
> On Tue, Feb 18, 2014 at 11:08 AM, Greg KH <[email protected]> wrote:
> > On Mon, Feb 17, 2014 at 01:58:40PM -0800, John Stultz wrote:
> >> From: Serban Constantinescu <[email protected]>
> >>
> >> This patch fixes the ABI for 64bit Android userspace.
> >> BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
> >> to be using struct binder_ptr_cookie, but they are using a 32bit handle
> >> and a pointer.
> >>
> >> On 32bit systems the payload size is the same as the size of struct
> >> binder_ptr_cookie, however for 64bit systems this will differ. This
> >> patch adds struct binder_handle_cookie that fixes this issue for 64bit
> >> Android.
> >>
> >> Since there are no 64bit users of this interface that we know of this
> >> change should not affect any existing systems.
> >
> > But you are changing the ioctl structures here, what is that going to
> > cause with old programs?
>
> So I'd be glad for Serban or Arve to clarify, but my understanding
> (and as is described in the commit message) is that the assumption is
> there are no 64bit binder users at this point, and the ioctl structure
> changes are made such that existing 32bit applications are unaffected.

How does changing the structure size, and contents, not affect any
applications or the kernel code? What am I missing here?

> >> Cc: Colin Cross <[email protected]>
> >> Cc: Arve Hj?nnev?g <[email protected]>
> >> Cc: Serban Constantinescu <[email protected]>
> >> Cc: Android Kernel Team <[email protected]>
> >
> > I am going to require Acks from someone on the Android team to accept
> > this, or any other 64bit binder patch, given all the back-and-forth that
> > has happened with the different patch sets here over the past year or
> > so.
>
> Certainly reasonable given the earlier back and forth. For extra
> context, these have been merged into the 3.10 AOSP by Arve:
> https://android-review.googlesource.com/#/c/79228/

That's good to see, and a good reason to get them merged, but better
descriptions and acks would be nice to have :)

How about sending these as a separate series when all worked out, as
lots of people seem interested in them?

thanks,

greg k-h

2014-02-18 20:02:10

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

On Tue, Feb 18, 2014 at 11:49 AM, Greg KH <[email protected]> wrote:
> On Tue, Feb 18, 2014 at 11:30:26AM -0800, John Stultz wrote:
>> On Tue, Feb 18, 2014 at 11:08 AM, Greg KH <[email protected]> wrote:
>> > On Mon, Feb 17, 2014 at 01:58:40PM -0800, John Stultz wrote:
>> >> From: Serban Constantinescu <[email protected]>
>> >>
>> >> This patch fixes the ABI for 64bit Android userspace.
>> >> BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
>> >> to be using struct binder_ptr_cookie, but they are using a 32bit handle
>> >> and a pointer.
>> >>
>> >> On 32bit systems the payload size is the same as the size of struct
>> >> binder_ptr_cookie, however for 64bit systems this will differ. This
>> >> patch adds struct binder_handle_cookie that fixes this issue for 64bit
>> >> Android.
>> >>
>> >> Since there are no 64bit users of this interface that we know of this
>> >> change should not affect any existing systems.
>> >
>> > But you are changing the ioctl structures here, what is that going to
>> > cause with old programs?
>>
>> So I'd be glad for Serban or Arve to clarify, but my understanding
>> (and as is described in the commit message) is that the assumption is
>> there are no 64bit binder users at this point, and the ioctl structure
>> changes are made such that existing 32bit applications are unaffected.
>
> How does changing the structure size, and contents, not affect any
> applications or the kernel code? What am I missing here?

On 32bit pointers and ints are the same size? (Years ago I sat through
your presentation on this, so I'm worried I'm missing something here
:)

struct binder_ptr_cookie {
void *ptr;
void *cookie;
};

struct binder_handle_cookie {
__u32 handle;
void *cookie;
} __attribute__((packed));


On 32bit systems these are the same size. Now on 64bit systems, this
changes things, and would break users, but the assumption here is
there are no pre-existing 64bit binder users.


>> >> Cc: Colin Cross <[email protected]>
>> >> Cc: Arve Hj?nnev?g <[email protected]>
>> >> Cc: Serban Constantinescu <[email protected]>
>> >> Cc: Android Kernel Team <[email protected]>
>> >
>> > I am going to require Acks from someone on the Android team to accept
>> > this, or any other 64bit binder patch, given all the back-and-forth that
>> > has happened with the different patch sets here over the past year or
>> > so.
>>
>> Certainly reasonable given the earlier back and forth. For extra
>> context, these have been merged into the 3.10 AOSP by Arve:
>> https://android-review.googlesource.com/#/c/79228/
>
> That's good to see, and a good reason to get them merged, but better
> descriptions and acks would be nice to have :)
>
> How about sending these as a separate series when all worked out, as
> lots of people seem interested in them?

Yep. Will do.

thanks
-john

2014-02-18 20:31:16

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

On Tue, Feb 18, 2014 at 12:02:07PM -0800, John Stultz wrote:
> On Tue, Feb 18, 2014 at 11:49 AM, Greg KH <[email protected]> wrote:
> > On Tue, Feb 18, 2014 at 11:30:26AM -0800, John Stultz wrote:
> >> On Tue, Feb 18, 2014 at 11:08 AM, Greg KH <[email protected]> wrote:
> >> > On Mon, Feb 17, 2014 at 01:58:40PM -0800, John Stultz wrote:
> >> >> From: Serban Constantinescu <[email protected]>
> >> >>
> >> >> This patch fixes the ABI for 64bit Android userspace.
> >> >> BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
> >> >> to be using struct binder_ptr_cookie, but they are using a 32bit handle
> >> >> and a pointer.
> >> >>
> >> >> On 32bit systems the payload size is the same as the size of struct
> >> >> binder_ptr_cookie, however for 64bit systems this will differ. This
> >> >> patch adds struct binder_handle_cookie that fixes this issue for 64bit
> >> >> Android.
> >> >>
> >> >> Since there are no 64bit users of this interface that we know of this
> >> >> change should not affect any existing systems.
> >> >
> >> > But you are changing the ioctl structures here, what is that going to
> >> > cause with old programs?
> >>
> >> So I'd be glad for Serban or Arve to clarify, but my understanding
> >> (and as is described in the commit message) is that the assumption is
> >> there are no 64bit binder users at this point, and the ioctl structure
> >> changes are made such that existing 32bit applications are unaffected.
> >
> > How does changing the structure size, and contents, not affect any
> > applications or the kernel code? What am I missing here?
>
> On 32bit pointers and ints are the same size? (Years ago I sat through
> your presentation on this, so I'm worried I'm missing something here
> :)
>
> struct binder_ptr_cookie {
> void *ptr;
> void *cookie;
> };
>
> struct binder_handle_cookie {
> __u32 handle;
> void *cookie;
> } __attribute__((packed));
>
>
> On 32bit systems these are the same size. Now on 64bit systems, this
> changes things, and would break users, but the assumption here is
> there are no pre-existing 64bit binder users.

But you added a field to the existing structure, right? I don't really
remember the patch, it was a few hundred back in my review of stuff
today, sorry...

greg k-h

2014-02-19 00:08:22

by Arve Hjønnevåg

[permalink] [raw]
Subject: Re: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

On Tue, Feb 18, 2014 at 12:32 PM, Greg KH <[email protected]> wrote:
> On Tue, Feb 18, 2014 at 12:02:07PM -0800, John Stultz wrote:
>> On Tue, Feb 18, 2014 at 11:49 AM, Greg KH <[email protected]> wrote:
>> > On Tue, Feb 18, 2014 at 11:30:26AM -0800, John Stultz wrote:
>> >> On Tue, Feb 18, 2014 at 11:08 AM, Greg KH <[email protected]> wrote:
>> >> > On Mon, Feb 17, 2014 at 01:58:40PM -0800, John Stultz wrote:
>> >> >> From: Serban Constantinescu <[email protected]>
>> >> >>
>> >> >> This patch fixes the ABI for 64bit Android userspace.
>> >> >> BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
>> >> >> to be using struct binder_ptr_cookie, but they are using a 32bit handle
>> >> >> and a pointer.
>> >> >>
>> >> >> On 32bit systems the payload size is the same as the size of struct
>> >> >> binder_ptr_cookie, however for 64bit systems this will differ. This
>> >> >> patch adds struct binder_handle_cookie that fixes this issue for 64bit
>> >> >> Android.
>> >> >>
>> >> >> Since there are no 64bit users of this interface that we know of this
>> >> >> change should not affect any existing systems.
>> >> >
>> >> > But you are changing the ioctl structures here, what is that going to
>> >> > cause with old programs?
>> >>
>> >> So I'd be glad for Serban or Arve to clarify, but my understanding
>> >> (and as is described in the commit message) is that the assumption is
>> >> there are no 64bit binder users at this point, and the ioctl structure
>> >> changes are made such that existing 32bit applications are unaffected.
>> >
>> > How does changing the structure size, and contents, not affect any
>> > applications or the kernel code? What am I missing here?
>>
>> On 32bit pointers and ints are the same size? (Years ago I sat through
>> your presentation on this, so I'm worried I'm missing something here
>> :)
>>
>> struct binder_ptr_cookie {
>> void *ptr;
>> void *cookie;
>> };
>>
>> struct binder_handle_cookie {
>> __u32 handle;
>> void *cookie;
>> } __attribute__((packed));
>>
>>
>> On 32bit systems these are the same size. Now on 64bit systems, this
>> changes things, and would break users, but the assumption here is
>> there are no pre-existing 64bit binder users.
>
> But you added a field to the existing structure, right? I don't really
> remember the patch, it was a few hundred back in my review of stuff
> today, sorry...
>
> greg k-h

The existing structure is not changed. These two commands were defined
with wrong structure that did not match the code. Since a binder
pointer and handle are the same size on 32 bit systems, this change
does not affect them. On 64 bit systems, the ioctl number does change,
but these systems need the next patch to run 32 bit processes anyway,
so I don't expect anyone to ship a system without this change. The
main purpose of this patch is to add the binder_handle_cookie struct
so the service manager does not have to define its own version
(libbinder writes one field at a time so it does not use the struct).

--
Arve Hj?nnev?g

2014-02-19 00:18:09

by Arve Hjønnevåg

[permalink] [raw]
Subject: Re: [PATCH 14/14] staging: binder: Improve Kconfig entry for ANDROID_BINDER_IPC_32BIT

On Mon, Feb 17, 2014 at 1:58 PM, John Stultz <[email protected]> wrote:
> Add a more clear explanation of the option in the prompt, and
> make the config depend on ANDROID_BINDER_IPC being selected.
>
> Also sets the default to y, which matches AOSP.
>

I don't know if you want the default to be y. By the time this kernel
is used, the user-space default may have switched to using the 64 bit
interface by default. The android-3.10 kernel currently defaulting to
y since it may get used with a 4.4 android user-space, and because the
32 bit arm kernel is missing the 8 byte get_user variant that it
needs.

--
Arve Hj?nnev?g

2014-02-19 00:30:48

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 14/14] staging: binder: Improve Kconfig entry for ANDROID_BINDER_IPC_32BIT

On 02/18/2014 04:18 PM, Arve Hj?nnev?g wrote:
> On Mon, Feb 17, 2014 at 1:58 PM, John Stultz <[email protected]> wrote:
>> Add a more clear explanation of the option in the prompt, and
>> make the config depend on ANDROID_BINDER_IPC being selected.
>>
>> Also sets the default to y, which matches AOSP.
>>
> I don't know if you want the default to be y. By the time this kernel
> is used, the user-space default may have switched to using the 64 bit
> interface by default. The android-3.10 kernel currently defaulting to
> y since it may get used with a 4.4 android user-space, and because the
> 32 bit arm kernel is missing the 8 byte get_user variant that it
> needs.

My thought is that currently folks who might be using the upstream
kernel would experience less friction, as they are likely to be using
the current userspace. Once the new userspace with the 64bit aware
binder is commonly available, removing the "default y" makes sense.

But until then it seems having a new config that defaults to n and
breaks existing users might be viewed as "unfriendly" :)

Does that seem reasonable?

thanks
-john

2014-02-19 02:11:16

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 12/14] staging: binder: Fix ABI for 64bit Android

On Tue, Feb 18, 2014 at 04:08:20PM -0800, Arve Hj?nnev?g wrote:
> On Tue, Feb 18, 2014 at 12:32 PM, Greg KH <[email protected]> wrote:
> > On Tue, Feb 18, 2014 at 12:02:07PM -0800, John Stultz wrote:
> >> On Tue, Feb 18, 2014 at 11:49 AM, Greg KH <[email protected]> wrote:
> >> > On Tue, Feb 18, 2014 at 11:30:26AM -0800, John Stultz wrote:
> >> >> On Tue, Feb 18, 2014 at 11:08 AM, Greg KH <[email protected]> wrote:
> >> >> > On Mon, Feb 17, 2014 at 01:58:40PM -0800, John Stultz wrote:
> >> >> >> From: Serban Constantinescu <[email protected]>
> >> >> >>
> >> >> >> This patch fixes the ABI for 64bit Android userspace.
> >> >> >> BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim
> >> >> >> to be using struct binder_ptr_cookie, but they are using a 32bit handle
> >> >> >> and a pointer.
> >> >> >>
> >> >> >> On 32bit systems the payload size is the same as the size of struct
> >> >> >> binder_ptr_cookie, however for 64bit systems this will differ. This
> >> >> >> patch adds struct binder_handle_cookie that fixes this issue for 64bit
> >> >> >> Android.
> >> >> >>
> >> >> >> Since there are no 64bit users of this interface that we know of this
> >> >> >> change should not affect any existing systems.
> >> >> >
> >> >> > But you are changing the ioctl structures here, what is that going to
> >> >> > cause with old programs?
> >> >>
> >> >> So I'd be glad for Serban or Arve to clarify, but my understanding
> >> >> (and as is described in the commit message) is that the assumption is
> >> >> there are no 64bit binder users at this point, and the ioctl structure
> >> >> changes are made such that existing 32bit applications are unaffected.
> >> >
> >> > How does changing the structure size, and contents, not affect any
> >> > applications or the kernel code? What am I missing here?
> >>
> >> On 32bit pointers and ints are the same size? (Years ago I sat through
> >> your presentation on this, so I'm worried I'm missing something here
> >> :)
> >>
> >> struct binder_ptr_cookie {
> >> void *ptr;
> >> void *cookie;
> >> };
> >>
> >> struct binder_handle_cookie {
> >> __u32 handle;
> >> void *cookie;
> >> } __attribute__((packed));
> >>
> >>
> >> On 32bit systems these are the same size. Now on 64bit systems, this
> >> changes things, and would break users, but the assumption here is
> >> there are no pre-existing 64bit binder users.
> >
> > But you added a field to the existing structure, right? I don't really
> > remember the patch, it was a few hundred back in my review of stuff
> > today, sorry...
> >
> > greg k-h
>
> The existing structure is not changed. These two commands were defined
> with wrong structure that did not match the code. Since a binder
> pointer and handle are the same size on 32 bit systems, this change
> does not affect them. On 64 bit systems, the ioctl number does change,
> but these systems need the next patch to run 32 bit processes anyway,
> so I don't expect anyone to ship a system without this change. The
> main purpose of this patch is to add the binder_handle_cookie struct
> so the service manager does not have to define its own version
> (libbinder writes one field at a time so it does not use the struct).

Ah, ok, that makes more sense, can someone put it in the changelog
information so that I don't have to reject the patch for the same reason
again?

thanks,

greg k-h