All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] kthread: fix possible infinite wait for parking when kthread exits meanwhile
Date: Tue, 25 Oct 2016 17:16:40 +0200	[thread overview]
Message-ID: <20161025151639.GE4326@redhat.com> (raw)
In-Reply-To: <20161025110525.9100-1-roman.penyaev@profitbricks.com>

Well. I was going to ignore this patch, I will leave this to Thomas
anyway...

But can't resist, because I have to admit I dislike this one too ;)

On 10/25, Roman Pen wrote:
>
>  int kthread_park(struct task_struct *k)
>  {
> -	struct kthread *kthread = to_live_kthread_and_get(k);
> +	struct kthread *kthread;
>  	int ret = -ENOSYS;
>  
> -	if (kthread) {
> -		if (!test_bit(KTHREAD_IS_PARKED, &kthread->flags)) {
> +	/*
> +	 * Here we try to be careful and handle the case, when kthread
> +	 * is going to die and will never park.

But why?

IMO we should not complicate this code for the case when the kernel
thread crashes.

> +	do {
> +		kthread = to_live_kthread_and_get(k);
> +		if (!kthread)
> +			break;
> +		if (!__kthread_isparked(kthread)) {
>  			set_bit(KTHREAD_SHOULD_PARK, &kthread->flags);
>  			if (k != current) {
>  				wake_up_process(k);
>  				wait_for_completion(&kthread->parked);
>  			}
>  		}
> +		if (k == current || __kthread_isparked(kthread))
> +			/* The way out */
> +			ret = 0;

And why do we need to restart if __kthread_isparked() is false? In this
case we know that we were woken up by the exiting kthread which won't
park anyway.

Oleg.

  reply	other threads:[~2016-10-25 15:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-25 11:05 [PATCH 1/1] kthread: fix possible infinite wait for parking when kthread exits meanwhile Roman Pen
2016-10-25 15:16 ` Oleg Nesterov [this message]
2016-10-25 17:34   ` Roman Penyaev

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=20161025151639.GE4326@redhat.com \
    --to=oleg@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=roman.penyaev@profitbricks.com \
    --cc=tglx@linutronix.de \
    --cc=tj@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.