All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tiwei Bie <tiwei.bie@intel.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>,
	virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	virtio-dev@lists.oasis-open.org, wexu@redhat.com,
	jfreimann@redhat.com
Subject: Re: [virtio-dev] Re: [PATCH net-next v2 0/5] virtio: support packed ring
Date: Thu, 11 Oct 2018 20:12:21 +0800	[thread overview]
Message-ID: <20181011121221.GA27106@debian> (raw)
In-Reply-To: <20181010103335-mutt-send-email-mst@kernel.org>

On Wed, Oct 10, 2018 at 10:36:26AM -0400, Michael S. Tsirkin wrote:
> On Thu, Sep 13, 2018 at 05:47:29PM +0800, Jason Wang wrote:
> > On 2018年09月13日 16:59, Tiwei Bie wrote:
> > > > If what you say is true then we should take a careful look
> > > > and not supporting these generic things with packed layout.
> > > > Once we do support them it will be too late and we won't
> > > > be able to get performance back.
> > > I think it's a good point that we don't need to support
> > > everything in packed ring (especially these which would
> > > hurt the performance), as the packed ring aims at high
> > > performance. I'm also wondering about the features. Is
> > > there any possibility that we won't support the out of
> > > order processing (at least not by default) in packed ring?
> > > If I didn't miss anything, the need to support out of order
> > > processing in packed ring will make the data structure
> > > inside the driver not cache friendly which is similar to
> > > the case of the descriptor table in the split ring (the
> > > difference is that, it only happens in driver now).
> > 
> > Out of order is not the only user, DMA is another one. We don't have used
> > ring(len), so we need to maintain buffer length somewhere even for in order
> > device.
> 
> For a bunch of systems dma unmap is a nop so we do not really
> need to maintain it. It's a question of an API to detect that
> and optimize for it. I posted a proposed patch for that -
> want to try using that?

Yeah, definitely!

> 
> > But if it's not too late, I second for a OUT_OF_ORDER feature.
> > Starting from in order can have much simpler code in driver.
> > 
> > Thanks
> 
> It's tricky to change the flag polarity because of compatibility
> with legacy interfaces. Why is this such a big deal?
> 
> Let's teach drivers about IN_ORDER, then if devices
> are in order it will get enabled by default.

Yeah, make sense.

Besides, I have done some further profiling and debugging
both in kernel driver and DPDK vhost. Previously I was mislead
by a bug in vhost code. I will send a patch to fix that bug.
With that bug fixed, the performance of packed ring in the
test between kernel driver and DPDK vhost is better now.
I will send a new series soon. Thanks!

> 
> -- 
> MST

WARNING: multiple messages have this Message-ID (diff)
From: Tiwei Bie <tiwei.bie@intel.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>,
	virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	virtio-dev@lists.oasis-open.org, wexu@redhat.com,
	jfreimann@redhat.com
Subject: Re: Re: [PATCH net-next v2 0/5] virtio: support packed ring
Date: Thu, 11 Oct 2018 20:12:21 +0800	[thread overview]
Message-ID: <20181011121221.GA27106@debian> (raw)
In-Reply-To: <20181010103335-mutt-send-email-mst@kernel.org>

On Wed, Oct 10, 2018 at 10:36:26AM -0400, Michael S. Tsirkin wrote:
> On Thu, Sep 13, 2018 at 05:47:29PM +0800, Jason Wang wrote:
> > On 2018年09月13日 16:59, Tiwei Bie wrote:
> > > > If what you say is true then we should take a careful look
> > > > and not supporting these generic things with packed layout.
> > > > Once we do support them it will be too late and we won't
> > > > be able to get performance back.
> > > I think it's a good point that we don't need to support
> > > everything in packed ring (especially these which would
> > > hurt the performance), as the packed ring aims at high
> > > performance. I'm also wondering about the features. Is
> > > there any possibility that we won't support the out of
> > > order processing (at least not by default) in packed ring?
> > > If I didn't miss anything, the need to support out of order
> > > processing in packed ring will make the data structure
> > > inside the driver not cache friendly which is similar to
> > > the case of the descriptor table in the split ring (the
> > > difference is that, it only happens in driver now).
> > 
> > Out of order is not the only user, DMA is another one. We don't have used
> > ring(len), so we need to maintain buffer length somewhere even for in order
> > device.
> 
> For a bunch of systems dma unmap is a nop so we do not really
> need to maintain it. It's a question of an API to detect that
> and optimize for it. I posted a proposed patch for that -
> want to try using that?

Yeah, definitely!

> 
> > But if it's not too late, I second for a OUT_OF_ORDER feature.
> > Starting from in order can have much simpler code in driver.
> > 
> > Thanks
> 
> It's tricky to change the flag polarity because of compatibility
> with legacy interfaces. Why is this such a big deal?
> 
> Let's teach drivers about IN_ORDER, then if devices
> are in order it will get enabled by default.

Yeah, make sense.

Besides, I have done some further profiling and debugging
both in kernel driver and DPDK vhost. Previously I was mislead
by a bug in vhost code. I will send a patch to fix that bug.
With that bug fixed, the performance of packed ring in the
test between kernel driver and DPDK vhost is better now.
I will send a new series soon. Thanks!

> 
> -- 
> MST

WARNING: multiple messages have this Message-ID (diff)
From: Tiwei Bie <tiwei.bie@intel.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>,
	virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	virtio-dev@lists.oasis-open.org, wexu@redhat.com,
	jfreimann@redhat.com
Subject: Re: [virtio-dev] Re: [PATCH net-next v2 0/5] virtio: support packed ring
Date: Thu, 11 Oct 2018 20:12:21 +0800	[thread overview]
Message-ID: <20181011121221.GA27106@debian> (raw)
In-Reply-To: <20181010103335-mutt-send-email-mst@kernel.org>

On Wed, Oct 10, 2018 at 10:36:26AM -0400, Michael S. Tsirkin wrote:
> On Thu, Sep 13, 2018 at 05:47:29PM +0800, Jason Wang wrote:
> > On 2018年09月13日 16:59, Tiwei Bie wrote:
> > > > If what you say is true then we should take a careful look
> > > > and not supporting these generic things with packed layout.
> > > > Once we do support them it will be too late and we won't
> > > > be able to get performance back.
> > > I think it's a good point that we don't need to support
> > > everything in packed ring (especially these which would
> > > hurt the performance), as the packed ring aims at high
> > > performance. I'm also wondering about the features. Is
> > > there any possibility that we won't support the out of
> > > order processing (at least not by default) in packed ring?
> > > If I didn't miss anything, the need to support out of order
> > > processing in packed ring will make the data structure
> > > inside the driver not cache friendly which is similar to
> > > the case of the descriptor table in the split ring (the
> > > difference is that, it only happens in driver now).
> > 
> > Out of order is not the only user, DMA is another one. We don't have used
> > ring(len), so we need to maintain buffer length somewhere even for in order
> > device.
> 
> For a bunch of systems dma unmap is a nop so we do not really
> need to maintain it. It's a question of an API to detect that
> and optimize for it. I posted a proposed patch for that -
> want to try using that?

Yeah, definitely!

> 
> > But if it's not too late, I second for a OUT_OF_ORDER feature.
> > Starting from in order can have much simpler code in driver.
> > 
> > Thanks
> 
> It's tricky to change the flag polarity because of compatibility
> with legacy interfaces. Why is this such a big deal?
> 
> Let's teach drivers about IN_ORDER, then if devices
> are in order it will get enabled by default.

Yeah, make sense.

Besides, I have done some further profiling and debugging
both in kernel driver and DPDK vhost. Previously I was mislead
by a bug in vhost code. I will send a patch to fix that bug.
With that bug fixed, the performance of packed ring in the
test between kernel driver and DPDK vhost is better now.
I will send a new series soon. Thanks!

> 
> -- 
> MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


  parent reply	other threads:[~2018-10-11 12:13 UTC|newest]

Thread overview: 167+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-11  2:27 [PATCH net-next v2 0/5] virtio: support packed ring Tiwei Bie
2018-07-11  2:27 ` [virtio-dev] " Tiwei Bie
2018-07-11  2:27 ` Tiwei Bie
2018-07-11  2:27 ` [PATCH net-next v2 1/5] virtio: add packed ring definitions Tiwei Bie
2018-07-11  2:27   ` [virtio-dev] " Tiwei Bie
2018-09-07 13:51   ` Michael S. Tsirkin
2018-09-07 13:51     ` [virtio-dev] " Michael S. Tsirkin
2018-09-10  2:13     ` Tiwei Bie
2018-09-10  2:13       ` [virtio-dev] " Tiwei Bie
2018-09-12 12:53       ` Michael S. Tsirkin
2018-09-12 12:53       ` Michael S. Tsirkin
2018-09-12 12:53         ` [virtio-dev] " Michael S. Tsirkin
2018-09-10  2:13     ` Tiwei Bie
2018-09-07 13:51   ` Michael S. Tsirkin
2018-07-11  2:27 ` Tiwei Bie
2018-07-11  2:27 ` [PATCH net-next v2 2/5] virtio_ring: support creating packed ring Tiwei Bie
2018-07-11  2:27   ` [virtio-dev] " Tiwei Bie
2018-09-07 14:03   ` Michael S. Tsirkin
2018-09-07 14:03   ` Michael S. Tsirkin
2018-09-07 14:03     ` [virtio-dev] " Michael S. Tsirkin
2018-09-10  2:28     ` Tiwei Bie
2018-09-10  2:28       ` [virtio-dev] " Tiwei Bie
2018-09-12  7:51       ` Tiwei Bie
2018-09-12  7:51       ` Tiwei Bie
2018-09-12  7:51         ` [virtio-dev] " Tiwei Bie
2018-09-12 16:12         ` Michael S. Tsirkin
2018-09-12 16:12         ` Michael S. Tsirkin
2018-09-12 16:12           ` [virtio-dev] " Michael S. Tsirkin
2018-09-12 12:45       ` Michael S. Tsirkin
2018-09-12 12:45         ` [virtio-dev] " Michael S. Tsirkin
2018-09-12 12:45       ` Michael S. Tsirkin
2018-09-10  2:28     ` Tiwei Bie
2018-07-11  2:27 ` Tiwei Bie
2018-07-11  2:27 ` [PATCH net-next v2 3/5] virtio_ring: add packed ring support Tiwei Bie
2018-07-11  2:27   ` [virtio-dev] " Tiwei Bie
2018-09-07 13:49   ` Michael S. Tsirkin
2018-09-07 13:49   ` Michael S. Tsirkin
2018-09-07 13:49     ` [virtio-dev] " Michael S. Tsirkin
2018-09-10  2:03     ` Tiwei Bie
2018-09-10  2:03       ` [virtio-dev] " Tiwei Bie
2018-09-10  2:03       ` Tiwei Bie
2018-09-12 16:22   ` Michael S. Tsirkin
2018-09-12 16:22     ` [virtio-dev] " Michael S. Tsirkin
2018-09-12 16:22   ` Michael S. Tsirkin
2018-11-07 17:48   ` Michael S. Tsirkin
2018-11-07 17:48     ` [virtio-dev] " Michael S. Tsirkin
2018-11-08  1:38     ` Tiwei Bie
2018-11-08  1:38     ` Tiwei Bie
2018-11-08  1:38       ` [virtio-dev] " Tiwei Bie
2018-11-08  8:18       ` Jason Wang
2018-11-08  8:18       ` Jason Wang
2018-11-08  8:18         ` [virtio-dev] " Jason Wang
2018-11-08 11:51         ` Tiwei Bie
2018-11-08 11:51           ` [virtio-dev] " Tiwei Bie
2018-11-08 15:56           ` Michael S. Tsirkin
2018-11-08 15:56           ` Michael S. Tsirkin
2018-11-08 15:56             ` [virtio-dev] " Michael S. Tsirkin
2018-11-09  1:50             ` Tiwei Bie
2018-11-09  1:50             ` Tiwei Bie
2018-11-09  1:50               ` [virtio-dev] " Tiwei Bie
2018-11-09  2:30             ` Jason Wang
2018-11-09  2:30             ` Jason Wang
2018-11-09  2:30               ` [virtio-dev] " Jason Wang
2018-11-09  4:00               ` Michael S. Tsirkin
2018-11-09  4:00                 ` [virtio-dev] " Michael S. Tsirkin
2018-11-09 10:05                 ` Jason Wang
2018-11-09 10:05                   ` [virtio-dev] " Jason Wang
2018-11-09 10:05                 ` Jason Wang
2018-11-09  4:00               ` Michael S. Tsirkin
2018-11-08 11:51         ` Tiwei Bie
2018-11-08 14:14         ` Michael S. Tsirkin
2018-11-08 14:14         ` Michael S. Tsirkin
2018-11-08 14:14           ` [virtio-dev] " Michael S. Tsirkin
2018-11-09  2:25           ` Jason Wang
2018-11-09  2:25             ` [virtio-dev] " Jason Wang
2018-11-09  3:58             ` Michael S. Tsirkin
2018-11-09  3:58             ` Michael S. Tsirkin
2018-11-09  3:58               ` [virtio-dev] " Michael S. Tsirkin
2018-11-09 10:04               ` Jason Wang
2018-11-09 10:04                 ` [virtio-dev] " Jason Wang
2018-11-09 10:04                 ` Jason Wang
2018-11-09  2:25           ` Jason Wang
2018-11-07 17:48   ` Michael S. Tsirkin
2018-07-11  2:27 ` Tiwei Bie
2018-07-11  2:27 ` [PATCH net-next v2 4/5] virtio_ring: add event idx support in packed ring Tiwei Bie
2018-07-11  2:27   ` [virtio-dev] " Tiwei Bie
2018-09-07 14:10   ` Michael S. Tsirkin
2018-09-07 14:10   ` Michael S. Tsirkin
2018-09-07 14:10     ` [virtio-dev] " Michael S. Tsirkin
2018-09-10  2:35     ` Tiwei Bie
2018-09-10  2:35     ` Tiwei Bie
2018-09-10  2:35       ` Tiwei Bie
2018-09-10  2:35       ` Tiwei Bie
2018-07-11  2:27 ` Tiwei Bie
2018-07-11  2:27 ` [PATCH net-next v2 5/5] virtio_ring: enable " Tiwei Bie
2018-07-11  2:27 ` Tiwei Bie
2018-07-11  2:27   ` [virtio-dev] " Tiwei Bie
2018-07-11  2:52 ` [PATCH net-next v2 0/5] virtio: support " Jason Wang
2018-07-11  2:52 ` Jason Wang
2018-07-11  2:52   ` [virtio-dev] " Jason Wang
2018-07-12 21:44 ` David Miller
2018-07-12 21:44   ` David Miller
2018-07-13  0:52   ` Jason Wang
2018-07-13  0:52   ` Jason Wang
2018-07-13  0:52     ` [virtio-dev] " Jason Wang
2018-07-13  3:26   ` Michael S. Tsirkin
2018-07-13  3:26     ` [virtio-dev] " Michael S. Tsirkin
2018-07-13  3:26   ` Michael S. Tsirkin
2018-08-27 14:00 ` Michael S. Tsirkin
2018-08-27 14:00 ` Michael S. Tsirkin
2018-08-27 14:00   ` [virtio-dev] " Michael S. Tsirkin
2018-08-28  5:51   ` Jens Freimann
2018-09-07  1:22   ` Tiwei Bie
2018-09-07  1:22   ` Tiwei Bie
2018-09-07  1:22     ` Tiwei Bie
2018-09-07  1:22     ` Tiwei Bie
2018-09-07 13:00     ` [virtio-dev] " Michael S. Tsirkin
2018-09-07 13:00     ` Michael S. Tsirkin
2018-09-07 13:00       ` Michael S. Tsirkin
2018-09-07 13:00       ` Michael S. Tsirkin
2018-09-10  3:00       ` [virtio-dev] " Tiwei Bie
2018-09-10  3:00       ` Tiwei Bie
2018-09-10  3:00         ` Tiwei Bie
2018-09-10  3:00         ` Tiwei Bie
2018-09-10  3:33         ` [virtio-dev] " Jason Wang
2018-09-10  3:33           ` Jason Wang
2018-09-11  5:37           ` Tiwei Bie
2018-09-11  5:37           ` Tiwei Bie
2018-09-11  5:37             ` Tiwei Bie
2018-09-11  5:37             ` Tiwei Bie
2018-09-12 16:16             ` [virtio-dev] " Michael S. Tsirkin
2018-09-12 16:16               ` Michael S. Tsirkin
2018-09-13  8:59               ` Tiwei Bie
2018-09-13  8:59                 ` Tiwei Bie
2018-09-13  8:59                 ` Tiwei Bie
2018-09-13  9:47                 ` [virtio-dev] " Jason Wang
2018-09-13  9:47                   ` Jason Wang
2018-09-13  9:47                   ` Jason Wang
2018-10-10 14:36                   ` [virtio-dev] " Michael S. Tsirkin
2018-10-10 14:36                     ` Michael S. Tsirkin
2018-10-11 12:12                     ` Tiwei Bie
2018-10-11 12:12                     ` Tiwei Bie [this message]
2018-10-11 12:12                       ` Tiwei Bie
2018-10-11 12:12                       ` Tiwei Bie
2018-10-11 13:48                       ` [virtio-dev] " Michael S. Tsirkin
2018-10-11 13:48                       ` Michael S. Tsirkin
2018-10-11 13:48                         ` Michael S. Tsirkin
2018-10-11 14:13                         ` Tiwei Bie
2018-10-11 14:13                           ` Tiwei Bie
2018-10-11 14:17                           ` Michael S. Tsirkin
2018-10-11 14:17                             ` Michael S. Tsirkin
2018-10-11 14:17                             ` Michael S. Tsirkin
2018-10-11 14:34                             ` [virtio-dev] " Tiwei Bie
2018-10-11 14:34                               ` Tiwei Bie
2018-10-11 14:34                               ` Tiwei Bie
2018-10-11 14:34                             ` [virtio-dev] " Tiwei Bie
2018-10-11 14:17                           ` Michael S. Tsirkin
2018-10-11 14:13                         ` Tiwei Bie
2018-10-10 14:36                   ` Michael S. Tsirkin
2018-09-13  9:47                 ` Jason Wang
2018-09-13  8:59               ` Tiwei Bie
2018-09-12 16:16             ` Michael S. Tsirkin
2018-09-10  3:33         ` Jason Wang
2018-09-12 13:06         ` Michael S. Tsirkin
2018-09-12 13:06         ` Michael S. Tsirkin
2018-09-12 13:06           ` Michael S. Tsirkin
2018-09-12 13:06           ` Michael S. Tsirkin

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=20181011121221.GA27106@debian \
    --to=tiwei.bie@intel.com \
    --cc=jasowang@redhat.com \
    --cc=jfreimann@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=wexu@redhat.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 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.