From: Manfred Spraul <manfred@colorfullife.com>
To: Hillf Danton <hdanton@sina.com>, Davidlohr Bueso <dbueso@suse.de>
Cc: Matthias von Faber <matthias.vonfaber@aox-tech.de>,
Varad Gautam <varad.gautam@suse.com>,
Christian Brauner <christian.brauner@ubuntu.com>,
Oleg Nesterov <oleg@redhat.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Andrew Morton <akpm@linux-foundation.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] ipc/mqueue: avoid sleep after wakeup
Date: Fri, 14 May 2021 17:51:47 +0200 [thread overview]
Message-ID: <7f00542c-748f-e4ff-7596-d18525664811@colorfullife.com> (raw)
In-Reply-To: <20210514030130.3253-1-hdanton@sina.com>
Hi Hillf,
On 5/14/21 5:01 AM, Hillf Danton wrote:
> The pipeline waker could start doing its job once waiter releases lock and
> get the work done before waiter takes a nap, so check wait condition before
> sleep to avoid waiting the wakeup that will never come, though that does not
> hurt much thanks to timer timeouts like a second.
First: The timeout could be infinity, thus the code must not rely on a
timeout wakeup.
A wrong wait is would be a bug.
>
> Check signal for the same reason.
>
> Signed-off-by: Hillf Danton <hdanton@sina.com>
> ---
>
> --- y/ipc/mqueue.c
> +++ x/ipc/mqueue.c
> @@ -710,15 +710,24 @@ static int wq_sleep(struct mqueue_inode_
> __set_current_state(TASK_INTERRUPTIBLE);
>
> spin_unlock(&info->lock);
> - time = schedule_hrtimeout_range_clock(timeout, 0,
> - HRTIMER_MODE_ABS, CLOCK_REALTIME);
>
I do not see a bug:
We do the __set_current_state() while holding the spinlock. If there is
a wakeup, then the wakeup will change current->state to TASK_RUNNING.
schedule() will not remove us from the run queue when current->state is
TASK_RUNNING. The same applies if there are pending signals: schedule()
checks for pending signals and sets current->state to TASK_RUNNING.
Since the __set_current_state() is done while we hold info->lock, and
since the wakeup cannot happen before we have dropped the lock [because
the task that wakes us up needs the same lock], I do not see how a
wakeup could be lost.
Thus: Which issue do you see?
--
Manfred
next parent reply other threads:[~2021-05-14 15:51 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210514030130.3253-1-hdanton@sina.com>
2021-05-14 15:51 ` Manfred Spraul [this message]
[not found] ` <20210515040613.12820-1-hdanton@sina.com>
2021-05-15 5:41 ` Manfred Spraul
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=7f00542c-748f-e4ff-7596-d18525664811@colorfullife.com \
--to=manfred@colorfullife.com \
--cc=akpm@linux-foundation.org \
--cc=christian.brauner@ubuntu.com \
--cc=dbueso@suse.de \
--cc=ebiederm@xmission.com \
--cc=hdanton@sina.com \
--cc=linux-kernel@vger.kernel.org \
--cc=matthias.vonfaber@aox-tech.de \
--cc=oleg@redhat.com \
--cc=varad.gautam@suse.com \
/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
all inboxes | Powered by JetHome®