2012-06-23 06:10:39

by Sherwin Soltani

[permalink] [raw]
Subject: [PATCH] drivers: staging: android: fix binder.c printk macros

Changed to pr_warn, pr_err, and pr_info macros and reformatted some
lines to bring them under the 80 column limit in the binder.c code.

Signed-off-by: Sherwin Soltani <[email protected]>
---
drivers/staging/android/binder.c | 74 ++++++++++++++++++++------------------
1 file changed, 40 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index c283212..090d594 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -124,13 +124,13 @@ module_param_call(stop_on_user_error, binder_set_stop_on_user_error,
#define binder_debug(mask, x...) \
do { \
if (binder_debug_mask & mask) \
- printk(KERN_INFO x); \
+ pr_info(x); \
} while (0)

#define binder_user_error(x...) \
do { \
if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) \
- printk(KERN_INFO x); \
+ pr_info(x); \
if (binder_stop_on_user_error) \
binder_stop_on_user_error = 2; \
} while (0)
@@ -418,7 +418,7 @@ repeat:
#if 1
/* Sanity check */
if (fdt->fd[fd] != NULL) {
- printk(KERN_WARNING "get_unused_fd: slot %d not NULL!\n", fd);
+ pr_warn("get_unused_fd: slot %d not NULL!\n", fd);
fdt->fd[fd] = NULL;
}
#endif
@@ -644,7 +644,7 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
goto free_range;

if (vma == NULL) {
- printk(KERN_ERR "binder: %d: binder_alloc_buf failed to "
+ pr_err("binder: %d: binder_alloc_buf failed to "
"map pages in userspace, no vma\n", proc->pid);
goto err_no_vma;
}
@@ -657,7 +657,7 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
BUG_ON(*page);
*page = alloc_page(GFP_KERNEL | __GFP_ZERO);
if (*page == NULL) {
- printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
+ pr_err("binder: %d: binder_alloc_buf failed "
"for page at %p\n", proc->pid, page_addr);
goto err_alloc_page_failed;
}
@@ -666,7 +666,7 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
page_array_ptr = page;
ret = map_vm_area(&tmp_area, PAGE_KERNEL, &page_array_ptr);
if (ret) {
- printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
+ pr_err("binder: %d: binder_alloc_buf failed "
"to map page at %p in kernel\n",
proc->pid, page_addr);
goto err_map_kernel_failed;
@@ -675,7 +675,7 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
(uintptr_t)page_addr + proc->user_buffer_offset;
ret = vm_insert_page(vma, user_page_addr, page[0]);
if (ret) {
- printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
+ pr_err("binder: %d: binder_alloc_buf failed "
"to map page at %lx in userspace\n",
proc->pid, user_page_addr);
goto err_vm_insert_page_failed;
@@ -724,7 +724,7 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
size_t size;

if (proc->vma == NULL) {
- printk(KERN_ERR "binder: %d: binder_alloc_buf, no vma\n",
+ pr_err("binder: %d: binder_alloc_buf, no vma\n",
proc->pid);
return NULL;
}
@@ -762,7 +762,7 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
}
}
if (best_fit == NULL) {
- printk(KERN_ERR "binder: %d: binder_alloc_buf size %zd failed, "
+ pr_err("binder: %d: binder_alloc_buf size %zd failed, "
"no address space\n", proc->pid, size);
return NULL;
}
@@ -997,7 +997,7 @@ static int binder_inc_node(struct binder_node *node, int strong, int internal,
node->internal_strong_refs == 0 &&
!(node == binder_context_mgr_node &&
node->has_strong_ref)) {
- printk(KERN_ERR "binder: invalid inc strong "
+ pr_err("binder: invalid inc strong "
"node for %d\n", node->debug_id);
return -EINVAL;
}
@@ -1013,8 +1013,8 @@ static int binder_inc_node(struct binder_node *node, int strong, int internal,
node->local_weak_refs++;
if (!node->has_weak_ref && list_empty(&node->work.entry)) {
if (target_list == NULL) {
- printk(KERN_ERR "binder: invalid inc weak node "
- "for %d\n", node->debug_id);
+ pr_err("binder: invalid inc weak node for %d\n",
+ node->debug_id);
return -EINVAL;
}
list_add_tail(&node->work.entry, target_list);
@@ -1276,7 +1276,7 @@ static void binder_send_failed_reply(struct binder_transaction *t,
target_thread->return_error = error_code;
wake_up_interruptible(&target_thread->wait);
} else {
- printk(KERN_ERR "binder: reply failed, target "
+ pr_err("binder: reply failed, target "
"thread, %d:%d, has error code %d "
"already\n", target_thread->proc->pid,
target_thread->pid,
@@ -1321,7 +1321,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 = (size_t *)(buffer->data +
+ ALIGN(buffer->data_size, sizeof(void *)));
if (failed_at)
off_end = failed_at;
else
@@ -1331,18 +1332,19 @@ 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(void *))) {
- printk(KERN_ERR "binder: transaction release %d bad"
- "offset %zd, size %zd\n", debug_id,
- *offp, buffer->data_size);
+ pr_err("binder: transaction release %d bad"
+ "offset %zd, size %zd\n",
+ debug_id, *offp, buffer->data_size);
continue;
}
fp = (struct flat_binder_object *)(buffer->data + *offp);
switch (fp->type) {
case BINDER_TYPE_BINDER:
case BINDER_TYPE_WEAK_BINDER: {
- struct binder_node *node = binder_get_node(proc, fp->binder);
+ struct binder_node *node =
+ binder_get_node(proc, fp->binder);
if (node == NULL) {
- printk(KERN_ERR "binder: transaction release %d"
+ pr_err("binder: transaction release %d"
" bad node %p\n", debug_id, fp->binder);
break;
}
@@ -1355,14 +1357,15 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
case BINDER_TYPE_WEAK_HANDLE: {
struct binder_ref *ref = binder_get_ref(proc, fp->handle);
if (ref == NULL) {
- printk(KERN_ERR "binder: transaction release %d"
+ pr_err("binder: transaction release %d"
" bad handle %ld\n", debug_id,
fp->handle);
break;
}
binder_debug(BINDER_DEBUG_TRANSACTION,
" ref %d desc %d (node %d)\n",
- ref->debug_id, ref->desc, ref->node->debug_id);
+ ref->debug_id, ref->desc,
+ ref->node->debug_id);
binder_dec_ref(ref, fp->type == BINDER_TYPE_HANDLE);
} break;

@@ -1374,7 +1377,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
break;

default:
- printk(KERN_ERR "binder: transaction release %d bad "
+ pr_err("binder: transaction release %d bad "
"object type %lx\n", debug_id, fp->type);
break;
}
@@ -1925,10 +1928,10 @@ int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread,
break;
}
case BC_ATTEMPT_ACQUIRE:
- printk(KERN_ERR "binder: BC_ATTEMPT_ACQUIRE not supported\n");
+ pr_err("binder: BC_ATTEMPT_ACQUIRE not supported\n");
return -EINVAL;
case BC_ACQUIRE_RESULT:
- printk(KERN_ERR "binder: BC_ACQUIRE_RESULT not supported\n");
+ pr_err("binder: BC_ACQUIRE_RESULT not supported\n");
return -EINVAL;

case BC_FREE_BUFFER: {
@@ -2165,7 +2168,7 @@ int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread,
} break;

default:
- printk(KERN_ERR "binder: %d:%d unknown command %d\n",
+ pr_err("binder: %d:%d unknown command %d\n",
proc->pid, thread->pid, cmd);
return -EINVAL;
}
@@ -2254,7 +2257,8 @@ retry:
if (!binder_has_proc_work(proc, thread))
ret = -EAGAIN;
} else
- ret = wait_event_interruptible_exclusive(proc->wait, binder_has_proc_work(proc, thread));
+ ret = wait_event_interruptible_exclusive(proc->wait,
+ binder_has_proc_work(proc, thread));
} else {
if (non_block) {
if (!binder_has_thread_work(thread))
@@ -2635,7 +2639,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
unsigned int size = _IOC_SIZE(cmd);
void __user *ubuf = (void __user *)arg;

- /*printk(KERN_INFO "binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
+ /*pr_err("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/

ret = wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
if (ret)
@@ -2701,13 +2705,13 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
break;
case BINDER_SET_CONTEXT_MGR:
if (binder_context_mgr_node != NULL) {
- printk(KERN_ERR "binder: BINDER_SET_CONTEXT_MGR already set\n");
+ pr_err("binder: BINDER_SET_CONTEXT_MGR already set\n");
ret = -EBUSY;
goto err;
}
if (binder_context_mgr_uid != -1) {
if (binder_context_mgr_uid != current->cred->euid) {
- printk(KERN_ERR "binder: BINDER_SET_"
+ pr_err("binder: BINDER_SET_"
"CONTEXT_MGR bad uid %d != %d\n",
current->cred->euid,
binder_context_mgr_uid);
@@ -2753,7 +2757,7 @@ err:
mutex_unlock(&binder_lock);
wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
if (ret && ret != -ERESTARTSYS)
- printk(KERN_INFO "binder: %d:%d ioctl %x %lx returned %d\n", proc->pid, current->pid, cmd, arg, ret);
+ pr_info("binder: %d:%d ioctl %x %lx returned %d\n", proc->pid, current->pid, cmd, arg, ret);
return ret;
}

@@ -2829,7 +2833,9 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
#ifdef CONFIG_CPU_CACHE_VIPT
if (cache_is_vipt_aliasing()) {
while (CACHE_COLOUR((vma->vm_start ^ (uint32_t)proc->buffer))) {
- printk(KERN_INFO "binder_mmap: %d %lx-%lx maps %p bad alignment\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer);
+ pr_info("binder_mmap: %d %lx-%lx maps %p bad alignment\n",
+ proc->pid, vma->vm_start,
+ vma->vm_end, proc->buffer);
vma->vm_start += PAGE_SIZE;
}
}
@@ -2861,7 +2867,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
proc->vma = vma;
proc->vma_vm_mm = vma->vm_mm;

- /*printk(KERN_INFO "binder_mmap: %d %lx-%lx maps %p\n",
+ /*pr_info("binder_mmap: %d %lx-%lx maps %p\n",
proc->pid, vma->vm_start, vma->vm_end, proc->buffer);*/
return 0;

@@ -2876,7 +2882,7 @@ err_get_vm_area_failed:
err_already_mapped:
mutex_unlock(&binder_mmap_lock);
err_bad_arg:
- printk(KERN_ERR "binder_mmap: %d %lx-%lx %s failed %d\n",
+ pr_err("binder_mmap: %d %lx-%lx %s failed %d\n",
proc->pid, vma->vm_start, vma->vm_end, failure_string, ret);
return ret;
}
@@ -3031,7 +3037,7 @@ static void binder_deferred_release(struct binder_proc *proc)
if (t) {
t->buffer = NULL;
buffer->transaction = NULL;
- printk(KERN_ERR "binder: release proc %d, "
+ pr_err("binder: release proc %d, "
"transaction %d, not freed\n",
proc->pid, t->debug_id);
/*BUG();*/
--
1.7.9.5


2012-06-25 17:17:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] drivers: staging: android: fix binder.c printk macros

On Sat, Jun 23, 2012 at 02:09:41AM -0400, Sherwin Soltani wrote:
> Changed to pr_warn, pr_err, and pr_info macros and reformatted some
> lines to bring them under the 80 column limit in the binder.c code.
>
> Signed-off-by: Sherwin Soltani <[email protected]>
> ---
> drivers/staging/android/binder.c | 74 ++++++++++++++++++++------------------
> 1 file changed, 40 insertions(+), 34 deletions(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

--- Your patch did many different things all at once, making it
difficult to review. All Linux kernel patches need to only do one
thing at a time. If you need to do multiple things (such as clean
up all coding style issues in a file/driver), do it in a sequence
of patches, each one doing only one thing. This will make it
easier to review the patches to ensure that they are correct, and
to help alleviate any merge issues that larger patches can cause.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2012-06-25 17:20:03

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] drivers: staging: android: fix binder.c printk macros

On Mon, 2012-06-25 at 10:17 -0700, Greg KH wrote:
> On Sat, Jun 23, 2012 at 02:09:41AM -0400, Sherwin Soltani wrote:
> > Changed to pr_warn, pr_err, and pr_info macros and reformatted some
> > lines to bring them under the 80 column limit in the binder.c code.
> >
> > Signed-off-by: Sherwin Soltani <[email protected]>
> > ---
> > drivers/staging/android/binder.c | 74 ++++++++++++++++++++------------------
> > 1 file changed, 40 insertions(+), 34 deletions(-)
>
> Hi,
>
> This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
> a patch that has triggered this response. He used to manually respond
> to these common problems, but in order to save his sanity (he kept
> writing the same thing over and over, yet to different people), I was
> created. Hopefully you will not take offence and will fix the problem
> in your patch and resubmit it so that it can be accepted into the Linux
> kernel tree.

> You are receiving this message because of the following common error(s)
> as indicated below:
>
> --- Your patch did many different things all at once, making it
> difficult to review.

Your patch-bot needs work.

Converting printks to pr_<level> is trivial
and should be done all at once, not piecemeal.

2012-06-25 17:24:28

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] drivers: staging: android: fix binder.c printk macros

On Mon, Jun 25, 2012 at 10:20:00AM -0700, Joe Perches wrote:
> On Mon, 2012-06-25 at 10:17 -0700, Greg KH wrote:
> > On Sat, Jun 23, 2012 at 02:09:41AM -0400, Sherwin Soltani wrote:
> > > Changed to pr_warn, pr_err, and pr_info macros and reformatted some
> > > lines to bring them under the 80 column limit in the binder.c code.
> > >
> > > Signed-off-by: Sherwin Soltani <[email protected]>
> > > ---
> > > drivers/staging/android/binder.c | 74 ++++++++++++++++++++------------------
> > > 1 file changed, 40 insertions(+), 34 deletions(-)
> >
> > Hi,
> >
> > This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
> > a patch that has triggered this response. He used to manually respond
> > to these common problems, but in order to save his sanity (he kept
> > writing the same thing over and over, yet to different people), I was
> > created. Hopefully you will not take offence and will fix the problem
> > in your patch and resubmit it so that it can be accepted into the Linux
> > kernel tree.
>
> > You are receiving this message because of the following common error(s)
> > as indicated below:
> >
> > --- Your patch did many different things all at once, making it
> > difficult to review.
>
> Your patch-bot needs work.
>
> Converting printks to pr_<level> is trivial
> and should be done all at once, not piecemeal.

I'm not disagreeing with that, unfortunately this patch also did more
than just pr_<level> conversions, which is why my patch-bot kicked in.

So your patch-bot complaint generator needs work :)

greg k-h

2012-06-25 17:30:10

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] drivers: staging: android: fix binder.c printk macros

On Mon, 2012-06-25 at 10:24 -0700, Greg KH wrote:
> On Mon, Jun 25, 2012 at 10:20:00AM -0700, Joe Perches wrote:
> > Your patch-bot needs work.
> >
> > Converting printks to pr_<level> is trivial
> > and should be done all at once, not piecemeal.
>
> I'm not disagreeing with that, unfortunately this patch also did more
> than just pr_<level> conversions, which is why my patch-bot kicked in.
>
> So your patch-bot complaint generator needs work :)

Or not. I think wholesale whitespace changes are just
fine too.

git diff -w can ignore all those things pretty easily.

Using a bot is certainly sensible and sanity-preserving.
I think it just needs a little tweaking.

cheers, Joe

2012-06-25 17:45:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] drivers: staging: android: fix binder.c printk macros

On Mon, Jun 25, 2012 at 10:30:08AM -0700, Joe Perches wrote:
> On Mon, 2012-06-25 at 10:24 -0700, Greg KH wrote:
> > On Mon, Jun 25, 2012 at 10:20:00AM -0700, Joe Perches wrote:
> > > Your patch-bot needs work.
> > >
> > > Converting printks to pr_<level> is trivial
> > > and should be done all at once, not piecemeal.
> >
> > I'm not disagreeing with that, unfortunately this patch also did more
> > than just pr_<level> conversions, which is why my patch-bot kicked in.
> >
> > So your patch-bot complaint generator needs work :)
>
> Or not. I think wholesale whitespace changes are just
> fine too.

That's nice, but you aren't the one accepting these patches, sorry.

Also, again, I don't want to see whitespace changes at the same time as
other changes like this, as it makes things harder to review.

> git diff -w can ignore all those things pretty easily.

Ok, but you can't "ignore" those changes before you apply them, which is
what I have to do in reviewing patches.

> Using a bot is certainly sensible and sanity-preserving.
> I think it just needs a little tweaking.

Again, the bot worked properly here, there was more than one logical
change in the patch, so it needs to be split up. One for the pr_<level>
chanegs, and one for the "lines greater than 80 columns" changes.

If the bot annoys you so much, you can filter it away, or, if others
complain, I can have it not post to mailing lists and only respond to
the original author, but that's generally rude for having a
conversation.

thanks,

greg k-h