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.
next prev parent 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.