Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Stephen Boyd <swboyd@chromium.org>,
        John Stultz <john.stultz@linaro.org>
Cc:     linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        Tejun Heo <tj@kernel.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Guenter Roeck <groeck@chromium.org>
Subject: Re: [PATCH] timers: Provide a better debugobjects hint for delayed
 works
In-Reply-To: <20220504223148.644228-1-swboyd@chromium.org>
References: <20220504223148.644228-1-swboyd@chromium.org>
Date:   Tue, 10 May 2022 11:20:01 +0200
Message-ID: <87sfphpwvy.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Wed, May 04 2022 at 15:31, Stephen Boyd wrote:
> Provide better information here by special casing delayed works in the
> timer debugobjects hint logic so that the work function is returned
> instead of the timer function delayed_work_timer_fn(). This will help us
> understand what delayed work was pending that got freed, leading to
> faster bug resolutions.

Makes sense.

> ---
> I have an alternative approach which is to treat delayed works with a
> different debug_obj_descr structure but it basically boils down to
> another version of timer debugobjects in the workqueue code. The idea is
> to make the delayed work active once the timer is queued and then
> convert it over from a delayed work descriptor to a work descriptor once
> the timer runs delayed_work_timer_fn() or when we pull it off to flush
> out.

Nah.

>  #include <linux/uaccess.h>
>  #include <asm/unistd.h>
> @@ -617,7 +618,17 @@ static const struct debug_obj_descr timer_debug_descr;
>  
>  static void *timer_debug_hint(void *addr)
>  {
> -	return ((struct timer_list *) addr)->function;
> +	struct timer_list *timer = addr;
> +
> +	if (timer->function == delayed_work_timer_fn) {
> +		struct delayed_work *dwork;
> +
> +		dwork = container_of(timer, struct delayed_work, timer);
> +
> +		return dwork->work.func;
> +	}

The same issue exists for kthread_delayed_work_timer_fn.

So maybe something like the uncompiled/untested below.

Thanks,

        tglx
---
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -638,9 +638,35 @@ static void internal_add_timer(struct ti
 
 static const struct debug_obj_descr timer_debug_descr;
 
+struct timer_hint {
+	void	(*function)(struct timer_list *);
+	long	offset;
+};
+
+#define TIMER_HINT(fn, container, timr, hintfn)			\
+	{							\
+		.function = fn,					\
+		.offset	  = offsetof(container, hintfn) -	\
+			    offsetof(container, timr)	\
+	}
+
+static const struct timer_hint timer_hints[] = {
+	TIMER_HINT(delayed_work_timer_fn,
+		   struct delayed_work, timer, work.func),
+	TIMER_HINT(kthread_delayed_work_timer_fn,
+		   struct kthread_delayed_work, timer, work.func),
+};
+
 static void *timer_debug_hint(void *addr)
 {
-	return ((struct timer_list *) addr)->function;
+	struct timer_list *timer = addr;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(timer_hints); i++) {
+		if (timer_hints[i].function == timer->function)
+			return addr + timer_hints[i].offset;
+	}
+	return timer->function;
 }
 
 static bool timer_is_static_object(void *addr)