public inbox for linux-bcache@vger.kernel.org
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Coly Li <colyli@suse.de>, linux-bcache@vger.kernel.org
Cc: linux-block@vger.kernel.org, stable@vger.kernel.org,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: Re: [PATCH] bcache: set task state correctly in allocator_wait()
Date: Wed, 22 Nov 2017 15:10:51 +0100	[thread overview]
Message-ID: <0c8e78ab-e369-a40c-ee02-6a6c8f3a5cdd@suse.de> (raw)
In-Reply-To: <20171122123349.74347-1-colyli@suse.de>

On 11/22/2017 01:33 PM, Coly Li wrote:
> Kthread function bch_allocator_thread() references allocator_wait(ca, cond)
> and when kthread_should_stop() is true, this kthread exits.
> 
> The problem is, if kthread_should_stop() is true, macro allocator_wait()
> calls "return 0" with current task state TASK_INTERRUPTIBLE. After function
> bch_allocator_thread() returns to do_exit(), there are some blocking
> operations are called, then a kenrel warning is popped up by __might_sleep
> from kernel/sched/core.c,
>   "WARNING: do not call blocking ops when !TASK_RUNNING; state=1 set at [xxxx]"
> 
> If the task is interrupted and preempted out, since its status is
> TASK_INTERRUPTIBLE, it means scheduler won't pick it back to run forever,
> and the allocator thread may hang in do_exit().
> 
> This patch sets allocator kthread state back to TASK_RUNNING before it
> returns to do_exit(), which avoids a potential deadlock.
> 
> Signed-off-by: Coly Li <colyli@suse.de>
> Cc: stable@vger.kernel.org
> ---
>  drivers/md/bcache/alloc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
> index a27d85232ce1..996ebbabd819 100644
> --- a/drivers/md/bcache/alloc.c
> +++ b/drivers/md/bcache/alloc.c
> @@ -286,9 +286,12 @@ do {									\
>  		if (cond)						\
>  			break;						\
>  									\
> +									\
>  		mutex_unlock(&(ca)->set->bucket_lock);			\
> -		if (kthread_should_stop())				\
> +		if (kthread_should_stop()) {				\
> +			__set_current_state(TASK_RUNNING);		\
>  			return 0;					\
> +		}							\
>  									\
>  		schedule();						\
>  		mutex_lock(&(ca)->set->bucket_lock);			\
> 
_Actually_ there is a push to remove all kthreads in the kernel, as they
don't play nice together with RT.
So while you're at it, do you think it'd be possible to convert it to a
workqueue? Sebastian will be happy to help you here, right, Sebastian?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

  reply	other threads:[~2017-11-22 14:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-22 12:33 [PATCH] bcache: set task state correctly in allocator_wait() Coly Li
2017-11-22 14:10 ` Hannes Reinecke [this message]
2017-11-22 14:55   ` Sebastian Andrzej Siewior
2017-11-22 16:44     ` Coly Li
2017-11-22 17:57   ` Michael Lyle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0c8e78ab-e369-a40c-ee02-6a6c8f3a5cdd@suse.de \
    --to=hare@suse.de \
    --cc=bigeasy@linutronix.de \
    --cc=colyli@suse.de \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox