Hi Nikolay,
Thanks for the comment.
I agree ,like timer , hrtimer we have to mark inactive in destroy function and finally freeing the debug object
after destruction of percpu_counter.
But i am still not sure that this double freeing with same address may create race or not in debug_object list.
Regards
Gaurav
On 4/13/2018 1:12 PM, Nikolay Borisov wrote:
On 13.04.2018 10:32, Kohli, Gaurav wrote:
Hi ,
I have checked below code and it seems we are calling debug_object_free
twice, ideally we should deactivate and later we
have to destroy.
1st call -> percpu_counter_destroy->debug_percpu_counter_deactivate ->
debug_object_free
2nd call ->
debug_object_free
static bool percpu_counter_fixup_free(void *addr, enum debug_obj_state
state)
{
struct percpu_counter *fbc = addr;
switch (state) {
case ODEBUG_STATE_ACTIVE:
percpu_counter_destroy(fbc); -> first call
debug_object_free(fbc, &percpu_counter_debug_descr); 2nd
Having looked at the code I'd say this is indeed buggy. I'd say it
stemmed from same cargo culting since timer_fixup_free follows the same
structure of code, except that in del_timer_sync there is no code which
does debug_object_free. The situation is similar in work_fixup_free.
So at this point I guess the question is whether we want to leave the
debug_object_free call in percpu_counter_fixup_free and just remove
debug_percpu_counter_deactivate and open-code the call to
debug_object_deactivate in percpu_counter_destroy. Or remove the
explicit call in percpu_counter_fixup_free and leave
debug_percpu_counter_deactivate.
In the end it's a matter of style, so perhaps Tejun, as the maintainer,
has the final say what style he prefers. Personally, I'd go for the
former solution so that the percpu follows the style of the rest of the
kernel.
call
return true;
default:
return false;
}
}
We are seeing one issue, where one list contain garbage data in
obj_hash, just before element of that is percpu_counter but
still not sure as it is very difficult to reproduce.
Can some one please review above code or can we remove one instance of
debug_object_free from above code.
Regards
Gaurav
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
