* Problem with multiple hvc consoles via virtio-console
From: Amit Shah @ 2010-03-24 15:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Virtualization List
Hello,
When multiple hvc console ports are initialised and used via the
virtio_console driver, I can interact with only the first console, the
2nd one doesn't respond.
If I call hvc_kick() even if hvc_poll() returns 0, the 2nd console
becomes responsive.
I've checked that hvc_poll() receives data via get_chars() and feeds it
to tty_insert_flip_char().
Any idea what could be going wrong? This is reproducible on Linus' git
kernel as the guest kernel with the qemu.git repo.
Any pointers?
Thanks,
Amit
^ permalink raw reply
* Re: [RFC] vhost-blk implementation
From: Michael S. Tsirkin @ 2010-03-24 11:23 UTC (permalink / raw)
To: Badari Pulavarty; +Cc: qemu-devel, virtualization
In-Reply-To: <4BA91C9B.8070900@us.ibm.com>
On Tue, Mar 23, 2010 at 12:55:07PM -0700, Badari Pulavarty wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Mar 23, 2010 at 10:57:33AM -0700, Badari Pulavarty wrote:
>>
>>> Michael S. Tsirkin wrote:
>>>
>>>> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>>>>
>>>>> Write Results:
>>>>> ==============
>>>>>
>>>>> I see degraded IO performance when doing sequential IO write
>>>>> tests with vhost-blk compared to virtio-blk.
>>>>>
>>>>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>>>>
>>>>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>>>>> vhost-blk. Wondering why ?
>>>>>
>>>> Try to look and number of interrupts and/or number of exits.
>>>>
>>> I checked interrupts and IO exits - there is no major noticeable
>>> difference between
>>> vhost-blk and virtio-blk scenerios.
>>>
>>>> It could also be that you are overrunning some queue.
>>>>
>>>> I don't see any exit mitigation strategy in your patch:
>>>> when there are already lots of requests in a queue, it's usually
>>>> a good idea to disable notifications and poll the
>>>> queue as requests complete. That could help performance.
>>>>
>>> Do you mean poll eventfd for new requests instead of waiting for new
>>> notifications ?
>>> Where do you do that in vhost-net code ?
>>>
>>
>> vhost_disable_notify does this.
>>
>>
>>> Unlike network socket, since we are dealing with a file, there is no
>>> ->poll support for it.
>>> So I can't poll for the data. And also, Issue I am having is on the
>>> write() side.
>>>
>>
>> Not sure I understand.
>>
>>
>>> I looked at it some more - I see 512K write requests on the
>>> virtio-queue in both vhost-blk and virtio-blk cases. Both qemu or
>>> vhost is doing synchronous writes to page cache (there is no write
>>> batching in qemu that is affecting this case). I still puzzled on
>>> why virtio-blk outperforms vhost-blk.
>>>
>>> Thanks,
>>> Badari
>>>
>>
>> If you say the number of requests is the same, we are left with:
>> - requests are smaller for some reason?
>> - something is causing retries?
>>
> No. IO requests sizes are exactly same (512K) in both cases. There are
> no retries or
> errors in both cases. One thing I am not clear is - for some reason
> guest kernel
> could push more data into virtio-ring in case of virtio-blk vs
> vhost-blk. Is this possible ?
> Does guest gets to run much sooner in virtio-blk case than vhost-blk ?
> Sorry, if its dumb question -
> I don't understand all the vhost details :(
>
> Thanks,
> Badari
BTW, did you put the backend in non-blocking mode?
As I said, vhost net passes non-blocking flag to
socket backend, but vfs_write/read that you use does
not have an option to do this.
So we'll need to extend the backend to fix that,
but just for demo purposes, you could set non-blocking
mode on the file from userspace.
--
MST
^ permalink raw reply
* Re: [RFC] vhost-blk implementation
From: Michael S. Tsirkin @ 2010-03-24 9:52 UTC (permalink / raw)
To: Badari Pulavarty; +Cc: qemu-devel, virtualization
In-Reply-To: <4BA91C9B.8070900@us.ibm.com>
On Tue, Mar 23, 2010 at 12:55:07PM -0700, Badari Pulavarty wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Mar 23, 2010 at 10:57:33AM -0700, Badari Pulavarty wrote:
>>
>>> Michael S. Tsirkin wrote:
>>>
>>>> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>>>>
>>>>> Write Results:
>>>>> ==============
>>>>>
>>>>> I see degraded IO performance when doing sequential IO write
>>>>> tests with vhost-blk compared to virtio-blk.
>>>>>
>>>>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>>>>
>>>>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>>>>> vhost-blk. Wondering why ?
>>>>>
>>>> Try to look and number of interrupts and/or number of exits.
>>>>
>>> I checked interrupts and IO exits - there is no major noticeable
>>> difference between
>>> vhost-blk and virtio-blk scenerios.
>>>
>>>> It could also be that you are overrunning some queue.
>>>>
>>>> I don't see any exit mitigation strategy in your patch:
>>>> when there are already lots of requests in a queue, it's usually
>>>> a good idea to disable notifications and poll the
>>>> queue as requests complete. That could help performance.
>>>>
>>> Do you mean poll eventfd for new requests instead of waiting for new
>>> notifications ?
>>> Where do you do that in vhost-net code ?
>>>
>>
>> vhost_disable_notify does this.
>>
>>
>>> Unlike network socket, since we are dealing with a file, there is no
>>> ->poll support for it.
>>> So I can't poll for the data. And also, Issue I am having is on the
>>> write() side.
>>>
>>
>> Not sure I understand.
>>
>>
>>> I looked at it some more - I see 512K write requests on the
>>> virtio-queue in both vhost-blk and virtio-blk cases. Both qemu or
>>> vhost is doing synchronous writes to page cache (there is no write
>>> batching in qemu that is affecting this case). I still puzzled on
>>> why virtio-blk outperforms vhost-blk.
>>>
>>> Thanks,
>>> Badari
>>>
>>
>> If you say the number of requests is the same, we are left with:
>> - requests are smaller for some reason?
>> - something is causing retries?
>>
> No. IO requests sizes are exactly same (512K) in both cases. There are
> no retries or
> errors in both cases. One thing I am not clear is - for some reason
> guest kernel
> could push more data into virtio-ring in case of virtio-blk vs
> vhost-blk. Is this possible ?
> Does guest gets to run much sooner in virtio-blk case than vhost-blk ?
> Sorry, if its dumb question -
> I don't understand all the vhost details :(
>
> Thanks,
> Badari
>
You said you observed same number of requests in userspace versus kernel above.
And request size is the same as well. But somehow more data is
transferred? I'm confused.
--
MST
^ permalink raw reply
* Re: [RFC] vhost-blk implementation
From: Badari Pulavarty @ 2010-03-23 19:55 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, virtualization
In-Reply-To: <20100323180642.GA20175@redhat.com>
Michael S. Tsirkin wrote:
> On Tue, Mar 23, 2010 at 10:57:33AM -0700, Badari Pulavarty wrote:
>
>> Michael S. Tsirkin wrote:
>>
>>> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>>>
>>>
>>>> Write Results:
>>>> ==============
>>>>
>>>> I see degraded IO performance when doing sequential IO write
>>>> tests with vhost-blk compared to virtio-blk.
>>>>
>>>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>>>
>>>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>>>> vhost-blk. Wondering why ?
>>>>
>>>>
>>> Try to look and number of interrupts and/or number of exits.
>>>
>>>
>> I checked interrupts and IO exits - there is no major noticeable
>> difference between
>> vhost-blk and virtio-blk scenerios.
>>
>>> It could also be that you are overrunning some queue.
>>>
>>> I don't see any exit mitigation strategy in your patch:
>>> when there are already lots of requests in a queue, it's usually
>>> a good idea to disable notifications and poll the
>>> queue as requests complete. That could help performance.
>>>
>>>
>> Do you mean poll eventfd for new requests instead of waiting for new
>> notifications ?
>> Where do you do that in vhost-net code ?
>>
>
> vhost_disable_notify does this.
>
>
>> Unlike network socket, since we are dealing with a file, there is no
>> ->poll support for it.
>> So I can't poll for the data. And also, Issue I am having is on the
>> write() side.
>>
>
> Not sure I understand.
>
>
>> I looked at it some more - I see 512K write requests on the
>> virtio-queue in both vhost-blk and virtio-blk cases. Both qemu or
>> vhost is doing synchronous writes to page cache (there is no write
>> batching in qemu that is affecting this case). I still puzzled on
>> why virtio-blk outperforms vhost-blk.
>>
>> Thanks,
>> Badari
>>
>
> If you say the number of requests is the same, we are left with:
> - requests are smaller for some reason?
> - something is causing retries?
>
No. IO requests sizes are exactly same (512K) in both cases. There are
no retries or
errors in both cases. One thing I am not clear is - for some reason
guest kernel
could push more data into virtio-ring in case of virtio-blk vs
vhost-blk. Is this possible ?
Does guest gets to run much sooner in virtio-blk case than vhost-blk ?
Sorry, if its dumb question -
I don't understand all the vhost details :(
Thanks,
Badari
^ permalink raw reply
* Re: [RFC] vhost-blk implementation
From: Michael S. Tsirkin @ 2010-03-23 18:06 UTC (permalink / raw)
To: Badari Pulavarty
Cc: Discussion of kvm memory usage, qemu-devel, virtualization
In-Reply-To: <4BA9010D.6010607@us.ibm.com>
On Tue, Mar 23, 2010 at 10:57:33AM -0700, Badari Pulavarty wrote:
> Michael S. Tsirkin wrote:
>> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>>
>>> Write Results:
>>> ==============
>>>
>>> I see degraded IO performance when doing sequential IO write
>>> tests with vhost-blk compared to virtio-blk.
>>>
>>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>>
>>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>>> vhost-blk. Wondering why ?
>>>
>>
>> Try to look and number of interrupts and/or number of exits.
>>
>
> I checked interrupts and IO exits - there is no major noticeable
> difference between
> vhost-blk and virtio-blk scenerios.
>> It could also be that you are overrunning some queue.
>>
>> I don't see any exit mitigation strategy in your patch:
>> when there are already lots of requests in a queue, it's usually
>> a good idea to disable notifications and poll the
>> queue as requests complete. That could help performance.
>>
> Do you mean poll eventfd for new requests instead of waiting for new
> notifications ?
> Where do you do that in vhost-net code ?
vhost_disable_notify does this.
> Unlike network socket, since we are dealing with a file, there is no
> ->poll support for it.
> So I can't poll for the data. And also, Issue I am having is on the
> write() side.
Not sure I understand.
> I looked at it some more - I see 512K write requests on the
> virtio-queue in both vhost-blk and virtio-blk cases. Both qemu or
> vhost is doing synchronous writes to page cache (there is no write
> batching in qemu that is affecting this case). I still puzzled on
> why virtio-blk outperforms vhost-blk.
>
> Thanks,
> Badari
If you say the number of requests is the same, we are left with:
- requests are smaller for some reason?
- something is causing retries?
--
MST
^ permalink raw reply
* Re: [RFC] vhost-blk implementation
From: Badari Pulavarty @ 2010-03-23 17:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Discussion of kvm memory usage, qemu-devel, virtualization
In-Reply-To: <20100323123916.GA24750@redhat.com>
Michael S. Tsirkin wrote:
> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>
>> Write Results:
>> ==============
>>
>> I see degraded IO performance when doing sequential IO write
>> tests with vhost-blk compared to virtio-blk.
>>
>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>
>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>> vhost-blk. Wondering why ?
>>
>
> Try to look and number of interrupts and/or number of exits.
>
I checked interrupts and IO exits - there is no major noticeable
difference between
vhost-blk and virtio-blk scenerios.
> It could also be that you are overrunning some queue.
>
> I don't see any exit mitigation strategy in your patch:
> when there are already lots of requests in a queue, it's usually
> a good idea to disable notifications and poll the
> queue as requests complete. That could help performance.
>
Do you mean poll eventfd for new requests instead of waiting for new
notifications ?
Where do you do that in vhost-net code ?
Unlike network socket, since we are dealing with a file, there is no
->poll support for it.
So I can't poll for the data. And also, Issue I am having is on the
write() side.
I looked at it some more - I see 512K write requests on the virtio-queue
in both
vhost-blk and virtio-blk cases. Both qemu or vhost is doing synchronous
writes
to page cache (there is no write batching in qemu that is affecting this
case).
I still puzzled on why virtio-blk outperforms vhost-blk.
Thanks,
Badari
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH 4/9] virtio-serial: Handle scatter-gather buffers for control messages
From: Michael S. Tsirkin @ 2010-03-23 16:23 UTC (permalink / raw)
To: Amit Shah; +Cc: qemu-devel, virtualization, Avi Kivity, quintela
In-Reply-To: <20100323161508.GF3080@amit-x200.redhat.com>
On Tue, Mar 23, 2010 at 09:45:08PM +0530, Amit Shah wrote:
> On (Tue) Mar 23 2010 [17:51:26], Michael S. Tsirkin wrote:
> > On Mon, Mar 22, 2010 at 10:48:02AM +0530, Amit Shah wrote:
> > > On (Sat) Mar 20 2010 [09:40:50], Avi Kivity wrote:
> > > > On 03/19/2010 01:58 PM, Amit Shah wrote:
> > > >> +
> > > >> + offset = 0;
> > > >> + for (i = 0; i< elem.out_num; i++) {
> > > >> + memcpy(buf + offset, elem.out_sg[i].iov_base,
> > > >> + elem.out_sg[i].iov_len);
> > > >> + offset += elem.out_sg[i].iov_len;
> > > >> + }
> > > >> + len = cur_len;
> > > >> +
> > > >> + handle_control_message(vser, buf, len);
> > > >> + virtqueue_push(vq,&elem, len);
> > > >> + }
> > > >> + if (len) {
> > > >> + qemu_free(buf);
> > > >> }
> > > >> virtio_notify(vdev, vq);
> > > >> }
> > > >
> > > > Isn't there some virtio function to linearize requests?
> > >
> > > I don't see one.
> >
> > virtio-net has iov_fill. Reuse it?
>
> Hm, yeah. Any ideas on how to share it? Put it in some common file?
>
> Just copying it seems good for now..
>
> Amit
Add iov.c
--
MST
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH 4/9] virtio-serial: Handle scatter-gather buffers for control messages
From: Amit Shah @ 2010-03-23 16:15 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, virtualization, Avi Kivity, quintela
In-Reply-To: <20100323155126.GA25772@redhat.com>
On (Tue) Mar 23 2010 [17:51:26], Michael S. Tsirkin wrote:
> On Mon, Mar 22, 2010 at 10:48:02AM +0530, Amit Shah wrote:
> > On (Sat) Mar 20 2010 [09:40:50], Avi Kivity wrote:
> > > On 03/19/2010 01:58 PM, Amit Shah wrote:
> > >> +
> > >> + offset = 0;
> > >> + for (i = 0; i< elem.out_num; i++) {
> > >> + memcpy(buf + offset, elem.out_sg[i].iov_base,
> > >> + elem.out_sg[i].iov_len);
> > >> + offset += elem.out_sg[i].iov_len;
> > >> + }
> > >> + len = cur_len;
> > >> +
> > >> + handle_control_message(vser, buf, len);
> > >> + virtqueue_push(vq,&elem, len);
> > >> + }
> > >> + if (len) {
> > >> + qemu_free(buf);
> > >> }
> > >> virtio_notify(vdev, vq);
> > >> }
> > >
> > > Isn't there some virtio function to linearize requests?
> >
> > I don't see one.
>
> virtio-net has iov_fill. Reuse it?
Hm, yeah. Any ideas on how to share it? Put it in some common file?
Just copying it seems good for now..
Amit
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH 4/9] virtio-serial: Handle scatter-gather buffers for control messages
From: Michael S. Tsirkin @ 2010-03-23 15:51 UTC (permalink / raw)
To: Amit Shah; +Cc: qemu-devel, virtualization, Avi Kivity, quintela
In-Reply-To: <20100322051802.GE24577@amit-x200.redhat.com>
On Mon, Mar 22, 2010 at 10:48:02AM +0530, Amit Shah wrote:
> On (Sat) Mar 20 2010 [09:40:50], Avi Kivity wrote:
> > On 03/19/2010 01:58 PM, Amit Shah wrote:
> >> +
> >> + offset = 0;
> >> + for (i = 0; i< elem.out_num; i++) {
> >> + memcpy(buf + offset, elem.out_sg[i].iov_base,
> >> + elem.out_sg[i].iov_len);
> >> + offset += elem.out_sg[i].iov_len;
> >> + }
> >> + len = cur_len;
> >> +
> >> + handle_control_message(vser, buf, len);
> >> + virtqueue_push(vq,&elem, len);
> >> + }
> >> + if (len) {
> >> + qemu_free(buf);
> >> }
> >> virtio_notify(vdev, vq);
> >> }
> >
> > Isn't there some virtio function to linearize requests?
>
> I don't see one.
>
> Amit
virtio-net has iov_fill. Reuse it?
--
MST
^ permalink raw reply
* Re: Virtio_console usage of early printk
From: François Diakhate @ 2010-03-23 14:58 UTC (permalink / raw)
To: Amit Shah; +Cc: linuxppc-dev, virtualization
In-Reply-To: <20100323085520.GF8675@amit-x200.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
On Tue, Mar 23, 2010 at 9:55 AM, Amit Shah <amit.shah@redhat.com> wrote:
> On (Mon) Mar 22 2010 [18:45:47], François Diakhate wrote:
>> Hi all,
>>
>> As far as I can see, early_put_chars is not used by virtio_console
>> because it checks whether there is a port available before using it.
>> If I understand correctly, this makes it useless because once we have
>> a port, we can use the regular virtio transport to output things to
>> the console. Does the attached patch seem valid ? Feedback from s390
>> and powerpc users who use this functionality would be appreciated.
>>
>> Thanks,
>> François
>
>> From 3961f380bbe84a1036ddfc823039cbee31b44dcb Mon Sep 17 00:00:00 2001
>> From: =?utf-8?q?Fran=C3=A7ois=20Diakhat=C3=A9?= <fdiakh@gmail.com>
>> Date: Thu, 18 Mar 2010 14:48:20 +0100
>> Subject: virtio: console: Fix early_put_chars usage
>>
>> Currently early_put_chars is not used by virtio_console because it can only be used once a port has been found, at which point it's too late because it is no longer needed. This patch should fix it.
>
> You'll have to give a signed-off-by for the patch.
>
> Amit
>
Sorry, I didn't have access to my computer this morning. Here is the
patch with a sign-off, if you still need it.
François
[-- Attachment #2: 0001-virtio-console-Fix-early_put_chars-usage.patch --]
[-- Type: text/x-patch, Size: 1265 bytes --]
From 209c553355a3ac04e68a9e9560cea90897588304 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Fran=C3=A7ois=20Diakhat=C3=A9?= <fdiakh@gmail.com>
Date: Thu, 18 Mar 2010 14:48:20 +0100
Subject: [PATCH] virtio: console: Fix early_put_chars usage
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Currently early_put_chars is not used by virtio_console because it can only be used once a port has been found, at which point it's too late because it is no longer needed. This patch should fix it.
Signed-off-by: François Diakhaté <fdiakh@gmail.com>
---
drivers/char/virtio_console.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index f404ccf..691ba21 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -645,13 +645,13 @@ static int put_chars(u32 vtermno, const char *buf, int count)
{
struct port *port;
+ if (unlikely(early_put_chars))
+ return early_put_chars(vtermno, buf, count);
+
port = find_port_by_vtermno(vtermno);
if (!port)
return 0;
- if (unlikely(early_put_chars))
- return early_put_chars(vtermno, buf, count);
-
return send_buf(port, (void *)buf, count);
}
--
1.6.3.3
[-- Attachment #3: Type: text/plain, Size: 184 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* Re: [RFC] vhost-blk implementation
From: Badari Pulavarty @ 2010-03-23 14:47 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, virtualization
In-Reply-To: <20100323123916.GA24750@redhat.com>
Michael S. Tsirkin wrote:
> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>
>> Write Results:
>> ==============
>>
>> I see degraded IO performance when doing sequential IO write
>> tests with vhost-blk compared to virtio-blk.
>>
>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>
>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>> vhost-blk. Wondering why ?
>>
>
> Try to look and number of interrupts and/or number of exits.
>
> It could also be that you are overrunning some queue.
>
Yeah.
> I don't see any exit mitigation strategy in your patch:
> when there are already lots of requests in a queue, it's usually
> a good idea to disable notifications and poll the
> queue as requests complete. That could help performance.
>
Thanks for the suggestions. I will take a closer look.
Thanks,
Badari
^ permalink raw reply
* [PATCH 8/8] virtio: console: Add partial blocking/non-blocking support to write()
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-8-git-send-email-amit.shah@redhat.com>
If the host port is not open, a write() should either just return if the
file is opened in non-blocking mode, or block till the host port is
opened.
This is just half of the blocking/non-blocking story.
The other half will be addressed when we get rid of the cpu_relax() in
send_buf(). Add a fixme mentioning that case.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index f33ceaa..fa616aa 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -536,6 +536,24 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
port = filp->private_data;
+ if (!port->host_connected) {
+ /*
+ * FIXME: Also check if the vq has enough room to send
+ * this buffer out to the guest. Return EAGAIN or
+ * block in case there's no room. Currently the vq
+ * always has room because we only use one buffer (and
+ * cpu_relax()), but when send_buf() becomes
+ * non-spinning, we'll use more buffers.
+ */
+ if (filp->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(port->waitqueue,
+ port->host_connected);
+ if (ret < 0)
+ return ret;
+ }
+
count = min((size_t)(32 * 1024), count);
buf = kmalloc(count, GFP_KERNEL);
--
1.6.2.5
^ permalink raw reply related
* [PATCH 7/8] virtio: console: Fix early_put_chars usage
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, François Diakhaté, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-7-git-send-email-amit.shah@redhat.com>
From: François Diakhaté <fdiakh@gmail.com>
Currently early_put_chars is not used by virtio_console because it can
only be used once a port has been found, at which point it's too late
because it is no longer needed. This patch should fix it.
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 18b1b06..f33ceaa 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -650,13 +650,13 @@ static int put_chars(u32 vtermno, const char *buf, int count)
{
struct port *port;
+ if (unlikely(early_put_chars))
+ return early_put_chars(vtermno, buf, count);
+
port = find_port_by_vtermno(vtermno);
if (!port)
return 0;
- if (unlikely(early_put_chars))
- return early_put_chars(vtermno, buf, count);
-
return send_buf(port, (void *)buf, count);
}
--
1.6.2.5
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH 6/8] virtio: console: Don't always create a port 0 if using multiport
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-6-git-send-email-amit.shah@redhat.com>
If we're using multiport, there's no point in always creating a console
port. Create the console port only if the host doesn't support
multiport.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 33 +++++++++++++++------------------
1 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8b67ff1..18b1b06 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -783,6 +783,13 @@ int init_port_console(struct port *port)
spin_unlock_irq(&pdrvdata_lock);
port->guest_connected = true;
+ /*
+ * Start using the new console output if this is the first
+ * console to come up.
+ */
+ if (early_put_chars)
+ early_put_chars = NULL;
+
/* Notify host of port being opened */
send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
@@ -1044,14 +1051,8 @@ static void handle_control_message(struct ports_device *portdev,
switch (cpkt->event) {
case VIRTIO_CONSOLE_PORT_ADD:
if (port) {
- /*
- * This can happen for port 0: we have to
- * create a console port during probe() as was
- * the behaviour before the MULTIPORT feature.
- * On a newer host, when the host tells us
- * that a port 0 is available, we should just
- * say we have the port all set up.
- */
+ dev_dbg(&portdev->vdev->dev,
+ "Port %u already added\n", port->id);
send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
break;
}
@@ -1395,18 +1396,14 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
err = -ENOMEM;
goto free_vqs;
}
-
+ } else {
+ /*
+ * If we're running on an older host, we always want
+ * to create a console port.
+ */
+ add_port(portdev, 0);
}
- /*
- * For backward compatibility: if we're running on an older
- * host, we always want to create a console port.
- */
- add_port(portdev, 0);
-
- /* Start using the new console output. */
- early_put_chars = NULL;
-
__send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
VIRTIO_CONSOLE_DEVICE_READY, 1);
return 0;
--
1.6.2.5
^ permalink raw reply related
* [PATCH 5/8] virtio: console: Use a control message to add ports
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-5-git-send-email-amit.shah@redhat.com>
Instead of the host and guest independently enumerating ports, switch to
a control message to add ports where the host supplies the port number
so there's no ambiguity or a possibility of a race between the host and
the guest port numbers.
We now no longer need the 'nr_ports' config value. Since no kernel has
been released with the MULTIPORT changes yet, we have a chance to fiddle
with the config space without adding compatibility features.
This is beneficial for management software, which would now be able to
instantiate ports at known locations and avoid problems that arise with
implicit numbering in the host and the guest. This removes the 'guessing
game' part of it, and management software can now actually indicate
which id to spawn a particular port on.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 78 +++++++++++++++++----------------------
include/linux/virtio_console.h | 18 +++++----
2 files changed, 44 insertions(+), 52 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8056ad9..8b67ff1 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1034,7 +1034,7 @@ static void handle_control_message(struct ports_device *portdev,
cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
port = find_port_by_id(portdev, cpkt->id);
- if (!port) {
+ if (!port && cpkt->event != VIRTIO_CONSOLE_PORT_ADD) {
/* No valid header at start of buffer. Drop it. */
dev_dbg(&portdev->vdev->dev,
"Invalid index %u in control packet\n", cpkt->id);
@@ -1042,6 +1042,30 @@ static void handle_control_message(struct ports_device *portdev,
}
switch (cpkt->event) {
+ case VIRTIO_CONSOLE_PORT_ADD:
+ if (port) {
+ /*
+ * This can happen for port 0: we have to
+ * create a console port during probe() as was
+ * the behaviour before the MULTIPORT feature.
+ * On a newer host, when the host tells us
+ * that a port 0 is available, we should just
+ * say we have the port all set up.
+ */
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
+ break;
+ }
+ if (cpkt->id >= portdev->config.max_nr_ports) {
+ dev_warn(&portdev->vdev->dev,
+ "Request for adding port with out-of-bound id %u, max. supported id: %u\n",
+ cpkt->id, portdev->config.max_nr_ports - 1);
+ break;
+ }
+ add_port(portdev, cpkt->id);
+ break;
+ case VIRTIO_CONSOLE_PORT_REMOVE:
+ remove_port(port);
+ break;
case VIRTIO_CONSOLE_CONSOLE_PORT:
if (!cpkt->value)
break;
@@ -1100,32 +1124,6 @@ static void handle_control_message(struct ports_device *portdev,
kobject_uevent(&port->dev->kobj, KOBJ_CHANGE);
}
break;
- case VIRTIO_CONSOLE_PORT_REMOVE:
- /*
- * Hot unplug the port. We don't decrement nr_ports
- * since we don't want to deal with extra complexities
- * of using the lowest-available port id: We can just
- * pick up the nr_ports number as the id and not have
- * userspace send it to us. This helps us in two
- * ways:
- *
- * - We don't need to have a 'port_id' field in the
- * config space when a port is hot-added. This is a
- * good thing as we might queue up multiple hotplug
- * requests issued in our workqueue.
- *
- * - Another way to deal with this would have been to
- * use a bitmap of the active ports and select the
- * lowest non-active port from that map. That
- * bloats the already tight config space and we
- * would end up artificially limiting the
- * max. number of ports to sizeof(bitmap). Right
- * now we can support 2^32 ports (as the port id is
- * stored in a u32 type).
- *
- */
- remove_port(port);
- break;
}
}
@@ -1333,7 +1331,6 @@ static const struct file_operations portdev_fops = {
static int __devinit virtcons_probe(struct virtio_device *vdev)
{
struct ports_device *portdev;
- u32 i;
int err;
bool multiport;
@@ -1362,29 +1359,15 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
}
multiport = false;
- portdev->config.nr_ports = 1;
portdev->config.max_nr_ports = 1;
if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
multiport = true;
vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_MULTIPORT;
vdev->config->get(vdev, offsetof(struct virtio_console_config,
- nr_ports),
- &portdev->config.nr_ports,
- sizeof(portdev->config.nr_ports));
- vdev->config->get(vdev, offsetof(struct virtio_console_config,
max_nr_ports),
&portdev->config.max_nr_ports,
sizeof(portdev->config.max_nr_ports));
- if (portdev->config.nr_ports > portdev->config.max_nr_ports) {
- dev_warn(&vdev->dev,
- "More ports (%u) specified than allowed (%u). Will init %u ports.",
- portdev->config.nr_ports,
- portdev->config.max_nr_ports,
- portdev->config.max_nr_ports);
-
- portdev->config.nr_ports = portdev->config.max_nr_ports;
- }
}
/* Let the Host know we support multiple ports.*/
@@ -1412,13 +1395,20 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
err = -ENOMEM;
goto free_vqs;
}
+
}
- for (i = 0; i < portdev->config.nr_ports; i++)
- add_port(portdev, i);
+ /*
+ * For backward compatibility: if we're running on an older
+ * host, we always want to create a console port.
+ */
+ add_port(portdev, 0);
/* Start using the new console output. */
early_put_chars = NULL;
+
+ __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
+ VIRTIO_CONSOLE_DEVICE_READY, 1);
return 0;
free_vqs:
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index ae4f039..a85064d 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -14,6 +14,8 @@
#define VIRTIO_CONSOLE_F_SIZE 0 /* Does host provide console size? */
#define VIRTIO_CONSOLE_F_MULTIPORT 1 /* Does host provide multiple ports? */
+#define VIRTIO_CONSOLE_BAD_ID (~(u32)0)
+
struct virtio_console_config {
/* colums of the screens */
__u16 cols;
@@ -21,8 +23,6 @@ struct virtio_console_config {
__u16 rows;
/* max. number of ports this device can hold */
__u32 max_nr_ports;
- /* number of ports added so far */
- __u32 nr_ports;
} __attribute__((packed));
/*
@@ -36,12 +36,14 @@ struct virtio_console_control {
};
/* Some events for control messages */
-#define VIRTIO_CONSOLE_PORT_READY 0
-#define VIRTIO_CONSOLE_CONSOLE_PORT 1
-#define VIRTIO_CONSOLE_RESIZE 2
-#define VIRTIO_CONSOLE_PORT_OPEN 3
-#define VIRTIO_CONSOLE_PORT_NAME 4
-#define VIRTIO_CONSOLE_PORT_REMOVE 5
+#define VIRTIO_CONSOLE_DEVICE_READY 0
+#define VIRTIO_CONSOLE_PORT_ADD 1
+#define VIRTIO_CONSOLE_PORT_REMOVE 2
+#define VIRTIO_CONSOLE_PORT_READY 3
+#define VIRTIO_CONSOLE_CONSOLE_PORT 4
+#define VIRTIO_CONSOLE_RESIZE 5
+#define VIRTIO_CONSOLE_PORT_OPEN 6
+#define VIRTIO_CONSOLE_PORT_NAME 7
#ifdef __KERNEL__
int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
--
1.6.2.5
^ permalink raw reply related
* [PATCH 4/8] virtio: console: Move code around for future patches
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-4-git-send-email-amit.shah@redhat.com>
We're going to use add_port() from handle_control_message() in the next
patch.
Move the add_port() and fill_queue(), which depends on it, above
handle_control_message() to avoid forward declarations.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 262 ++++++++++++++++++++--------------------
1 files changed, 131 insertions(+), 131 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 99c36d4..8056ad9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -854,6 +854,137 @@ static const struct file_operations port_debugfs_ops = {
.read = debugfs_read,
};
+static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
+{
+ struct port_buffer *buf;
+ unsigned int nr_added_bufs;
+ int ret;
+
+ nr_added_bufs = 0;
+ do {
+ buf = alloc_buf(PAGE_SIZE);
+ if (!buf)
+ break;
+
+ spin_lock_irq(lock);
+ ret = add_inbuf(vq, buf);
+ if (ret < 0) {
+ spin_unlock_irq(lock);
+ free_buf(buf);
+ break;
+ }
+ nr_added_bufs++;
+ spin_unlock_irq(lock);
+ } while (ret > 0);
+
+ return nr_added_bufs;
+}
+
+static int add_port(struct ports_device *portdev, u32 id)
+{
+ char debugfs_name[16];
+ struct port *port;
+ struct port_buffer *buf;
+ dev_t devt;
+ unsigned int nr_added_bufs;
+ int err;
+
+ port = kmalloc(sizeof(*port), GFP_KERNEL);
+ if (!port) {
+ err = -ENOMEM;
+ goto fail;
+ }
+
+ port->portdev = portdev;
+ port->id = id;
+
+ port->name = NULL;
+ port->inbuf = NULL;
+ port->cons.hvc = NULL;
+
+ port->host_connected = port->guest_connected = false;
+
+ port->in_vq = portdev->in_vqs[port->id];
+ port->out_vq = portdev->out_vqs[port->id];
+
+ cdev_init(&port->cdev, &port_fops);
+
+ devt = MKDEV(portdev->chr_major, id);
+ err = cdev_add(&port->cdev, devt, 1);
+ if (err < 0) {
+ dev_err(&port->portdev->vdev->dev,
+ "Error %d adding cdev for port %u\n", err, id);
+ goto free_port;
+ }
+ port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev,
+ devt, port, "vport%up%u",
+ port->portdev->drv_index, id);
+ if (IS_ERR(port->dev)) {
+ err = PTR_ERR(port->dev);
+ dev_err(&port->portdev->vdev->dev,
+ "Error %d creating device for port %u\n",
+ err, id);
+ goto free_cdev;
+ }
+
+ spin_lock_init(&port->inbuf_lock);
+ init_waitqueue_head(&port->waitqueue);
+
+ /* Fill the in_vq with buffers so the host can send us data. */
+ nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock);
+ if (!nr_added_bufs) {
+ dev_err(port->dev, "Error allocating inbufs\n");
+ err = -ENOMEM;
+ goto free_device;
+ }
+
+ /*
+ * If we're not using multiport support, this has to be a console port
+ */
+ if (!use_multiport(port->portdev)) {
+ err = init_port_console(port);
+ if (err)
+ goto free_inbufs;
+ }
+
+ spin_lock_irq(&portdev->ports_lock);
+ list_add_tail(&port->list, &port->portdev->ports);
+ spin_unlock_irq(&portdev->ports_lock);
+
+ /*
+ * Tell the Host we're set so that it can send us various
+ * configuration parameters for this port (eg, port name,
+ * caching, whether this is a console port, etc.)
+ */
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
+
+ if (pdrvdata.debugfs_dir) {
+ /*
+ * Finally, create the debugfs file that we can use to
+ * inspect a port's state at any time
+ */
+ sprintf(debugfs_name, "vport%up%u",
+ port->portdev->drv_index, id);
+ port->debugfs_file = debugfs_create_file(debugfs_name, 0444,
+ pdrvdata.debugfs_dir,
+ port,
+ &port_debugfs_ops);
+ }
+ return 0;
+
+free_inbufs:
+ while ((buf = port->in_vq->vq_ops->detach_unused_buf(port->in_vq)))
+ free_buf(buf);
+free_device:
+ device_destroy(pdrvdata.class, port->dev->devt);
+free_cdev:
+ cdev_del(&port->cdev);
+free_port:
+ kfree(port);
+fail:
+ return err;
+}
+
/* Remove all port-specific data. */
static int remove_port(struct port *port)
{
@@ -1081,137 +1212,6 @@ static void config_intr(struct virtio_device *vdev)
resize_console(find_port_by_id(portdev, 0));
}
-static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
-{
- struct port_buffer *buf;
- unsigned int nr_added_bufs;
- int ret;
-
- nr_added_bufs = 0;
- do {
- buf = alloc_buf(PAGE_SIZE);
- if (!buf)
- break;
-
- spin_lock_irq(lock);
- ret = add_inbuf(vq, buf);
- if (ret < 0) {
- spin_unlock_irq(lock);
- free_buf(buf);
- break;
- }
- nr_added_bufs++;
- spin_unlock_irq(lock);
- } while (ret > 0);
-
- return nr_added_bufs;
-}
-
-static int add_port(struct ports_device *portdev, u32 id)
-{
- char debugfs_name[16];
- struct port *port;
- struct port_buffer *buf;
- dev_t devt;
- unsigned int nr_added_bufs;
- int err;
-
- port = kmalloc(sizeof(*port), GFP_KERNEL);
- if (!port) {
- err = -ENOMEM;
- goto fail;
- }
-
- port->portdev = portdev;
- port->id = id;
-
- port->name = NULL;
- port->inbuf = NULL;
- port->cons.hvc = NULL;
-
- port->host_connected = port->guest_connected = false;
-
- port->in_vq = portdev->in_vqs[port->id];
- port->out_vq = portdev->out_vqs[port->id];
-
- cdev_init(&port->cdev, &port_fops);
-
- devt = MKDEV(portdev->chr_major, id);
- err = cdev_add(&port->cdev, devt, 1);
- if (err < 0) {
- dev_err(&port->portdev->vdev->dev,
- "Error %d adding cdev for port %u\n", err, id);
- goto free_port;
- }
- port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev,
- devt, port, "vport%up%u",
- port->portdev->drv_index, id);
- if (IS_ERR(port->dev)) {
- err = PTR_ERR(port->dev);
- dev_err(&port->portdev->vdev->dev,
- "Error %d creating device for port %u\n",
- err, id);
- goto free_cdev;
- }
-
- spin_lock_init(&port->inbuf_lock);
- init_waitqueue_head(&port->waitqueue);
-
- /* Fill the in_vq with buffers so the host can send us data. */
- nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock);
- if (!nr_added_bufs) {
- dev_err(port->dev, "Error allocating inbufs\n");
- err = -ENOMEM;
- goto free_device;
- }
-
- /*
- * If we're not using multiport support, this has to be a console port
- */
- if (!use_multiport(port->portdev)) {
- err = init_port_console(port);
- if (err)
- goto free_inbufs;
- }
-
- spin_lock_irq(&portdev->ports_lock);
- list_add_tail(&port->list, &port->portdev->ports);
- spin_unlock_irq(&portdev->ports_lock);
-
- /*
- * Tell the Host we're set so that it can send us various
- * configuration parameters for this port (eg, port name,
- * caching, whether this is a console port, etc.)
- */
- send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
-
- if (pdrvdata.debugfs_dir) {
- /*
- * Finally, create the debugfs file that we can use to
- * inspect a port's state at any time
- */
- sprintf(debugfs_name, "vport%up%u",
- port->portdev->drv_index, id);
- port->debugfs_file = debugfs_create_file(debugfs_name, 0444,
- pdrvdata.debugfs_dir,
- port,
- &port_debugfs_ops);
- }
- return 0;
-
-free_inbufs:
- while ((buf = port->in_vq->vq_ops->detach_unused_buf(port->in_vq)))
- free_buf(buf);
-free_device:
- device_destroy(pdrvdata.class, port->dev->devt);
-free_cdev:
- cdev_del(&port->cdev);
-free_port:
- kfree(port);
-fail:
- return err;
-}
-
static int init_vqs(struct ports_device *portdev)
{
vq_callback_t **io_callbacks;
--
1.6.2.5
^ permalink raw reply related
* [PATCH 3/8] virtio: console: Add a __send_control_msg() that can send messages without a valid port
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-3-git-send-email-amit.shah@redhat.com>
We will introduce control messages that operate on the device as a whole
rather than just ports. Make send_control_msg() a wrapper around
__send_control_msg() which does not need a valid port.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c7894f3..99c36d4 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -372,22 +372,22 @@ out:
return ret;
}
-static ssize_t send_control_msg(struct port *port, unsigned int event,
- unsigned int value)
+static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
+ unsigned int event, unsigned int value)
{
struct scatterlist sg[1];
struct virtio_console_control cpkt;
struct virtqueue *vq;
unsigned int len;
- if (!use_multiport(port->portdev))
+ if (!use_multiport(portdev))
return 0;
- cpkt.id = port->id;
+ cpkt.id = port_id;
cpkt.event = event;
cpkt.value = value;
- vq = port->portdev->c_ovq;
+ vq = portdev->c_ovq;
sg_init_one(sg, &cpkt, sizeof(cpkt));
if (vq->vq_ops->add_buf(vq, sg, 1, 0, &cpkt) >= 0) {
@@ -398,6 +398,12 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
return 0;
}
+static ssize_t send_control_msg(struct port *port, unsigned int event,
+ unsigned int value)
+{
+ return __send_control_msg(port->portdev, port->id, event, value);
+}
+
static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
{
struct scatterlist sg[1];
--
1.6.2.5
^ permalink raw reply related
* [PATCH 2/8] virtio: console: Remove config work handler
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-2-git-send-email-amit.shah@redhat.com>
We're going to switch to using control messages for port hot-plug and
initial port discovery. Remove the config work handler which handled
port hot-plug so far.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 64 +----------------------------------------
1 files changed, 1 insertions(+), 63 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 44288ce..c7894f3 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -109,7 +109,6 @@ struct ports_device {
* notification
*/
struct work_struct control_work;
- struct work_struct config_work;
struct list_head ports;
@@ -1066,10 +1065,7 @@ static void config_intr(struct virtio_device *vdev)
struct ports_device *portdev;
portdev = vdev->priv;
- if (use_multiport(portdev)) {
- /* Handle port hot-add */
- schedule_work(&portdev->config_work);
- }
+
/*
* We'll use this way of resizing only for legacy support.
* For newer userspace (VIRTIO_CONSOLE_F_MULTPORT+), use
@@ -1210,62 +1206,6 @@ fail:
return err;
}
-/*
- * The workhandler for config-space updates.
- *
- * This is called when ports are hot-added.
- */
-static void config_work_handler(struct work_struct *work)
-{
- struct virtio_console_config virtconconf;
- struct ports_device *portdev;
- struct virtio_device *vdev;
- int err;
-
- portdev = container_of(work, struct ports_device, config_work);
-
- vdev = portdev->vdev;
- vdev->config->get(vdev,
- offsetof(struct virtio_console_config, nr_ports),
- &virtconconf.nr_ports,
- sizeof(virtconconf.nr_ports));
-
- if (portdev->config.nr_ports == virtconconf.nr_ports) {
- /*
- * Port 0 got hot-added. Since we already did all the
- * other initialisation for it, just tell the Host
- * that the port is ready if we find the port. In
- * case the port was hot-removed earlier, we call
- * add_port to add the port.
- */
- struct port *port;
-
- port = find_port_by_id(portdev, 0);
- if (!port)
- add_port(portdev, 0);
- else
- send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
- return;
- }
- if (virtconconf.nr_ports > portdev->config.max_nr_ports) {
- dev_warn(&vdev->dev,
- "More ports specified (%u) than allowed (%u)",
- portdev->config.nr_ports + 1,
- portdev->config.max_nr_ports);
- return;
- }
- if (virtconconf.nr_ports < portdev->config.nr_ports)
- return;
-
- /* Hot-add ports */
- while (virtconconf.nr_ports - portdev->config.nr_ports) {
- err = add_port(portdev, portdev->config.nr_ports);
- if (err)
- break;
- portdev->config.nr_ports++;
- }
-}
-
static int init_vqs(struct ports_device *portdev)
{
vq_callback_t **io_callbacks;
@@ -1458,7 +1398,6 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
spin_lock_init(&portdev->cvq_lock);
INIT_WORK(&portdev->control_work, &control_work_handler);
- INIT_WORK(&portdev->config_work, &config_work_handler);
nr_added_bufs = fill_queue(portdev->c_ivq, &portdev->cvq_lock);
if (!nr_added_bufs) {
@@ -1498,7 +1437,6 @@ static void virtcons_remove(struct virtio_device *vdev)
portdev = vdev->priv;
cancel_work_sync(&portdev->control_work);
- cancel_work_sync(&portdev->config_work);
list_for_each_entry_safe(port, port2, &portdev->ports, list)
remove_port(port);
--
1.6.2.5
^ permalink raw reply related
* [PATCH 1/8] MAINTAINERS: Put the virtio-console entry in correct alphabetical order
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <1269348796-8660-1-git-send-email-amit.shah@redhat.com>
Move around the entry for virtio-console to keep the file sorted.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
MAINTAINERS | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 449d444..e3548dd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2474,12 +2474,6 @@ L: linuxppc-dev@ozlabs.org
S: Odd Fixes
F: drivers/char/hvc_*
-VIRTIO CONSOLE DRIVER
-M: Amit Shah <amit.shah@redhat.com>
-L: virtualization@lists.linux-foundation.org
-S: Maintained
-F: drivers/char/virtio_console.c
-
iSCSI BOOT FIRMWARE TABLE (iBFT) DRIVER
M: Peter Jones <pjones@redhat.com>
M: Konrad Rzeszutek Wilk <konrad@kernel.org>
@@ -5961,6 +5955,13 @@ S: Maintained
F: Documentation/filesystems/vfat.txt
F: fs/fat/
+VIRTIO CONSOLE DRIVER
+M: Amit Shah <amit.shah@redhat.com>
+L: virtualization@lists.linux-foundation.org
+S: Maintained
+F: drivers/char/virtio_console.c
+F: include/linux/virtio_console.h
+
VIRTIO HOST (VHOST)
M: "Michael S. Tsirkin" <mst@redhat.com>
L: kvm@vger.kernel.org
--
1.6.2.5
^ permalink raw reply related
* [PATCH 0/8] (v4) virtio: console: Fixes, new way of discovering ports
From: Amit Shah @ 2010-03-23 12:53 UTC (permalink / raw)
To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1331 bytes --]
Hello,
This version addresses comments by Gerd and Michael:
- instead of returning -EPIPE if host is not connected in the write
case, just block the write if it's a blocking file and return -EAGAIN
if it's non-blocking.
- Removes early_console_printk after the first console is found
- Adds early_put_chars fix by François Diakhaté. It still needs a
sign-off by him.
Standard message: Since the ABI is being reworked, it's essential to
push this to 2.6.34 so that we're not left supporting the older
interface.
Amit Shah (7):
MAINTAINERS: Put the virtio-console entry in correct alphabetical
order
virtio: console: Remove config work handler
virtio: console: Add a __send_control_msg() that can send messages
without a valid port
virtio: console: Move code around for future patches
virtio: console: Use a control message to add ports
virtio: console: Don't always create a port 0 if using multiport
virtio: console: Add partial blocking/non-blocking support to write()
François Diakhaté (1):
virtio: console: Fix early_put_chars usage
MAINTAINERS | 13 +-
drivers/char/virtio_console.c | 447 ++++++++++++++++++----------------------
include/linux/virtio_console.h | 18 +-
3 files changed, 215 insertions(+), 263 deletions(-)
[-- Attachment #2: Type: text/plain, Size: 184 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] vhost-blk implementation
From: Michael S. Tsirkin @ 2010-03-23 12:39 UTC (permalink / raw)
To: Badari Pulavarty; +Cc: qemu-devel, virtualization
In-Reply-To: <1269304444.7931.68.camel@badari-desktop>
On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
> Write Results:
> ==============
>
> I see degraded IO performance when doing sequential IO write
> tests with vhost-blk compared to virtio-blk.
>
> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>
> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
> vhost-blk. Wondering why ?
Try to look and number of interrupts and/or number of exits.
It could also be that you are overrunning some queue.
I don't see any exit mitigation strategy in your patch:
when there are already lots of requests in a queue, it's usually
a good idea to disable notifications and poll the
queue as requests complete. That could help performance.
--
MST
^ permalink raw reply
* Re: [PATCH 7/7] virtio: console: Return -EPIPE if port on the host isn't connected
From: Amit Shah @ 2010-03-23 11:21 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: virtualization, Michael S. Tsirkin
In-Reply-To: <4BA8A2C7.4070908@redhat.com>
On (Tue) Mar 23 2010 [12:15:19], Gerd Hoffmann wrote:
> On 03/23/10 12:09, Amit Shah wrote:
>> I didn't really look at blocking / non-blocking support in write()
>> because of the cpu_relax(), which will always block the app.
>
> Right, the spinning again ...
>
> When we kill this we really must care to implement blocking +
> non-blocking + poll + partial writes correctly.
Yes; I'm adding blocking/non-blocking right away for the host_connected
condition.
Partial writes, blocking/non-blocking vq operations will have to come
for 2.6.35 at the latest.
Amit
^ permalink raw reply
* Re: [PATCH 7/7] virtio: console: Return -EPIPE if port on the host isn't connected
From: Gerd Hoffmann @ 2010-03-23 11:15 UTC (permalink / raw)
To: Amit Shah; +Cc: virtualization, Michael S. Tsirkin
In-Reply-To: <20100323110957.GB3080@amit-x200.redhat.com>
On 03/23/10 12:09, Amit Shah wrote:
> I didn't really look at blocking / non-blocking support in write()
> because of the cpu_relax(), which will always block the app.
Right, the spinning again ...
When we kill this we really must care to implement blocking +
non-blocking + poll + partial writes correctly.
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH 7/7] virtio: console: Return -EPIPE if port on the host isn't connected
From: Amit Shah @ 2010-03-23 11:09 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: virtualization, Michael S. Tsirkin
In-Reply-To: <4BA8A03C.8030002@redhat.com>
On (Tue) Mar 23 2010 [12:04:28], Gerd Hoffmann wrote:
> On 03/23/10 11:52, Amit Shah wrote:
>
>>> With poll support done right (don't signal "writable" when not
>>> connected) this shouldn't be a issue. Apps retrying over and over
>>> without waiting for the filehandle becoming writable are broken by
>>> design.
>>
>> Yeah; poll returns POLLOUT only when the host connection is open. This
>> is for apps that write() without consulting poll().
>
> Well, /me just remembers this also depends on the file mode. Returning
> -EAGAIN instantly is fine for non-blocking file handles. For file
> handles in blocking mode you would have to block here, waiting for the
> host to connect.
Ah, right.
I didn't really look at blocking / non-blocking support in write()
because of the cpu_relax(), which will always block the app.
I'll add this.
Amit
^ permalink raw reply
* Re: [PATCH 7/7] virtio: console: Return -EPIPE if port on the host isn't connected
From: Amit Shah @ 2010-03-23 11:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization
In-Reply-To: <20100323090023.GD22774@redhat.com>
On (Tue) Mar 23 2010 [11:00:23], Michael S. Tsirkin wrote:
> On Tue, Mar 23, 2010 at 02:28:00PM +0530, Amit Shah wrote:
> > The virtio-serial ports are like pipes, if there's no reader on the
> > other end, sending data might get it either ignored or the host might
> > return '0', which would make guests get -EAGAIN. Since we know the state
> > of the host port connection, it's appropriate to let the application
> > know that the other end isn't connected.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> > drivers/char/virtio_console.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 55de0b5..4562964 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -411,6 +411,9 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
> > ssize_t ret;
> > unsigned int len;
> >
> > + if (use_multiport(port->portdev) && !port->host_connected)
> > + return -EPIPE;
> > +
>
> Hmm, do applications actually handle this error?
As Michael points out on irc, console-handling apps may not handle EPIPE
properly. So I guess the test should be reworked to:
if (!is_console_port(port) && !port->host_connected)
return -EPIPE;
Also, he points out that write(2) mentions other error messages can be
returned depending on the file type. So we could return -ENOTCONN.
Amit
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox