* [PATCH 0/2] virtio-serial: set up vqs on demand
@ 2012-01-12 1:20 zanghongyong
2012-01-12 1:20 ` [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs zanghongyong
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: zanghongyong @ 2012-01-12 1:20 UTC (permalink / raw)
To: kvm
Cc: rusty, amit.shah, aliguori, xiaowei.yang, hanweidong, wusongwei,
jiangningyu, Hongyong Zang
From: Hongyong Zang <zanghongyong@huawei.com>
Virtio-serial set up (max_ports+1)*2 vqs when device probes, but may not all
io_ports are used.
These patches create vqs of port0 and control port when probing the device, then
create io-vqs when called add_port().
Hongyong Zang (2):
virtio-pci: add setup_vqs flag in vp_try_to_find_vqs
virtio-serial: setup_port_vq when adding port
drivers/char/virtio_console.c | 65 ++++++++++++++++++++++++++++++++++++++--
drivers/virtio/virtio_pci.c | 17 ++++++++--
2 files changed, 74 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs 2012-01-12 1:20 [PATCH 0/2] virtio-serial: set up vqs on demand zanghongyong @ 2012-01-12 1:20 ` zanghongyong 2012-02-01 8:14 ` Amit Shah 2012-02-01 9:54 ` Michael S. Tsirkin 2012-01-12 1:20 ` [PATCH 2/2] virtio-serial: setup_port_vq when adding port zanghongyong ` (2 subsequent siblings) 3 siblings, 2 replies; 10+ messages in thread From: zanghongyong @ 2012-01-12 1:20 UTC (permalink / raw) To: kvm Cc: rusty, amit.shah, aliguori, xiaowei.yang, hanweidong, wusongwei, jiangningyu, Hongyong Zang From: Hongyong Zang <zanghongyong@huawei.com> changes in vp_try_to_find_vqs: Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and controls; add_port() calls it to set up vqs of io_port. it will not create virtqueue if the name is null. Signed-off-by: Hongyong Zang <zanghongyong@huawei.com> --- drivers/virtio/virtio_pci.c | 17 +++++++++++++---- 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index baabb79..1f98c36 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -492,9 +492,11 @@ static void vp_del_vqs(struct virtio_device *vdev) list_for_each_entry_safe(vq, n, &vdev->vqs, list) { info = vq->priv; if (vp_dev->per_vq_vectors && - info->msix_vector != VIRTIO_MSI_NO_VECTOR) + info->msix_vector != VIRTIO_MSI_NO_VECTOR) { free_irq(vp_dev->msix_entries[info->msix_vector].vector, vq); + vp_dev->msix_used_vectors--; + } vp_del_vq(vq); } vp_dev->per_vq_vectors = false; @@ -511,7 +513,10 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, { struct virtio_pci_device *vp_dev = to_vp_device(vdev); u16 msix_vec; - int i, err, nvectors, allocated_vectors; + int i, err, nvectors; + + if (vp_dev->msix_used_vectors) + goto setup_vqs; if (!use_msix) { /* Old style: one normal interrupt for change and all vqs. */ @@ -536,12 +541,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, } vp_dev->per_vq_vectors = per_vq_vectors; - allocated_vectors = vp_dev->msix_used_vectors; + +setup_vqs: for (i = 0; i < nvqs; ++i) { + if (names[i] == NULL) + continue; + if (!callbacks[i] || !vp_dev->msix_enabled) msix_vec = VIRTIO_MSI_NO_VECTOR; else if (vp_dev->per_vq_vectors) - msix_vec = allocated_vectors++; + msix_vec = vp_dev->msix_used_vectors++; else msix_vec = VP_MSIX_VQ_VECTOR; vqs[i] = setup_vq(vdev, i, callbacks[i], names[i], msix_vec); -- 1.7.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs 2012-01-12 1:20 ` [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs zanghongyong @ 2012-02-01 8:14 ` Amit Shah 2012-02-01 9:54 ` Michael S. Tsirkin 1 sibling, 0 replies; 10+ messages in thread From: Amit Shah @ 2012-02-01 8:14 UTC (permalink / raw) To: zanghongyong Cc: aliguori, kvm, wusongwei, hanweidong, Virtualization List, xiaowei.yang, Michael S. Tsirkin, jiangningyu Michael, Rusty, any comments? On (Thu) 12 Jan 2012 [09:20:06], zanghongyong@huawei.com wrote: > From: Hongyong Zang <zanghongyong@huawei.com> > > changes in vp_try_to_find_vqs: > Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and > controls; add_port() calls it to set up vqs of io_port. > it will not create virtqueue if the name is null. > > Signed-off-by: Hongyong Zang <zanghongyong@huawei.com> > --- > drivers/virtio/virtio_pci.c | 17 +++++++++++++---- > 1 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c > index baabb79..1f98c36 100644 > --- a/drivers/virtio/virtio_pci.c > +++ b/drivers/virtio/virtio_pci.c > @@ -492,9 +492,11 @@ static void vp_del_vqs(struct virtio_device *vdev) > list_for_each_entry_safe(vq, n, &vdev->vqs, list) { > info = vq->priv; > if (vp_dev->per_vq_vectors && > - info->msix_vector != VIRTIO_MSI_NO_VECTOR) > + info->msix_vector != VIRTIO_MSI_NO_VECTOR) { > free_irq(vp_dev->msix_entries[info->msix_vector].vector, > vq); > + vp_dev->msix_used_vectors--; > + } > vp_del_vq(vq); > } > vp_dev->per_vq_vectors = false; > @@ -511,7 +513,10 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > u16 msix_vec; > - int i, err, nvectors, allocated_vectors; > + int i, err, nvectors; > + > + if (vp_dev->msix_used_vectors) > + goto setup_vqs; > > if (!use_msix) { > /* Old style: one normal interrupt for change and all vqs. */ > @@ -536,12 +541,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, > } > > vp_dev->per_vq_vectors = per_vq_vectors; > - allocated_vectors = vp_dev->msix_used_vectors; > + > +setup_vqs: > for (i = 0; i < nvqs; ++i) { > + if (names[i] == NULL) > + continue; > + > if (!callbacks[i] || !vp_dev->msix_enabled) > msix_vec = VIRTIO_MSI_NO_VECTOR; > else if (vp_dev->per_vq_vectors) > - msix_vec = allocated_vectors++; > + msix_vec = vp_dev->msix_used_vectors++; > else > msix_vec = VP_MSIX_VQ_VECTOR; > vqs[i] = setup_vq(vdev, i, callbacks[i], names[i], msix_vec); > -- > 1.7.1 > Amit ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs 2012-01-12 1:20 ` [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs zanghongyong 2012-02-01 8:14 ` Amit Shah @ 2012-02-01 9:54 ` Michael S. Tsirkin 2012-02-03 3:56 ` Rusty Russell 1 sibling, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2012-02-01 9:54 UTC (permalink / raw) To: zanghongyong Cc: kvm, rusty, amit.shah, aliguori, xiaowei.yang, hanweidong, wusongwei, jiangningyu On Thu, Jan 12, 2012 at 09:20:06AM +0800, zanghongyong@huawei.com wrote: > From: Hongyong Zang <zanghongyong@huawei.com> > > changes in vp_try_to_find_vqs: > Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and > controls; add_port() calls it to set up vqs of io_port. > it will not create virtqueue if the name is null. > > Signed-off-by: Hongyong Zang <zanghongyong@huawei.com> This looks like a fragile hack, to me. virtio spec also implied that VQ initialization is done before status is set to OK (during probe) and devices might have relied on that. So if we want to change that, I think we need a feature bit. Besides, what about documentation? non pci transports? > --- > drivers/virtio/virtio_pci.c | 17 +++++++++++++---- > 1 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c > index baabb79..1f98c36 100644 > --- a/drivers/virtio/virtio_pci.c > +++ b/drivers/virtio/virtio_pci.c > @@ -492,9 +492,11 @@ static void vp_del_vqs(struct virtio_device *vdev) > list_for_each_entry_safe(vq, n, &vdev->vqs, list) { > info = vq->priv; > if (vp_dev->per_vq_vectors && > - info->msix_vector != VIRTIO_MSI_NO_VECTOR) > + info->msix_vector != VIRTIO_MSI_NO_VECTOR) { > free_irq(vp_dev->msix_entries[info->msix_vector].vector, > vq); > + vp_dev->msix_used_vectors--; > + } > vp_del_vq(vq); > } > vp_dev->per_vq_vectors = false; > @@ -511,7 +513,10 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > u16 msix_vec; > - int i, err, nvectors, allocated_vectors; > + int i, err, nvectors; > + > + if (vp_dev->msix_used_vectors) > + goto setup_vqs; Please don't abuse goto in this way. > > if (!use_msix) { > /* Old style: one normal interrupt for change and all vqs. */ > @@ -536,12 +541,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, > } > > vp_dev->per_vq_vectors = per_vq_vectors; > - allocated_vectors = vp_dev->msix_used_vectors; > + > +setup_vqs: > for (i = 0; i < nvqs; ++i) { > + if (names[i] == NULL) if (![names[i]) > + continue; > + > if (!callbacks[i] || !vp_dev->msix_enabled) > msix_vec = VIRTIO_MSI_NO_VECTOR; > else if (vp_dev->per_vq_vectors) > - msix_vec = allocated_vectors++; > + msix_vec = vp_dev->msix_used_vectors++; > else > msix_vec = VP_MSIX_VQ_VECTOR; > vqs[i] = setup_vq(vdev, i, callbacks[i], names[i], msix_vec); > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs 2012-02-01 9:54 ` Michael S. Tsirkin @ 2012-02-03 3:56 ` Rusty Russell 0 siblings, 0 replies; 10+ messages in thread From: Rusty Russell @ 2012-02-03 3:56 UTC (permalink / raw) To: Michael S. Tsirkin, zanghongyong Cc: kvm, amit.shah, aliguori, xiaowei.yang, hanweidong, wusongwei, jiangningyu On Wed, 1 Feb 2012 11:54:09 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jan 12, 2012 at 09:20:06AM +0800, zanghongyong@huawei.com wrote: > > From: Hongyong Zang <zanghongyong@huawei.com> > > > > changes in vp_try_to_find_vqs: > > Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and > > controls; add_port() calls it to set up vqs of io_port. > > it will not create virtqueue if the name is null. > > > > Signed-off-by: Hongyong Zang <zanghongyong@huawei.com> > > This looks like a fragile hack, to me. > > virtio spec also implied that VQ initialization is done > before status is set to OK (during probe) and > devices might have relied on that. So if we want to change > that, I think we need a feature bit. If virtio_serial doesn't use those virtqueues, I think it's OK. I think I'd rather have vp_find_vqs take an offset, however. > Besides, what about documentation? non pci transports? I think this is a device-specific issue, but re-calling find_vqs will have to be audited. Which adding an offset arg should lead to anyway. Thanks, Rusty. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] virtio-serial: setup_port_vq when adding port 2012-01-12 1:20 [PATCH 0/2] virtio-serial: set up vqs on demand zanghongyong 2012-01-12 1:20 ` [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs zanghongyong @ 2012-01-12 1:20 ` zanghongyong 2012-02-01 8:12 ` Amit Shah 2012-02-01 9:40 ` [PATCH 0/2] virtio-serial: set up vqs on demand Michael S. Tsirkin 2013-04-08 7:51 ` Amit Shah 3 siblings, 1 reply; 10+ messages in thread From: zanghongyong @ 2012-01-12 1:20 UTC (permalink / raw) To: kvm Cc: rusty, amit.shah, aliguori, xiaowei.yang, hanweidong, wusongwei, jiangningyu, Hongyong Zang From: Hongyong Zang <zanghongyong@huawei.com> Add setup_port_vq(). Create the io ports' vqs when add_port. Signed-off-by: Hongyong Zang <zanghongyong@huawei.com> --- drivers/char/virtio_console.c | 65 ++++++++++++++++++++++++++++++++++++++-- 1 files changed, 61 insertions(+), 4 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 8e3c46d..2e5187e 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1132,6 +1132,55 @@ static void send_sigio_to_port(struct port *port) kill_fasync(&port->async_queue, SIGIO, POLL_OUT); } +static void in_intr(struct virtqueue *vq); +static void out_intr(struct virtqueue *vq); + +static int setup_port_vq(struct ports_device *portdev, u32 id) +{ + int err, vq_num; + vq_callback_t **io_callbacks; + char **io_names; + struct virtqueue **vqs; + u32 i,j,nr_ports,nr_queues; + + err = 0; + vq_num = (id + 1) * 2; + nr_ports = portdev->config.max_nr_ports; + nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2; + + vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL); + io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL); + io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL); + if (!vqs || !io_callbacks || !io_names) { + err = -ENOMEM; + goto free; + } + + for (i = 0, j = 0; i <= nr_ports; i++) { + io_callbacks[j] = in_intr; + io_callbacks[j + 1] = out_intr; + io_names[j] = NULL; + io_names[j + 1] = NULL; + j += 2; + } + io_names[vq_num] = "serial-input"; + io_names[vq_num + 1] = "serial-output"; + err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs, + io_callbacks, + (const char **)io_names); + if (err) + goto free; + portdev->in_vqs[id] = vqs[vq_num]; + portdev->out_vqs[id] = vqs[vq_num + 1]; + +free: + kfree(io_names); + kfree(io_callbacks); + kfree(vqs); + + return err; +} + static int add_port(struct ports_device *portdev, u32 id) { char debugfs_name[16]; @@ -1163,6 +1212,14 @@ static int add_port(struct ports_device *portdev, u32 id) port->outvq_full = false; + if (!portdev->in_vqs[port->id] && !portdev->out_vqs[port->id]) { + spin_lock(&portdev->ports_lock); + err = setup_port_vq(portdev, port->id); + spin_unlock(&portdev->ports_lock); + if (err) + goto free_port; + } + port->in_vq = portdev->in_vqs[port->id]; port->out_vq = portdev->out_vqs[port->id]; @@ -1614,8 +1671,8 @@ static int init_vqs(struct ports_device *portdev) j += 2; io_callbacks[j] = in_intr; io_callbacks[j + 1] = out_intr; - io_names[j] = "input"; - io_names[j + 1] = "output"; + io_names[j] = NULL; + io_names[j + 1] = NULL; } } /* Find the queues. */ @@ -1635,8 +1692,8 @@ static int init_vqs(struct ports_device *portdev) for (i = 1; i < nr_ports; i++) { j += 2; - portdev->in_vqs[i] = vqs[j]; - portdev->out_vqs[i] = vqs[j + 1]; + portdev->in_vqs[i] = NULL; + portdev->out_vqs[i] = NULL; } } kfree(io_names); -- 1.7.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] virtio-serial: setup_port_vq when adding port 2012-01-12 1:20 ` [PATCH 2/2] virtio-serial: setup_port_vq when adding port zanghongyong @ 2012-02-01 8:12 ` Amit Shah 2012-02-01 9:32 ` Zang Hongyong 0 siblings, 1 reply; 10+ messages in thread From: Amit Shah @ 2012-02-01 8:12 UTC (permalink / raw) To: zanghongyong Cc: aliguori, kvm, wusongwei, hanweidong, Virtualization List, xiaowei.yang, jiangningyu Hi, Sorry for the late reply. On (Thu) 12 Jan 2012 [09:20:07], zanghongyong@huawei.com wrote: > From: Hongyong Zang <zanghongyong@huawei.com> > > Add setup_port_vq(). Create the io ports' vqs when add_port. Can you describe the changes in more detail, please? > Signed-off-by: Hongyong Zang <zanghongyong@huawei.com> > --- > drivers/char/virtio_console.c | 65 ++++++++++++++++++++++++++++++++++++++-- > 1 files changed, 61 insertions(+), 4 deletions(-) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 8e3c46d..2e5187e 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1132,6 +1132,55 @@ static void send_sigio_to_port(struct port *port) > kill_fasync(&port->async_queue, SIGIO, POLL_OUT); > } > > +static void in_intr(struct virtqueue *vq); > +static void out_intr(struct virtqueue *vq); > + > +static int setup_port_vq(struct ports_device *portdev, u32 id) > +{ > + int err, vq_num; > + vq_callback_t **io_callbacks; > + char **io_names; > + struct virtqueue **vqs; > + u32 i,j,nr_ports,nr_queues; > + > + err = 0; > + vq_num = (id + 1) * 2; > + nr_ports = portdev->config.max_nr_ports; > + nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2; > + > + vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL); > + io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL); > + io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL); > + if (!vqs || !io_callbacks || !io_names) { > + err = -ENOMEM; > + goto free; > + } > + > + for (i = 0, j = 0; i <= nr_ports; i++) { > + io_callbacks[j] = in_intr; > + io_callbacks[j + 1] = out_intr; > + io_names[j] = NULL; > + io_names[j + 1] = NULL; > + j += 2; > + } > + io_names[vq_num] = "serial-input"; > + io_names[vq_num + 1] = "serial-output"; > + err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs, > + io_callbacks, > + (const char **)io_names); > + if (err) > + goto free; > + portdev->in_vqs[id] = vqs[vq_num]; > + portdev->out_vqs[id] = vqs[vq_num + 1]; I don't think this approach will work fine for port hot-plug / hot-unplug cases at all. For example, I first start qemu with one port, at id 1. Then I add a port at id 5, then at 2, then at 10. Will that be fine? > + > +free: > + kfree(io_names); > + kfree(io_callbacks); > + kfree(vqs); > + > + return err; > +} > + > static int add_port(struct ports_device *portdev, u32 id) > { > char debugfs_name[16]; > @@ -1163,6 +1212,14 @@ static int add_port(struct ports_device *portdev, u32 id) > > port->outvq_full = false; > > + if (!portdev->in_vqs[port->id] && !portdev->out_vqs[port->id]) { > + spin_lock(&portdev->ports_lock); > + err = setup_port_vq(portdev, port->id); > + spin_unlock(&portdev->ports_lock); > + if (err) > + goto free_port; > + } > + > port->in_vq = portdev->in_vqs[port->id]; > port->out_vq = portdev->out_vqs[port->id]; > > @@ -1614,8 +1671,8 @@ static int init_vqs(struct ports_device *portdev) > j += 2; > io_callbacks[j] = in_intr; > io_callbacks[j + 1] = out_intr; > - io_names[j] = "input"; > - io_names[j + 1] = "output"; > + io_names[j] = NULL; > + io_names[j + 1] = NULL; > } > } > /* Find the queues. */ > @@ -1635,8 +1692,8 @@ static int init_vqs(struct ports_device *portdev) > > for (i = 1; i < nr_ports; i++) { > j += 2; > - portdev->in_vqs[i] = vqs[j]; > - portdev->out_vqs[i] = vqs[j + 1]; > + portdev->in_vqs[i] = NULL; > + portdev->out_vqs[i] = NULL; > } > } > kfree(io_names); So a queue once created will not be removed unless the module is removed or the device is removed. That seems reasonable, port hot-unplug will keep queues around, as is the case now. Amit ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] virtio-serial: setup_port_vq when adding port 2012-02-01 8:12 ` Amit Shah @ 2012-02-01 9:32 ` Zang Hongyong 0 siblings, 0 replies; 10+ messages in thread From: Zang Hongyong @ 2012-02-01 9:32 UTC (permalink / raw) To: Amit Shah Cc: aliguori, kvm, wusongwei, hanweidong, Virtualization List, xiaowei.yang, jiangningyu On 2012/2/1,星期三 16:12, Amit Shah wrote: > Hi, > > Sorry for the late reply. > > On (Thu) 12 Jan 2012 [09:20:07], zanghongyong@huawei.com wrote: >> From: Hongyong Zang<zanghongyong@huawei.com> >> >> Add setup_port_vq(). Create the io ports' vqs when add_port. > Can you describe the changes in more detail, please? The motivation of this patch is as follows. When we use virtio-serial for communication between guest and host, we usually use only a few ports (for example 1 or 2 ports), so there's no need to create max ports when build a virtio-serial. The patch does the port hot-plug thing without port hot-unplug. > >> Signed-off-by: Hongyong Zang<zanghongyong@huawei.com> >> --- >> drivers/char/virtio_console.c | 65 ++++++++++++++++++++++++++++++++++++++-- >> 1 files changed, 61 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c >> index 8e3c46d..2e5187e 100644 >> --- a/drivers/char/virtio_console.c >> +++ b/drivers/char/virtio_console.c >> @@ -1132,6 +1132,55 @@ static void send_sigio_to_port(struct port *port) >> kill_fasync(&port->async_queue, SIGIO, POLL_OUT); >> } >> >> +static void in_intr(struct virtqueue *vq); >> +static void out_intr(struct virtqueue *vq); >> + >> +static int setup_port_vq(struct ports_device *portdev, u32 id) >> +{ >> + int err, vq_num; >> + vq_callback_t **io_callbacks; >> + char **io_names; >> + struct virtqueue **vqs; >> + u32 i,j,nr_ports,nr_queues; >> + >> + err = 0; >> + vq_num = (id + 1) * 2; >> + nr_ports = portdev->config.max_nr_ports; >> + nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2; >> + >> + vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL); >> + io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL); >> + io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL); >> + if (!vqs || !io_callbacks || !io_names) { >> + err = -ENOMEM; >> + goto free; >> + } >> + >> + for (i = 0, j = 0; i<= nr_ports; i++) { >> + io_callbacks[j] = in_intr; >> + io_callbacks[j + 1] = out_intr; >> + io_names[j] = NULL; >> + io_names[j + 1] = NULL; >> + j += 2; >> + } >> + io_names[vq_num] = "serial-input"; >> + io_names[vq_num + 1] = "serial-output"; >> + err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs, >> + io_callbacks, >> + (const char **)io_names); >> + if (err) >> + goto free; >> + portdev->in_vqs[id] = vqs[vq_num]; >> + portdev->out_vqs[id] = vqs[vq_num + 1]; > I don't think this approach will work fine for port hot-plug / > hot-unplug cases at all. For example, I first start qemu with one > port, at id 1. Then I add a port at id 5, then at 2, then at 10. > Will that be fine? yes. In this case, the virtio-serial will create id 1's queue when probe the device. Then, add port id 5, it will create the id 5's queue and the queues of id 2-4 still be null. we find the right ioport by vq_num, and only the io_name[vq_num] and io_name[vq_num+1] are not null. So after portdev->vdev->config->find_vqs(), the id 5's queues are created(see the changes in virtio-pci), the others are still null. > >> + >> +free: >> + kfree(io_names); >> + kfree(io_callbacks); >> + kfree(vqs); >> + >> + return err; >> +} >> + >> static int add_port(struct ports_device *portdev, u32 id) >> { >> char debugfs_name[16]; >> @@ -1163,6 +1212,14 @@ static int add_port(struct ports_device *portdev, u32 id) >> >> port->outvq_full = false; >> >> + if (!portdev->in_vqs[port->id]&& !portdev->out_vqs[port->id]) { >> + spin_lock(&portdev->ports_lock); >> + err = setup_port_vq(portdev, port->id); >> + spin_unlock(&portdev->ports_lock); >> + if (err) >> + goto free_port; >> + } >> + >> port->in_vq = portdev->in_vqs[port->id]; >> port->out_vq = portdev->out_vqs[port->id]; >> >> @@ -1614,8 +1671,8 @@ static int init_vqs(struct ports_device *portdev) >> j += 2; >> io_callbacks[j] = in_intr; >> io_callbacks[j + 1] = out_intr; >> - io_names[j] = "input"; >> - io_names[j + 1] = "output"; >> + io_names[j] = NULL; >> + io_names[j + 1] = NULL; >> } >> } >> /* Find the queues. */ >> @@ -1635,8 +1692,8 @@ static int init_vqs(struct ports_device *portdev) >> >> for (i = 1; i< nr_ports; i++) { >> j += 2; >> - portdev->in_vqs[i] = vqs[j]; >> - portdev->out_vqs[i] = vqs[j + 1]; >> + portdev->in_vqs[i] = NULL; >> + portdev->out_vqs[i] = NULL; >> } >> } >> kfree(io_names); > So a queue once created will not be removed unless the module is > removed or the device is removed. That seems reasonable, port > hot-unplug will keep queues around, as is the case now. As we use the virtio-serial with only a few io-ports, the maxport queues may waste more memory. So we try to create queues when port hot-plug, as for port hot-unplug there's no change. > > Amit > > . > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] virtio-serial: set up vqs on demand 2012-01-12 1:20 [PATCH 0/2] virtio-serial: set up vqs on demand zanghongyong 2012-01-12 1:20 ` [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs zanghongyong 2012-01-12 1:20 ` [PATCH 2/2] virtio-serial: setup_port_vq when adding port zanghongyong @ 2012-02-01 9:40 ` Michael S. Tsirkin 2013-04-08 7:51 ` Amit Shah 3 siblings, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2012-02-01 9:40 UTC (permalink / raw) To: zanghongyong Cc: kvm, rusty, amit.shah, aliguori, xiaowei.yang, hanweidong, wusongwei, jiangningyu On Thu, Jan 12, 2012 at 09:20:05AM +0800, zanghongyong@huawei.com wrote: > From: Hongyong Zang <zanghongyong@huawei.com> > > Virtio-serial set up (max_ports+1)*2 vqs when device probes, but may not all > io_ports are used. > These patches create vqs of port0 and control port when probing the device, then > create io-vqs when called add_port(). > > Hongyong Zang (2): > virtio-pci: add setup_vqs flag in vp_try_to_find_vqs > virtio-serial: setup_port_vq when adding port > > drivers/char/virtio_console.c | 65 ++++++++++++++++++++++++++++++++++++++-- > drivers/virtio/virtio_pci.c | 17 ++++++++-- > 2 files changed, 74 insertions(+), 8 deletions(-) This wasn't Cc'd to the right list, look at MAINTAINERS please. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] virtio-serial: set up vqs on demand 2012-01-12 1:20 [PATCH 0/2] virtio-serial: set up vqs on demand zanghongyong ` (2 preceding siblings ...) 2012-02-01 9:40 ` [PATCH 0/2] virtio-serial: set up vqs on demand Michael S. Tsirkin @ 2013-04-08 7:51 ` Amit Shah 3 siblings, 0 replies; 10+ messages in thread From: Amit Shah @ 2013-04-08 7:51 UTC (permalink / raw) To: zanghongyong Cc: kvm, rusty, aliguori, xiaowei.yang, hanweidong, wusongwei, jiangningyu, asias, Virtualization List On (Thu) 12 Jan 2012 [09:20:05], zanghongyong@huawei.com wrote: > From: Hongyong Zang <zanghongyong@huawei.com> > > Virtio-serial set up (max_ports+1)*2 vqs when device probes, but may not all > io_ports are used. > These patches create vqs of port0 and control port when probing the device, then > create io-vqs when called add_port(). Hi, Can you resurrect this series? I think last time we were waiting for a v2, but looks like it got lost. Let me know if you're not interested anymore, and I can do something about it. Thanks, Amit ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-04-08 7:53 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-01-12 1:20 [PATCH 0/2] virtio-serial: set up vqs on demand zanghongyong 2012-01-12 1:20 ` [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs zanghongyong 2012-02-01 8:14 ` Amit Shah 2012-02-01 9:54 ` Michael S. Tsirkin 2012-02-03 3:56 ` Rusty Russell 2012-01-12 1:20 ` [PATCH 2/2] virtio-serial: setup_port_vq when adding port zanghongyong 2012-02-01 8:12 ` Amit Shah 2012-02-01 9:32 ` Zang Hongyong 2012-02-01 9:40 ` [PATCH 0/2] virtio-serial: set up vqs on demand Michael S. Tsirkin 2013-04-08 7:51 ` Amit Shah
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).