Re: bio linked list corruption.

From	Ingo Molnar <mingo@kernel.org>
Newsgroups	linux.kernel
Subject	Re: bio linked list corruption.
Date	2016-12-06 09:40 +0100
Message-ID	<sLjs6-87p-25@gated-at.bofh.it> (permalink)
References	(5 earlier) <sL5Iu-7AR-11@gated-at.bofh.it> <sL6XT-81-3@gated-at.bofh.it> <sL7TY-KS-7@gated-at.bofh.it> <sL8dj-QU-19@gated-at.bofh.it> <sLj8J-81i-9@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

* Peter Zijlstra <peterz@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

Thread

Re: bio linked list corruption. Vegard Nossum <vegard.nossum@gmail.com> - 2016-12-05 00:10 +0100
  Re: bio linked list corruption. Vegard Nossum <vegard.nossum@gmail.com> - 2016-12-05 18:20 +0100
    Re: bio linked list corruption. Dave Jones <davej@codemonkey.org.uk> - 2016-12-05 18:30 +0100
    Re: bio linked list corruption. Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-05 19:00 +0100
      Re: bio linked list corruption. Vegard Nossum <vegard.nossum@gmail.com> - 2016-12-05 20:20 +0100
        Re: bio linked list corruption. Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-05 21:20 +0100
          Re: bio linked list corruption. Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-05 21:40 +0100
            Re: bio linked list corruption. Vegard Nossum <vegard.nossum@gmail.com> - 2016-12-05 22:40 +0100
              Re: bio linked list corruption. Vegard Nossum <vegard.nossum@gmail.com> - 2016-12-06 09:50 +0100
            Re: bio linked list corruption. Peter Zijlstra <peterz@infradead.org> - 2016-12-06 09:20 +0100
              Re: bio linked list corruption. Ingo Molnar <mingo@kernel.org> - 2016-12-06 09:40 +0100
              Re: bio linked list corruption. Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-06 18:00 +0100
        Re: bio linked list corruption. Vegard Nossum <vegard.nossum@gmail.com> - 2016-12-05 21:20 +0100
  Re: bio linked list corruption. Andy Lutomirski <luto@kernel.org> - 2016-12-05 19:20 +0100
    Re: bio linked list corruption. Vegard Nossum <vegard.nossum@gmail.com> - 2016-12-05 19:30 +0100
    Re: bio linked list corruption. Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-05 19:30 +0100

csiph-web