On Sun, Sep 29, 2019 at 06:30:24PM +0200, Romain Perier wrote: > Now that all tasklet initializations have been replaced, this updates > the core of the DECLARE_TASKLET macros by passing the pointer of the > tasklet structure as .data, so current static tasklets will continue to > work by deadling with the tasklet_struct pointer in their handler, typo: dealing > without have to change the API of the macro. It also updates all > callbacks of all tasklets statically allocated via DECLARE_TASKLET() for > extracting the the parent data structure of the tasklet by using > from_tasklet(). I think this patch needs to be broken up... the users of DECLARE_TASKLET() shouldn't need to be changed along with DECLARE_TASKLET() since there are casts protecting the arguments. > [...] > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > index f5332ae2dbeb..949bbaeaff0e 100644 > --- a/include/linux/interrupt.h > +++ b/include/linux/interrupt.h > @@ -598,11 +598,14 @@ struct tasklet_struct > unsigned long data; > }; > > +#define TASKLET_DATA_TYPE unsigned long > +#define TASKLET_FUNC_TYPE void (*)(TASKLET_DATA_TYPE) > + > #define DECLARE_TASKLET(name, func, data) \ > -struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(0), func, data } > +struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(0), (TASKLET_FUNC_TYPE)func, (TASKLET_DATA_TYPE)&name } > > #define DECLARE_TASKLET_DISABLED(name, func, data) \ > -struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(1), func, data } > +struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(1), (TASKLET_FUNC_TYPE)func, (TASKLET_DATA_TYPE)&name } > > > enum > @@ -673,9 +676,6 @@ extern void tasklet_kill_immediate(struct tasklet_struct *t, unsigned int cpu); > extern void tasklet_init(struct tasklet_struct *t, > void (*func)(unsigned long), unsigned long data); > > -#define TASKLET_DATA_TYPE unsigned long > -#define TASKLET_FUNC_TYPE void (*)(TASKLET_DATA_TYPE) > - This patch feels like it should be combined with the first one, and only make the changes to the macros. (And keep them one place: we shouldn't have to move them later.) > #define from_tasklet(var, callback_tasklet, tasklet_fieldname) \ > container_of(callback_tasklet, typeof(*var), tasklet_fieldname) > > diff --git a/kernel/backtracetest.c b/kernel/backtracetest.c > index a2a97fa3071b..b5b9e16f0083 100644 > --- a/kernel/backtracetest.c > +++ b/kernel/backtracetest.c > @@ -23,7 +23,7 @@ static void backtrace_test_normal(void) > > static DECLARE_COMPLETION(backtrace_work); > > -static void backtrace_test_irq_callback(unsigned long data) > +static void backtrace_test_irq_callback(struct tasklet_struct *unused) > { > dump_stack(); > complete(&backtrace_work); And all these other changes should be separated out in a different pass. -- Kees Cook