From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751723AbcEGI2W (ORCPT ); Sat, 7 May 2016 04:28:22 -0400 Received: from www.linutronix.de ([62.245.132.108]:33414 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751011AbcEGI2T (ORCPT ); Sat, 7 May 2016 04:28:19 -0400 Date: Sat, 7 May 2016 10:26:28 +0200 (CEST) From: Thomas Gleixner To: "Du, Changbin" cc: Andrew Morton , "Paul E. McKenney" , josh@joshtriplett.org, Steven Rostedt , Mathieu Desnoyers , jiangshanlai@gmail.com, John Stultz , Tejun Heo , LKML Subject: Re: [PATCH] debugobjects: insulate non-fixup logic related to static obj from fixup callbacks In-Reply-To: <1462576157-14539-1-git-send-email-changbin.du@intel.com> Message-ID: References: <1462576157-14539-1-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 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 7 May 2016, changbin.du@intel.com wrote: Can you please fix your mail client. Every mail you send has: Cc: ..... "Du, Changbin" , Du And that stray 'Du' is just broken. > At last, I have a concern about the fixups that can it change the > object which is in incorrect state on fixup? Because the 'addr' may > not point to any valid object if a non-static object is not tracked. > Then Change such object can overwrite someone's memory and cause > unexpected behaviour. For example, the timer_fixup_activate bind > timer to function stub_timer. Well, you have the choice of: 1) Leave the object uninitialized and watch the resulting explosion 2) Assume that the pointer is a valid object and initialize it The latter has been chosen as the lesser of two evils. > raw_spin_unlock_irqrestore(&db->lock, flags); > /* > - * Maybe the object is static. Let the type specific > + * Maybe the object is static. Let the type specific > * code decide what to do. Instead of doing white space changes you really want to explain the logic here. > */ > - if (debug_object_fixup(descr->fixup_assert_init, addr, > - ODEBUG_STATE_NOTAVAILABLE)) > + if (descr->is_static_object && descr->is_static_object(addr)) { > + /* Make sure that it is tracked in the object tracker */ > + debug_object_init(addr, descr); > + } else { > debug_print_object(&o, "assert_init"); > + debug_object_fixup(descr->fixup_assert_init, addr, > + ODEBUG_STATE_NOTAVAILABLE); > + } > return; > } Other than the missing comment this looks good. Thanks, tglx