From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752764AbcEGMrW (ORCPT ); Sat, 7 May 2016 08:47:22 -0400 Received: from e17.ny.us.ibm.com ([129.33.205.207]:54442 "EHLO e17.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752585AbcEGMrU (ORCPT ); Sat, 7 May 2016 08:47:20 -0400 X-IBM-Helo: d01dlp01.pok.ibm.com X-IBM-MailFrom: paulmck@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Date: Sat, 7 May 2016 05:47:16 -0700 From: "Paul E. McKenney" To: Thomas Gleixner Cc: "Du, Changbin" , Andrew Morton , 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 Message-ID: <20160507124716.GM3593@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <1462576157-14539-1-git-send-email-changbin.du@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16050712-0041-0000-0000-000004138838 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, May 07, 2016 at 10:26:28AM +0200, Thomas Gleixner wrote: > 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. The transformation to the RCU code looks fine. So given changes so that Thomas is good with the overall change, I am good with it from an RCU perspective. Thanx, Paul