qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: rusty@rustcorp.com.au, qemu-devel@nongnu.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [Qemu-devel] Re: [PATCH] qemu/virtio: move features to an inline function
Date: Mon, 10 Aug 2009 22:47:53 +0300	[thread overview]
Message-ID: <20090810194753.GA16803@redhat.com> (raw)
In-Reply-To: <4A8076F0.5050700@codemonkey.ws>

On Mon, Aug 10, 2009 at 02:37:20PM -0500, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> devices should have the final say over which virtio features they
>> support. E.g. indirect entries may or may not make sense in the context
>> of virtio-console.  Move the common bits from virtio-pci to an inline
>> function and let each device call it.
>>   
>
> What drove this in vhost?

vhost does not support indirect ring entries and notify on empty yet.

> Normally, the common features are transport features and the devices  
> should have absolutely no knowledge of transport feature (since they're  
> transport dependent).

Good point. But

1. note that with my patch they don't. They call
virtio_get_common_features and that's all.

2. some features may not make sense for some devices. For example, it is
quite possible that indirect ring entries feature improves performance
on block but hurts on net, as net has a similar merged buffers feature.
Someone should try benchmarking with it disabled, and it becomes
possible with my patch.

> IOW, VIRTIO_RING_F_INDIRECT_DESC is meaningless to virtio-console  
> because virtio-console has no idea what the ring implementation is that  
> it sits on top of.

At some level.  Same can be said to be true for virtio-pci: it sits
below the ring implementation. However, it is not *completely*
meaningless: some devices may benefit from indirect entries, others may
not, or may benefit from disabling them.

> Regards,
>
> Anthony Liguori
>

  reply	other threads:[~2009-08-10 19:49 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-10 19:28 [Qemu-devel] [PATCH] qemu/virtio: move features to an inline function Michael S. Tsirkin
2009-08-10 19:37 ` [Qemu-devel] " Anthony Liguori
2009-08-10 19:47   ` Michael S. Tsirkin [this message]
2009-08-10 20:33     ` Anthony Liguori
2009-08-10 22:19       ` Michael S. Tsirkin
2009-08-10 22:35         ` Anthony Liguori
2009-08-11  8:48           ` Michael S. Tsirkin
2009-08-11 13:15             ` Anthony Liguori
2009-08-11 13:43               ` Michael S. Tsirkin
2009-08-11 16:08                 ` Anthony Liguori
2009-08-11 16:25                   ` Michael S. Tsirkin
2009-08-12 19:18                     ` Anthony Liguori
2009-08-13  6:15                       ` Michael S. Tsirkin
2009-08-13  9:28                         ` Avi Kivity
2009-08-13  9:51                           ` 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=20090810194753.GA16803@redhat.com \
    --to=mst@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=qemu-devel@nongnu.org \
    --cc=rusty@rustcorp.com.au \
    --cc=virtualization@lists.linux-foundation.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).