On 2013年04月06日 08:40, � wrote:
> On Fri, Apr 5, 2013 at 3:13 AM, Chen Gang <gang.chen@xxxxxxxxxxx> wrote:
>>
>> for space consummation statistics, 'buffer_size' is more accurate than 'size'.
>
> No it is not. buffer_size may grow to allocate a new free buffer
> header, which is unrelated to the current allocation.
>
I am not quite agree with what your said (your reasons).
but after reading code carefully, my patch is really incorrect.
I think:
original statistics are not quite precise.
but it can not cause issue (it is correct, but waste a little free spaces)
in our condition, need not let it quite precise (we can bear it, at least)
if we try to let it more precise.
it will let the statistics code much complex which we will not prefer.
:-)
>>
>> only when "n == NULL" and 'size' is more smaller than 'buffer_size'
>> it will be "size + sizeof(struct binder_buffer)".
>> else
>> it is still 'size'.
>>
>> that is just the meaning of 'buffer_size'.
>>
>> so use 'buffer_size' instead of 'size', which can save some free space.
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@xxxxxxxxxxx>
>> ---
>> drivers/staging/android/binder.c | 9 ++++-----
>> 1 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
>> index 1567ac2..a207e58 100644
>> --- a/drivers/staging/android/binder.c
>> +++ b/drivers/staging/android/binder.c
>> @@ -667,8 +667,7 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
>> return NULL;
>> }
>>
>> - if (is_async &&
>> - proc->free_async_space < size + sizeof(struct binder_buffer)) {
>> + if (is_async && proc->free_async_space < size) {
>> binder_debug(BINDER_DEBUG_BUFFER_ALLOC,
>> "%d: binder_alloc_buf size %zd failed, no async space left\n",
>> proc->pid, size);
>> @@ -736,7 +735,7 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
>> buffer->offsets_size = offsets_size;
>> buffer->async_transaction = is_async;
>> if (is_async) {
>> - proc->free_async_space -= size + sizeof(struct binder_buffer);
>> + proc->free_async_space -= buffer_size;
>> binder_debug(BINDER_DEBUG_BUFFER_ALLOC_ASYNC,
>> "%d: binder_alloc_buf size %zd async free %zd\n",
>> proc->pid, size, proc->free_async_space);
>> @@ -821,11 +820,11 @@ static void binder_free_buf(struct binder_proc *proc,
>> BUG_ON((void *)buffer > proc->buffer + proc->buffer_size);
>>
>> if (buffer->async_transaction) {
>> - proc->free_async_space += size + sizeof(struct binder_buffer);
>> + proc->free_async_space += buffer_size;
>
> This buffer size is not the same as the buffer_size you subtracted in
> binder_alloc_buf.
>
no, they are not the same, although they are the same names.
originally, I knew about it (they are not same).
since we have agreed with my patch is incorrect, we need not talk
about it again.
>>
>> binder_debug(BINDER_DEBUG_BUFFER_ALLOC_ASYNC,
>> "%d: binder_free_buf size %zd async free %zd\n",
>> - proc->pid, size, proc->free_async_space);
>> + proc->pid, buffer_size, proc->free_async_space);
>> }
>>
>> binder_update_page_range(proc, 0,
>> --
>> 1.7.7.6
>
> I don't think this change works.
>
ok, thanks.
> --
> Arve Hj�nnev�g
>
>
--
Chen Gang
Asianux Corporation
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
