* Peter Zijlstra <pet...@infradead.org> wrote: > $ git grep DECLARE_WAIT_QUEUE_HEAD_ONSTACK | wc -l > 28
This debug facility looks sensible. A couple of minor suggestions: > --- a/include/linux/wait.h > +++ b/include/linux/wait.h > @@ -39,6 +39,9 @@ struct wait_bit_queue { > struct __wait_queue_head { > spinlock_t lock; > struct list_head task_list; > +#ifdef CONFIG_DEBUG_WAITQUEUE > + int onstack; > +#endif The structure will pack better in the debug-enabled case if 'onstack' is next to 'lock', as spinlock_t is 4 bytes on many architectures. > -#define __WAIT_QUEUE_HEAD_INITIALIZER(name) { > \ > +#ifdef CONFIG_DEBUG_WAITQUEUE > +#define ___WAIT_QUEUE_ONSTACK(onstack) .onstack = (onstack), > +#else > +#define ___WAIT_QUEUE_ONSTACK(onstack) > +#endif Please help readers by showing the internal structure of the definition: #ifdef CONFIG_DEBUG_WAITQUEUE # define ___WAIT_QUEUE_ONSTACK(onstack) .onstack = (onstack), #else # define ___WAIT_QUEUE_ONSTACK(onstack) #endif > +static inline void prepare_debug(wait_queue_head_t *q, wait_queue_t *wait) > +{ > +#ifdef CONFIG_DEBUG_WAITQUEUE > + WARN_ON_ONCE(q->onstack && wait->func == autoremove_wake_function) > +#endif > +} I'd name this debug_waitqueue_check() or such - as the 'prepare' is a bit misleadig (we don't prepare debugging, we do the debug check here). > +config DEBUG_WAITQUEUE > + bool "Debug waitqueue" > + depends on DEBUG_KERNEL I'd name it DEBUG_SCHED_WAITQUEUE=y and I'd also make it depend on CONFIG_DEBUG_SCHED. LGTM otherwise! Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html