Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754088AbcDVJGx (ORCPT ); Fri, 22 Apr 2016 05:06:53 -0400 Received: from www.linutronix.de ([62.245.132.108]:52397 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753857AbcDVJGs (ORCPT ); Fri, 22 Apr 2016 05:06:48 -0400 Date: Fri, 22 Apr 2016 11:05:05 +0200 (CEST) From: Thomas Gleixner To: "Du, Changbin" cc: Andrew Morton , Jonathan Corbet , "Paul E. McKenney" , josh@joshtriplett.org, Steven Rostedt , Mathieu Desnoyers , jiangshanlai@gmail.com, John Stultz , Tejun Heo , borntraeger@de.ibm.com, dchinner@redhat.com, linux-doc@vger.kernel.org, LKML Subject: Re: [PATCH 2/7] debugobjects: correct the usage of fixup call results In-Reply-To: <1461312468-14335-3-git-send-email-changbin.du@intel.com> Message-ID: References: <1461312468-14335-1-git-send-email-changbin.du@intel.com> <1461312468-14335-3-git-send-email-changbin.du@intel.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001,URIBL_BLOCKED=0.001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1722 Lines: 60 On Fri, 22 Apr 2016, changbin.du@intel.com wrote: > From: "Du, Changbin" > > If debug_object_fixup() return non-zero when problem has been > fixed. But the code got it backwards, it taks 0 as fixup > successfully. So fix it. Wrong. > @@ -415,7 +415,7 @@ int debug_object_activate(void *addr, struct debug_obj_descr *descr) > state = obj->state; > raw_spin_unlock_irqrestore(&db->lock, flags); > ret = debug_object_fixup(descr->fixup_activate, addr, state); > - return ret ? -EINVAL : 0; > + return ret ? 0 : -EINVAL; So you need to look at the fixup_activate() callbacks. timer_fixup_activate() The only state handled there is ODEBUG_STATE_NOTAVAILABLE. This can happen for two reasons: 1) timer is statically allocated and initialized. That's a legitimate reason and it does not count as a fixup 2) timer has not been initialized, so we have no idea what to do with it. We set it up with a dummy callback and return 1 because that is a fixup. So the return check for ret != 0 is correct. #2 is invalid hrtimer_fixup_activate() There is not much we can do about it. work_fixup_activate() That's similar to timer_fixup_activate(). We need to handle the statically allocated work gracefully. rcuhead_fixup_activate() Handles the ODEBUG_STATE_NOTAVAILABLE case by tracking it. Not a fixup, returns 0. The other states are invalid and there is not much we can do about that. Returns 1. I agree that this is not really intuitive, but it's correct as it is. I'm happy to take patches which make it simpler to understand. Just blindly changing everything to bool does not fall into that category. Thanks, tglx