From: Li Liu <john.liuli@huawei.com> To: "Michael S. Tsirkin" <mst@redhat.com> Cc: <linux-kernel@vger.kernel.org>, <qemu-devel@nongnu.org>, <peter.huangpeng@huawei.com>, <rusty@rustcorp.com.au>, <virtualization@lists.linux-foundation.org>, <n.nikolaev@virtualopensystems.com>, <yingshiuan.pan@gmail.com>, <remy.gauguey@cea.fr>, <joel.schopp@amd.com> Subject: Re: [RFC PATCH 2/2] Assign a new irq handler while irqfd enabled Date: Mon, 27 Oct 2014 19:04:11 +0800 [thread overview] Message-ID: <544E26AB.6090200@huawei.com> (raw) In-Reply-To: <20141026115646.GB5497@redhat.com> On 2014/10/26 19:56, Michael S. Tsirkin wrote: > On Sat, Oct 25, 2014 at 04:24:54PM +0800, john.liuli wrote: >> From: Li Liu <john.liuli@huawei.com> >> >> This irq handler will get the interrupt reason from a >> shared memory. And will be assigned only while irqfd >> enabled. >> >> Signed-off-by: Li Liu <john.liuli@huawei.com> >> --- >> drivers/virtio/virtio_mmio.c | 34 ++++++++++++++++++++++++++++++++-- >> 1 file changed, 32 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c >> index 28ddb55..7229605 100644 >> --- a/drivers/virtio/virtio_mmio.c >> +++ b/drivers/virtio/virtio_mmio.c >> @@ -259,7 +259,31 @@ static irqreturn_t vm_interrupt(int irq, void *opaque) >> return ret; >> } >> >> +/* Notify all virtqueues on an interrupt. */ >> +static irqreturn_t vm_interrupt_irqfd(int irq, void *opaque) >> +{ >> + struct virtio_mmio_device *vm_dev = opaque; >> + struct virtio_mmio_vq_info *info; >> + unsigned long status; >> + unsigned long flags; >> + irqreturn_t ret = IRQ_NONE; >> >> + /* Read the interrupt reason and reset it */ >> + status = *vm_dev->isr_mem; >> + *vm_dev->isr_mem = 0x0; > > you are reading and modifying shared memory > without atomics and any memory barriers. > Why is this safe? > good catch, a stupid mistake. >> + >> + if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) { >> + virtio_config_changed(&vm_dev->vdev); >> + ret = IRQ_HANDLED; >> + } >> + >> + spin_lock_irqsave(&vm_dev->lock, flags); >> + list_for_each_entry(info, &vm_dev->virtqueues, node) >> + ret |= vring_interrupt(irq, info->vq); >> + spin_unlock_irqrestore(&vm_dev->lock, flags); >> + >> + return ret; >> +} >> >> static void vm_del_vq(struct virtqueue *vq) >> { > > So you invoke callbacks for all VQs. > This won't scale well as the number of VQs grows, will it? > >> @@ -391,6 +415,7 @@ error_available: >> return ERR_PTR(err); >> } >> >> +#define VIRTIO_MMIO_F_IRQFD (1 << 7) >> static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs, >> struct virtqueue *vqs[], >> vq_callback_t *callbacks[], >> @@ -400,8 +425,13 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs, >> unsigned int irq = platform_get_irq(vm_dev->pdev, 0); >> int i, err; >> >> - err = request_irq(irq, vm_interrupt, IRQF_SHARED, >> - dev_name(&vdev->dev), vm_dev); >> + if (*vm_dev->isr_mem & VIRTIO_MMIO_F_IRQFD) { >> + err = request_irq(irq, vm_interrupt_irqfd, IRQF_SHARED, >> + dev_name(&vdev->dev), vm_dev); >> + } else { >> + err = request_irq(irq, vm_interrupt, IRQF_SHARED, >> + dev_name(&vdev->dev), vm_dev); >> + } >> if (err) >> return err; > > > So still a single interrupt for all VQs. > Again this doesn't scale: a single CPU has to handle > interrupts for all of them. > I think you need to find a way to get per-VQ interrupts. Yeah, AFAIK it's impossible to distribute works to different CPUs with only one irq without MSI-X kind mechanism. Assign multiple gsis to one device, obviously it's consumptive and not scalable. Any ideas? Thx. > >> -- >> 1.7.9.5 >> > > . >
WARNING: multiple messages have this Message-ID (diff)
From: Li Liu <john.liuli@huawei.com> To: "Michael S. Tsirkin" <mst@redhat.com> Cc: joel.schopp@amd.com, yingshiuan.pan@gmail.com, qemu-devel@nongnu.org, linux-kernel@vger.kernel.org, remy.gauguey@cea.fr, rusty@rustcorp.com.au, peter.huangpeng@huawei.com, n.nikolaev@virtualopensystems.com, virtualization@lists.linux-foundation.org Subject: Re: [Qemu-devel] [RFC PATCH 2/2] Assign a new irq handler while irqfd enabled Date: Mon, 27 Oct 2014 19:04:11 +0800 [thread overview] Message-ID: <544E26AB.6090200@huawei.com> (raw) In-Reply-To: <20141026115646.GB5497@redhat.com> On 2014/10/26 19:56, Michael S. Tsirkin wrote: > On Sat, Oct 25, 2014 at 04:24:54PM +0800, john.liuli wrote: >> From: Li Liu <john.liuli@huawei.com> >> >> This irq handler will get the interrupt reason from a >> shared memory. And will be assigned only while irqfd >> enabled. >> >> Signed-off-by: Li Liu <john.liuli@huawei.com> >> --- >> drivers/virtio/virtio_mmio.c | 34 ++++++++++++++++++++++++++++++++-- >> 1 file changed, 32 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c >> index 28ddb55..7229605 100644 >> --- a/drivers/virtio/virtio_mmio.c >> +++ b/drivers/virtio/virtio_mmio.c >> @@ -259,7 +259,31 @@ static irqreturn_t vm_interrupt(int irq, void *opaque) >> return ret; >> } >> >> +/* Notify all virtqueues on an interrupt. */ >> +static irqreturn_t vm_interrupt_irqfd(int irq, void *opaque) >> +{ >> + struct virtio_mmio_device *vm_dev = opaque; >> + struct virtio_mmio_vq_info *info; >> + unsigned long status; >> + unsigned long flags; >> + irqreturn_t ret = IRQ_NONE; >> >> + /* Read the interrupt reason and reset it */ >> + status = *vm_dev->isr_mem; >> + *vm_dev->isr_mem = 0x0; > > you are reading and modifying shared memory > without atomics and any memory barriers. > Why is this safe? > good catch, a stupid mistake. >> + >> + if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) { >> + virtio_config_changed(&vm_dev->vdev); >> + ret = IRQ_HANDLED; >> + } >> + >> + spin_lock_irqsave(&vm_dev->lock, flags); >> + list_for_each_entry(info, &vm_dev->virtqueues, node) >> + ret |= vring_interrupt(irq, info->vq); >> + spin_unlock_irqrestore(&vm_dev->lock, flags); >> + >> + return ret; >> +} >> >> static void vm_del_vq(struct virtqueue *vq) >> { > > So you invoke callbacks for all VQs. > This won't scale well as the number of VQs grows, will it? > >> @@ -391,6 +415,7 @@ error_available: >> return ERR_PTR(err); >> } >> >> +#define VIRTIO_MMIO_F_IRQFD (1 << 7) >> static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs, >> struct virtqueue *vqs[], >> vq_callback_t *callbacks[], >> @@ -400,8 +425,13 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs, >> unsigned int irq = platform_get_irq(vm_dev->pdev, 0); >> int i, err; >> >> - err = request_irq(irq, vm_interrupt, IRQF_SHARED, >> - dev_name(&vdev->dev), vm_dev); >> + if (*vm_dev->isr_mem & VIRTIO_MMIO_F_IRQFD) { >> + err = request_irq(irq, vm_interrupt_irqfd, IRQF_SHARED, >> + dev_name(&vdev->dev), vm_dev); >> + } else { >> + err = request_irq(irq, vm_interrupt, IRQF_SHARED, >> + dev_name(&vdev->dev), vm_dev); >> + } >> if (err) >> return err; > > > So still a single interrupt for all VQs. > Again this doesn't scale: a single CPU has to handle > interrupts for all of them. > I think you need to find a way to get per-VQ interrupts. Yeah, AFAIK it's impossible to distribute works to different CPUs with only one irq without MSI-X kind mechanism. Assign multiple gsis to one device, obviously it's consumptive and not scalable. Any ideas? Thx. > >> -- >> 1.7.9.5 >> > > . >
next prev parent reply other threads:[~2014-10-27 11:06 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-10-25 8:24 [RFC PATCH 0/2] virtio-mmio: add irqfd support for vhost-net based on virtio-mmio john.liuli 2014-10-25 8:24 ` [Qemu-devel] " john.liuli 2014-10-25 8:24 ` [RFC PATCH 1/2] Add a new register offset let interrupt reason available john.liuli 2014-10-25 8:24 ` [Qemu-devel] " john.liuli 2014-10-26 12:01 ` Michael S. Tsirkin 2014-10-26 12:01 ` Michael S. Tsirkin 2014-10-26 12:01 ` [Qemu-devel] " Michael S. Tsirkin 2014-10-25 8:24 ` john.liuli 2014-10-25 8:24 ` [RFC PATCH 2/2] Assign a new irq handler while irqfd enabled john.liuli 2014-10-25 8:24 ` [Qemu-devel] " john.liuli 2014-10-26 11:56 ` Michael S. Tsirkin 2014-10-26 11:56 ` Michael S. Tsirkin 2014-10-26 11:56 ` [Qemu-devel] " Michael S. Tsirkin 2014-10-27 11:04 ` Li Liu 2014-10-27 11:04 ` Li Liu [this message] 2014-10-27 11:04 ` [Qemu-devel] " Li Liu 2014-10-27 12:03 ` Michael S. Tsirkin 2014-10-27 12:03 ` Michael S. Tsirkin 2014-10-27 12:03 ` [Qemu-devel] " Michael S. Tsirkin 2014-10-25 8:24 ` john.liuli 2014-10-26 11:52 ` [RFC PATCH 0/2] virtio-mmio: add irqfd support for vhost-net based on virtio-mmio Michael S. Tsirkin 2014-10-26 11:52 ` Michael S. Tsirkin 2014-10-26 11:52 ` [Qemu-devel] " Michael S. Tsirkin 2014-10-27 9:19 ` Li Liu 2014-10-27 9:19 ` [Qemu-devel] " Li Liu 2014-10-27 10:48 ` Michael S. Tsirkin 2014-10-27 10:48 ` Michael S. Tsirkin 2014-10-27 10:48 ` [Qemu-devel] " Michael S. Tsirkin 2014-10-27 9:19 ` Li Liu 2014-10-27 9:37 ` [Qemu-devel] " Peter Maydell 2014-10-27 9:37 ` Peter Maydell 2014-10-27 9:37 ` Peter Maydell 2014-10-27 11:23 ` Li Liu 2014-10-27 11:23 ` Li Liu 2014-10-27 11:23 ` Li Liu 2014-10-27 11:58 ` Peter Maydell 2014-10-27 11:58 ` Peter Maydell 2014-10-27 11:58 ` Peter Maydell 2014-11-05 9:30 ` Christoffer Dall 2014-11-05 9:30 ` Christoffer Dall 2014-11-05 9:30 ` Christoffer Dall 2014-11-05 8:43 ` Eric Auger 2014-11-05 8:43 ` Eric Auger 2014-11-06 1:59 ` Shannon Zhao 2014-11-06 1:59 ` Shannon Zhao 2014-11-06 9:24 ` Li Liu 2014-11-06 9:24 ` Li Liu 2014-11-06 9:24 ` Li Liu 2014-11-06 1:59 ` Shannon Zhao 2014-11-05 8:43 ` Eric Auger
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=544E26AB.6090200@huawei.com \ --to=john.liuli@huawei.com \ --cc=joel.schopp@amd.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mst@redhat.com \ --cc=n.nikolaev@virtualopensystems.com \ --cc=peter.huangpeng@huawei.com \ --cc=qemu-devel@nongnu.org \ --cc=remy.gauguey@cea.fr \ --cc=rusty@rustcorp.com.au \ --cc=virtualization@lists.linux-foundation.org \ --cc=yingshiuan.pan@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.