mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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


       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®