[PATCH] percpu_counter: Remove debug_object_free call twice

[PATCH] percpu_counter: Remove debug_object_free call twice

[Gaurav Kohli]

During percpu_counter destroy, debug_object_free is calling
twice which may create race. So removing once instance of call
from debug_percpu_counter_deactivate.

Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>

diff --git a/lib/percpu_counter.c b/lib/percpu_counter.c
index c72577e..68ad030 100644
--- a/lib/percpu_counter.c
+++ b/lib/percpu_counter.c
@@ -48,7 +48,6 @@ static inline void debug_percpu_counter_activate(struct percpu_counter *fbc)
 static inline void debug_percpu_counter_deactivate(struct percpu_counter *fbc)
 {
 	debug_object_deactivate(fbc, &percpu_counter_debug_descr);
-	debug_object_free(fbc, &percpu_counter_debug_descr);
 }
 
 #else	/* CONFIG_DEBUG_OBJECTS_PERCPU_COUNTER */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH] percpu_counter: Remove debug_object_free call twice

[Tejun Heo]

On Fri, Apr 13, 2018 at 03:05:03PM +0530, Gaurav Kohli wrote:
> During percpu_counter destroy, debug_object_free is calling
> twice which may create race. So removing once instance of call
> from debug_percpu_counter_deactivate.

I don't quite follow.  Can you please elaborate how it can be called
twice?

Thanks.

-- 
tejun

Re: [PATCH] percpu_counter: Remove debug_object_free call twice

[Kohli, Gaurav]

On 4/17/2018 3:18 AM, Tejun Heo wrote:

> On Fri, Apr 13, 2018 at 03:05:03PM +0530, Gaurav Kohli wrote:
>> During percpu_counter destroy, debug_object_free is calling
>> twice which may create race. So removing once instance of call
>> from debug_percpu_counter_deactivate.
> I don't quite follow.  Can you please elaborate how it can be called
> twice?

Hi Tejun,

In percpu_counter_fixup_free function, first call is percpu_counter_destroy -> debug_percpu_counter_deactivate (this will set inactive and free the debug object as well for percpu counter) -> free_percpu (finally freeing the counters).

Below is the code snippet:

         case DEBUG_STATE_ACTIVE:

                 percpu_counter_destroy(fbc); -> first call

                 debug_object_free(fbc, &percpu_counter_debug_descr); -> this will again call the same debug object free, if somehow counters will reinitialize between these two calls.

                 

We are seeing one race condition issue where one object of db list is corrupted and just before object of that corrupted node is percpu_counter, Still i am not sure the reason of race as it is very

difficult to reproduce. But i have found this during code review. Please correct me, if i misunderstood this.

>
> Thanks.
>
>
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH] percpu_counter: Remove debug_object_free call twice

[Kohli, Gaurav]

Hi ,

Sorry for very late reminder, just wanted to know is below understanding 
of code is wrong?

Regards
Gaurav

On 4/17/2018 11:59 AM, Kohli, Gaurav wrote:
> On 4/17/2018 3:18 AM, Tejun Heo wrote:
> 
>> On Fri, Apr 13, 2018 at 03:05:03PM +0530, Gaurav Kohli wrote:
>>> During percpu_counter destroy, debug_object_free is calling
>>> twice which may create race. So removing once instance of call
>>> from debug_percpu_counter_deactivate.
>> I don't quite follow.  Can you please elaborate how it can be called
>> twice?
> 
> Hi Tejun,
> 
> In percpu_counter_fixup_free function, first call is 
> percpu_counter_destroy -> debug_percpu_counter_deactivate (this will set 
> inactive and free the debug object as well for percpu counter) -> 
> free_percpu (finally freeing the counters).
> 
> Below is the code snippet:
> 
>          case DEBUG_STATE_ACTIVE:
> 
>                  percpu_counter_destroy(fbc); -> first call
> 
>                  debug_object_free(fbc, &percpu_counter_debug_descr); -> 
> this will again call the same debug object free, if somehow counters 
> will reinitialize between these two calls.
> 
> 
> We are seeing one race condition issue where one object of db list is 
> corrupted and just before object of that corrupted node is 
> percpu_counter, Still i am not sure the reason of race as it is very
> 
> difficult to reproduce. But i have found this during code review. Please 
> correct me, if i misunderstood this.
> 
>>
>> Thanks.
>>
>>

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.