* Re: [PATCH 2/2] virtio-serial: setup_port_vq when adding port [not found] ` <1326331207-10339-3-git-send-email-zanghongyong@huawei.com> @ 2012-02-01 8:12 ` Amit Shah 2012-02-01 9:32 ` Zang Hongyong 0 siblings, 1 reply; 3+ 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] 3+ messages in thread
* Re: [PATCH 2/2] virtio-serial: setup_port_vq when adding port 2012-02-01 8:12 ` [PATCH 2/2] virtio-serial: setup_port_vq when adding port Amit Shah @ 2012-02-01 9:32 ` Zang Hongyong 0 siblings, 0 replies; 3+ 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] 3+ messages in thread
[parent not found: <1326331207-10339-2-git-send-email-zanghongyong@huawei.com>]
* Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs [not found] ` <1326331207-10339-2-git-send-email-zanghongyong@huawei.com> @ 2012-02-01 8:14 ` Amit Shah 0 siblings, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2012-02-01 9:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1326331207-10339-1-git-send-email-zanghongyong@huawei.com>
[not found] ` <1326331207-10339-3-git-send-email-zanghongyong@huawei.com>
2012-02-01 8:12 ` [PATCH 2/2] virtio-serial: setup_port_vq when adding port Amit Shah
2012-02-01 9:32 ` Zang Hongyong
[not found] ` <1326331207-10339-2-git-send-email-zanghongyong@huawei.com>
2012-02-01 8:14 ` [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs 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).