netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: labbott@redhat.com
Cc: imbrenda@linux.vnet.ibm.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, labbott@fedoraproject.org
Subject: Re: [PATCHv2] vsock: Fix blocking ops call in prepare_to_wait
Date: Wed, 16 Mar 2016 19:19:24 -0400 (EDT)	[thread overview]
Message-ID: <20160316.191924.1375803074089875897.davem@davemloft.net> (raw)
In-Reply-To: <56E719EA.7040208@redhat.com>

From: Laura Abbott <labbott@redhat.com>
Date: Mon, 14 Mar 2016 13:07:06 -0700

> On 03/14/2016 12:24 PM, David Miller wrote:
>> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>> Date: Fri, 11 Mar 2016 13:39:23 +0100
>>
>>> I think I found a problem with the patch submitted by Laura Abbott
>>> ( https://lkml.org/lkml/2016/2/4/711 ): we might miss wakeups.
>>> Since the condition is not checked between the prepare_to_wait and the
>>> schedule(), if a wakeup happens after the condition is checked but
>>> before
>>> the sleep happens, and we miss it. ( A description of the problem can
>>> be
>>> found here: http://www.makelinux.net/ldd3/chp-6-sect-2 ).
>>>
>>> My solution (see patch below) is to shrink the area influenced by
>>> prepare_to_wait, but keeping the fragile section around the condition,
>>> and
>>> keep the rest of the code in "normal" running state.  This way the
>>> sleep is
>>> correct and the other functions don't need to worry.  The only caveat
>>> here
>>> is that the function(s) called to verify the conditions are really not
>>> allowed to sleep, so if you need synchronization in the backend of
>>> e.g.
>>> vsock_stream_has_space(), you should use spinlocks and not mutexes.
>>>
>>> In case we want to be able to sleep while waiting for conditions, we
>>> can
>>> consider this instead: https://lwn.net/Articles/628628/ .
>>>
>>>
>>> I stumbled on this problem while working on fixing the upcoming virtio
>>> backend for vsock, below is the patch I had prepared, with the
>>> original
>>> message.
>>
>> Can someone please look at this?  Who maintains this code anyways?
>>
> 
> Nobody was listed in MAINTAINERS. I tried cc-ing some of the e-mail
> addresses
> of the original authors (vmware?) when sending the original patch and
> they
> all bounced.

Ok, can you please submit this anew?  Your commit message format was
incorrect, you put the commit message content you wanted in the change
after the --- separater instead of beforehand.

      reply	other threads:[~2016-03-16 23:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-04 18:50 [PATCHv2] vsock: Fix blocking ops call in prepare_to_wait Laura Abbott
2016-02-13 10:59 ` David Miller
2016-03-11 12:39   ` Claudio Imbrenda
2016-03-14 19:24     ` David Miller
2016-03-14 20:07       ` Laura Abbott
2016-03-16 23:19         ` David Miller [this message]

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=20160316.191924.1375803074089875897.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=imbrenda@linux.vnet.ibm.com \
    --cc=labbott@fedoraproject.org \
    --cc=labbott@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@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;
as well as URLs for NNTP newsgroup(s).