On Sat, 04 Mar 2023 01:14:21 +0100 Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> On Sat, Mar 04 2023 at 01:53, Schspa Shi wrote:
> > Waiman Long <longman@xxxxxxxxxx> writes:
> >>> diff --git a/include/linux/debugobjects.h b/include/linux/debugobjects.h
> >>> index 32444686b6ff4..544a6111b97f6 100644
> >>> --- a/include/linux/debugobjects.h
> >>> +++ b/include/linux/debugobjects.h
> >>> @@ -30,6 +30,7 @@ struct debug_obj {
> >>> enum debug_obj_state state;
> >>> unsigned int astate;
> >>> void *object;
> >>> + bool is_static;
> >>> const struct debug_obj_descr *descr;
> >>> };
> >>
> >> The patch looks reasonable. My main concern is the increase in size of the
> >> debug_obj structure. It is an additional 8 bytes on 64-bit arches. How much will
> >> we save performance-wise by caching it in the debug_obj. Alternatively, you may
> >> pack it within the current size by, maybe, reducing the size of state.
> >>
> >
> > Yes, we can change this to:
> >
> > struct debug_obj {
> > struct hlist_node node;
> > struct {
> > enum debug_obj_state state : 31;
> > bool is_static : 1;
> > };
>
> and thereby making debugobjects even more slower than it is right
> now. Please check the resulting assembly code before proposing quick
> "solutions".
>
> Thanks,
>
> tglx
Add a debug flag to timer instead in order to make timer_is_static_object()
depend no longer on TIMER_ENTRY_STATIC. Then it always gives the correct
answer in every context.
Hillf
+++ y/include/linux/timer.h
@@ -18,6 +18,10 @@ struct timer_list {
void (*function)(struct timer_list *);
u32 flags;
+#ifdef CONFIG_DEBUG_OBJECTS_TIMERS
+ u32 is_static;
+#define TIMER_IS_STATIC_MAGIC 0xFBFBBEEF
+#endif
#ifdef CONFIG_LOCKDEP
struct lockdep_map lockdep_map;
#endif
@@ -73,6 +77,17 @@ struct timer_list {
#define TIMER_TRACE_FLAGMASK (TIMER_MIGRATING | TIMER_DEFERRABLE | TIMER_PINNED | TIMER_IRQSAFE)
+
+#ifdef CONFIG_DEBUG_OBJECTS_TIMERS
+#define __TIMER_INITIALIZER(_function, _flags) { \
+ .entry = { .next = TIMER_ENTRY_STATIC }, \
+ .function = (_function), \
+ .flags = (_flags), \
+ .is_static = TIMER_IS_STATIC_MAGIC, \
+ __TIMER_LOCKDEP_MAP_INITIALIZER( \
+ __FILE__ ":" __stringify(__LINE__)) \
+ }
+#else
#define __TIMER_INITIALIZER(_function, _flags) { \
.entry = { .next = TIMER_ENTRY_STATIC }, \
.function = (_function), \
@@ -80,6 +95,7 @@ struct timer_list {
__TIMER_LOCKDEP_MAP_INITIALIZER( \
__FILE__ ":" __stringify(__LINE__)) \
}
+#endif /* CONFIG_DEBUG_OBJECTS_TIMERS */
#define DEFINE_TIMER(_name, _function) \
struct timer_list _name = \
+++ y/kernel/time/timer.c
@@ -677,8 +677,7 @@ static bool timer_is_static_object(void
{
struct timer_list *timer = addr;
- return (timer->entry.pprev == NULL &&
- timer->entry.next == TIMER_ENTRY_STATIC);
+ return timer->is_static == TIMER_IS_STATIC_MAGIC;
}
/*
@@ -775,6 +774,7 @@ static const struct debug_obj_descr time
static inline void debug_timer_init(struct timer_list *timer)
{
debug_object_init(timer, &timer_debug_descr);
+ timer->is_static = 0;
}
static inline void debug_timer_activate(struct timer_list *timer)
@@ -804,6 +804,7 @@ void init_timer_on_stack_key(struct time
{
debug_object_init_on_stack(timer, &timer_debug_descr);
do_init_timer(timer, func, flags, name, key);
+ timer->is_static = 0;
}
EXPORT_SYMBOL_GPL(init_timer_on_stack_key);
