LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: KY Srinivasan <kys@microsoft.com>
To: Vitaly Kuznetsov <vkuznets@redhat.com>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>
Cc: Haiyang Zhang <haiyangz@microsoft.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Dexuan Cui <decui@microsoft.com>,
	Jason Wang <jasowang@redhat.com>
Subject: RE: [PATCH 0/4] Drivers: hv: Further protection for the rescind path
Date: Wed, 4 Feb 2015 18:26:26 +0000	[thread overview]
Message-ID: <BY2PR0301MB0711245C96E6B0AF47C74160A03A0@BY2PR0301MB0711.namprd03.prod.outlook.com> (raw)
In-Reply-To: <1422982839-3948-1-git-send-email-vkuznets@redhat.com>



> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Tuesday, February 3, 2015 9:01 AM
> To: KY Srinivasan; devel@linuxdriverproject.org
> Cc: Haiyang Zhang; linux-kernel@vger.kernel.org; Dexuan Cui; Jason Wang
> Subject: [PATCH 0/4] Drivers: hv: Further protection for the rescind path
> 
> This series is a continuation of the "Drivers: hv: vmbus: serialize Offer and
> Rescind offer". I'm trying to address a number of theoretically possible issues
> with rescind offer handling. All these complications come from the fact that a
> rescind offer results in vmbus channel being freed and we must ensure
> nobody still uses it. Instead of introducing new locks I suggest we switch
> channels usage to the get/put workflow.
> 
> The main part of the series is [PATCH 1/4] which introduces the workflow for
> vmbus channels, all other patches fix different corner cases using this
> workflow. I'm not sure all such cases are covered with this series (probably
> not), but in case protection is required in some other places it should become
> relatively easy to add one.
> 
> I did some sanity testing with CONFIG_DEBUG_LOCKDEP=y and nothing
> popped out, however, additional testing would be much appreciated.
> 
> K.Y., Haiyang, I'm not sending this series to netdev@ and linux-scsi@ as it is
> supposed to be applied as a whole, please resend these patches with your
> sign-offs when (and if) we're done with reviews. Thanks!

Vitaly,

Thanks for looking into this issue. While today, rescind offer results in the freeing of the channel, I don't think
that is required. By not freeing up the channel in the rescind path, we can have a safe way to access the channel and
that does not have to involve taking a reference on the channel every time you access it - the get/put workflow in your
patch set. As part of the network performance improvement work, I had eliminated all locks in the receive path by setting
up per-cpu data structures for mapping the relid to channel etc. These set of patches introduces locking/atomic operations
in performance critical code paths to deal with an event that is truly rare - the channel getting rescinded.

All channel messages are handled in a single work context:

vmbus_on_msg_dpc() -> vmbus_onmessage_work()-> Various channel messages [offer, rescind etc.]

So, the rescind message cannot be processed while we are processing the offer message and since an offer
cannot be rescinded before it is offered, offer and rescind are naturally serialized (I think I have patchset in my queue
from you that is trying to solve the concurrent execution of offer and rescind and looking at the code I cannot see how
this can occur).

As part of handling the rescind message, we will just set the channel state to indicate that the offer is rescinded (we can add
the rescind state to the channel states already defined and this will be done under the protection of the channel lock).
The cleanup of the channel and sending of the RELID release message  will only be done in the context of the driver as part of 
driver remove function. I think this should be doable in a way that does not penalize the normal path. If it is ok with you, I will
try to put together a patch along the lines I have described here.

Regards,

K. Y



> 
> Vitaly Kuznetsov (4):
>   Drivers: hv: vmbus: implement get/put usage workflow for vmbus
>     channels
>   Drivers: hv: vmbus: do not lose rescind offer on failure in
>     vmbus_process_offer()
>   Drivers: hv: vmbus: protect vmbus_get_outgoing_channel() against
>     channel removal
>   hyperv: netvsc: improve protection against rescind offer
> 
>  drivers/hv/channel_mgmt.c   | 75
> +++++++++++++++++++++++++++++++++++++--------
>  drivers/hv/connection.c     |  7 +++--
>  drivers/hv/hyperv_vmbus.h   |  4 +++
>  drivers/net/hyperv/netvsc.c | 10 ++++--  drivers/scsi/storvsc_drv.c  |  2 ++
>  include/linux/hyperv.h      | 13 ++++++++
>  6 files changed, 95 insertions(+), 16 deletions(-)
> 
> --
> 1.9.3


  parent reply	other threads:[~2015-02-04 18:26 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-03 17:00 Vitaly Kuznetsov
2015-02-03 17:00 ` [PATCH 1/4] Drivers: hv: vmbus: implement get/put usage workflow for vmbus channels Vitaly Kuznetsov
2015-02-04  8:18   ` Dexuan Cui
2015-02-04  9:32     ` Vitaly Kuznetsov
2015-02-04  9:54       ` Dexuan Cui
2015-02-04  9:14   ` Jason Wang
2015-02-04  9:33     ` Vitaly Kuznetsov
2015-02-03 17:00 ` [PATCH 2/4] Drivers: hv: vmbus: do not lose rescind offer on failure in vmbus_process_offer() Vitaly Kuznetsov
2015-02-04  7:42   ` Dexuan Cui
2015-02-03 17:00 ` [PATCH 3/4] Drivers: hv: vmbus: protect vmbus_get_outgoing_channel() against channel removal Vitaly Kuznetsov
2015-02-04  7:27   ` Dexuan Cui
2015-02-03 17:00 ` [PATCH 4/4] hyperv: netvsc: improve protection against rescind offer Vitaly Kuznetsov
2015-02-04  7:29   ` Dexuan Cui
2015-02-04 18:26 ` KY Srinivasan [this message]
2015-02-05 10:10   ` [PATCH 0/4] Drivers: hv: Further protection for the rescind path Vitaly Kuznetsov
2015-02-05 12:44     ` Dexuan Cui
2015-02-05 22:47       ` KY Srinivasan
2015-02-06 14:53         ` Dexuan Cui
2015-02-07 16:27           ` KY Srinivasan
2015-02-05 15:52     ` KY Srinivasan

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=BY2PR0301MB0711245C96E6B0AF47C74160A03A0@BY2PR0301MB0711.namprd03.prod.outlook.com \
    --to=kys@microsoft.com \
    --cc=decui@microsoft.com \
    --cc=devel@linuxdriverproject.org \
    --cc=haiyangz@microsoft.com \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vkuznets@redhat.com \
    --subject='RE: [PATCH 0/4] Drivers: hv: Further protection for the rescind path' \
    /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

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).