Signed-off-by: SeongJae Park <[email protected]>
---
drivers/staging/android/binder.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index eaec1da..b23cbc9 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -2553,8 +2553,6 @@ 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;
- /*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
-
trace_binder_ioctl(cmd, arg);
ret = wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
--
1.8.3.2
The variable `binder_debugfs_dir_entry_root` is declared as static
global variable although it is accessed from init function only. Declare
it as init function's local variable because it would be better to read
and memory efficiency.
Signed-off-by: SeongJae Park <[email protected]>
---
drivers/staging/android/binder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index b23cbc9..78de730 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -49,7 +49,6 @@ static HLIST_HEAD(binder_procs);
static HLIST_HEAD(binder_deferred_list);
static HLIST_HEAD(binder_dead_nodes);
-static struct dentry *binder_debugfs_dir_entry_root;
static struct dentry *binder_debugfs_dir_entry_proc;
static struct binder_node *binder_context_mgr_node;
static kuid_t binder_context_mgr_uid = INVALID_UID;
@@ -3514,6 +3513,7 @@ BINDER_DEBUG_ENTRY(transaction_log);
static int __init binder_init(void)
{
+ struct dentry *binder_debugfs_dir_entry_root;
int ret;
binder_deferred_workqueue = create_singlethread_workqueue("binder");
--
1.8.3.2
On Tue, Feb 18, 2014 at 08:23:26PM +0900, SeongJae Park wrote:
> The variable `binder_debugfs_dir_entry_root` is declared as static
> global variable although it is accessed from init function only. Declare
> it as init function's local variable because it would be better to read
> and memory efficiency.
Are you sure? You now just lost that pointer and it can never be freed.
Not that it ever was, but before it was obvious as to what would need to
be done if the module could ever be made to be unloaded...
As it is, this code is fine, I can't take this patch.
greg k-h
On Tue, Feb 18, 2014 at 08:23:25PM +0900, SeongJae Park wrote:
> Signed-off-by: SeongJae Park <[email protected]>
> ---
> drivers/staging/android/binder.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
> index eaec1da..b23cbc9 100644
> --- a/drivers/staging/android/binder.c
> +++ b/drivers/staging/android/binder.c
> @@ -2553,8 +2553,6 @@ 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;
>
> - /*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
It's useful for debugging, I'll leave it as-is, sorry.
On Wed, Feb 19, 2014 at 2:07 AM, Greg KH <[email protected]> wrote:
> On Tue, Feb 18, 2014 at 08:23:25PM +0900, SeongJae Park wrote:
>> Signed-off-by: SeongJae Park <[email protected]>
>> ---
>> drivers/staging/android/binder.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
>> index eaec1da..b23cbc9 100644
>> --- a/drivers/staging/android/binder.c
>> +++ b/drivers/staging/android/binder.c
>> @@ -2553,8 +2553,6 @@ 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;
>>
>> - /*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
>
> It's useful for debugging, I'll leave it as-is, sorry.
Agree, thank you for quick replying.
Hello,
On Wed, Feb 19, 2014 at 2:07 AM, Greg KH <[email protected]> wrote:
> On Tue, Feb 18, 2014 at 08:23:26PM +0900, SeongJae Park wrote:
>> The variable `binder_debugfs_dir_entry_root` is declared as static
>> global variable although it is accessed from init function only. Declare
>> it as init function's local variable because it would be better to read
>> and memory efficiency.
>
> Are you sure? You now just lost that pointer and it can never be freed.
Oops, you are right.
>
> Not that it ever was, but before it was obvious as to what would need to
> be done if the module could ever be made to be unloaded...
>
> As it is, this code is fine, I can't take this patch.
I agree. Thank you.
>
> greg k-h
On 19 February 2014 05:58, SeongJae Park <[email protected]> wrote:
> On Wed, Feb 19, 2014 at 2:07 AM, Greg KH <[email protected]> wrote:
>> On Tue, Feb 18, 2014 at 08:23:25PM +0900, SeongJae Park wrote:
>>> Signed-off-by: SeongJae Park <[email protected]>
>>> ---
>>> drivers/staging/android/binder.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
>>> index eaec1da..b23cbc9 100644
>>> --- a/drivers/staging/android/binder.c
>>> +++ b/drivers/staging/android/binder.c
>>> @@ -2553,8 +2553,6 @@ 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;
>>>
>>> - /*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
>>
>> It's useful for debugging, I'll leave it as-is, sorry.
Or just convert pr_info to pr_debug and leave uncommented?
--
With warm regards,
Sachin
On Wed, Feb 19, 2014 at 12:04 PM, Sachin Kamat <[email protected]> wrote:
> On 19 February 2014 05:58, SeongJae Park <[email protected]> wrote:
>> On Wed, Feb 19, 2014 at 2:07 AM, Greg KH <[email protected]> wrote:
>>> On Tue, Feb 18, 2014 at 08:23:25PM +0900, SeongJae Park wrote:
>>>> Signed-off-by: SeongJae Park <[email protected]>
>>>> ---
>>>> drivers/staging/android/binder.c | 2 --
>>>> 1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
>>>> index eaec1da..b23cbc9 100644
>>>> --- a/drivers/staging/android/binder.c
>>>> +++ b/drivers/staging/android/binder.c
>>>> @@ -2553,8 +2553,6 @@ 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;
>>>>
>>>> - /*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
>>>
>>> It's useful for debugging, I'll leave it as-is, sorry.
>
> Or just convert pr_info to pr_debug and leave uncommented?
Sounds much better. Thank you for suggestion.
I will send patch with your recommend again soon.
>
> --
> With warm regards,
> Sachin