Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1384811

Re: [PATCH 2/7] debugobjects: correct the usage of fixup call results

From Thomas Gleixner <tglx@linutronix.de>
Newsgroups linux.kernel
Subject Re: [PATCH 2/7] debugobjects: correct the usage of fixup call results
Date 2016-04-22 11:10 +0200
Message-ID <rqFg7-7ZV-25@gated-at.bofh.it> (permalink)
References <rqEtI-7kx-5@gated-at.bofh.it> <rqEDp-7qZ-27@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, 22 Apr 2016, changbin.du@intel.com wrote:
> From: "Du, Changbin" <changbin.du@intel.com>
> 
> 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


  

      	

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/7] Make debugobjects fixup functions return bool type changbin.du@intel.com - 2016-04-22 10:20 +0200
  [PATCH 3/7] workqueue: update debugobjects fixup callbacks return type changbin.du@intel.com - 2016-04-22 10:30 +0200
  [PATCH 5/7] rcu: update debugobjects fixup callbacks return type changbin.du@intel.com - 2016-04-22 10:30 +0200
  [PATCH 6/7] percpu_counter: update debugobjects fixup callbacks return type changbin.du@intel.com - 2016-04-22 10:30 +0200
  [PATCH 2/7] debugobjects: correct the usage of fixup call results changbin.du@intel.com - 2016-04-22 10:30 +0200
    Re: [PATCH 2/7] debugobjects: correct the usage of fixup call  results Thomas Gleixner <tglx@linutronix.de> - 2016-04-22 11:10 +0200
  [PATCH 7/7] Documentation: update debugobjects doc changbin.du@intel.com - 2016-04-22 10:30 +0200

csiph-web