Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors
On 10/15/2014 01:40 PM, Rusty Russell wrote: Jason Wang jasow...@redhat.com writes: Below should be useful for some experiments Jason is doing. I thought I'd send it out for early review/feedback. event idx feature allows us to defer interrupts until a specific # of descriptors were used. Sometimes it might be useful to get an interrupt after a specific descriptor, regardless. This adds a descriptor flag for this, and an API to create an urgent output descriptor. This is still an RFC: we'll need a feature bit for drivers to detect this, but we've run out of feature bits for virtio 0.X. For experimentation purposes, drivers can assume this is set, or add a driver-specific feature bit. Signed-off-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Jason Wang jasow...@redhat.com The new VRING_DESC_F_URGENT bit is theoretically nicer, but for networking (which tends to take packets in order) couldn't we just set the event counter to give us a tx interrupt at the packet we want? Cheers, Rusty. Yes, we could. Recent RFC of enabling tx interrupt use this. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors
Jason Wang jasow...@redhat.com writes: Below should be useful for some experiments Jason is doing. I thought I'd send it out for early review/feedback. event idx feature allows us to defer interrupts until a specific # of descriptors were used. Sometimes it might be useful to get an interrupt after a specific descriptor, regardless. This adds a descriptor flag for this, and an API to create an urgent output descriptor. This is still an RFC: we'll need a feature bit for drivers to detect this, but we've run out of feature bits for virtio 0.X. For experimentation purposes, drivers can assume this is set, or add a driver-specific feature bit. Signed-off-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Jason Wang jasow...@redhat.com The new VRING_DESC_F_URGENT bit is theoretically nicer, but for networking (which tends to take packets in order) couldn't we just set the event counter to give us a tx interrupt at the packet we want? Cheers, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors
On 10/12/2014 05:27 PM, Michael S. Tsirkin wrote:
On Sat, Oct 11, 2014 at 03:16:44PM +0800, Jason Wang wrote:
Below should be useful for some experiments Jason is doing.
I thought I'd send it out for early review/feedback.
event idx feature allows us to defer interrupts until
a specific # of descriptors were used.
Sometimes it might be useful to get an interrupt after
a specific descriptor, regardless.
This adds a descriptor flag for this, and an API
to create an urgent output descriptor.
This is still an RFC:
we'll need a feature bit for drivers to detect this,
but we've run out of feature bits for virtio 0.X.
For experimentation purposes, drivers can assume
this is set, or add a driver-specific feature bit.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
I see that as compared to my original patch, you have
added a new flag: VRING_AVAIL_F_NO_URGENT_INTERRUPT
I don't think it's necessary, see below.
As such, I think this patch should be split:
- original patch adding support for urgent descriptors
- a patch adding virtqueue_enable/disable_cb_urgent(_prepare)?
Not sure this is a good idea, since the api of first patch is in-completed.
---
drivers/virtio/virtio_ring.c | 75
+---
include/linux/virtio.h | 14
include/uapi/linux/virtio_ring.h | 5 ++-
3 files changed, 89 insertions(+), 5 deletions(-)
[...]
+unsigned virtqueue_enable_cb_prepare_urgent(struct virtqueue *_vq)
+{
+struct vring_virtqueue *vq = to_vvq(_vq);
+u16 last_used_idx;
+
+START_USE(vq);
+vq-vring.avail-flags = ~VRING_AVAIL_F_NO_URGENT_INTERRUPT;
+last_used_idx = vq-last_used_idx;
+END_USE(vq);
+return last_used_idx;
+}
+EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare_urgent);
+
You can implement virtqueue_enable_cb_prepare_urgent
simply by clearing ~VRING_AVAIL_F_NO_INTERRUPT.
The effect is same: host sends interrupts only if there
is an urgent descriptor.
Seems not, consider the case when event index was disabled. This will
turn on all interrupts.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors
On Mon, Oct 13, 2014 at 02:22:12PM +0800, Jason Wang wrote:
On 10/12/2014 05:27 PM, Michael S. Tsirkin wrote:
On Sat, Oct 11, 2014 at 03:16:44PM +0800, Jason Wang wrote:
Below should be useful for some experiments Jason is doing.
I thought I'd send it out for early review/feedback.
event idx feature allows us to defer interrupts until
a specific # of descriptors were used.
Sometimes it might be useful to get an interrupt after
a specific descriptor, regardless.
This adds a descriptor flag for this, and an API
to create an urgent output descriptor.
This is still an RFC:
we'll need a feature bit for drivers to detect this,
but we've run out of feature bits for virtio 0.X.
For experimentation purposes, drivers can assume
this is set, or add a driver-specific feature bit.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
I see that as compared to my original patch, you have
added a new flag: VRING_AVAIL_F_NO_URGENT_INTERRUPT
I don't think it's necessary, see below.
As such, I think this patch should be split:
- original patch adding support for urgent descriptors
- a patch adding virtqueue_enable/disable_cb_urgent(_prepare)?
Not sure this is a good idea, since the api of first patch is in-completed.
---
drivers/virtio/virtio_ring.c | 75
+---
include/linux/virtio.h | 14
include/uapi/linux/virtio_ring.h | 5 ++-
3 files changed, 89 insertions(+), 5 deletions(-)
[...]
+unsigned virtqueue_enable_cb_prepare_urgent(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 last_used_idx;
+
+ START_USE(vq);
+ vq-vring.avail-flags = ~VRING_AVAIL_F_NO_URGENT_INTERRUPT;
+ last_used_idx = vq-last_used_idx;
+ END_USE(vq);
+ return last_used_idx;
+}
+EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare_urgent);
+
You can implement virtqueue_enable_cb_prepare_urgent
simply by clearing ~VRING_AVAIL_F_NO_INTERRUPT.
The effect is same: host sends interrupts only if there
is an urgent descriptor.
Seems not, consider the case when event index was disabled. This will
turn on all interrupts.
This means that a legacy device without event index support
will get more interrupts.
Sounds reasonable.
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors
On Sat, Oct 11, 2014 at 03:16:44PM +0800, Jason Wang wrote:
Below should be useful for some experiments Jason is doing.
I thought I'd send it out for early review/feedback.
event idx feature allows us to defer interrupts until
a specific # of descriptors were used.
Sometimes it might be useful to get an interrupt after
a specific descriptor, regardless.
This adds a descriptor flag for this, and an API
to create an urgent output descriptor.
This is still an RFC:
we'll need a feature bit for drivers to detect this,
but we've run out of feature bits for virtio 0.X.
For experimentation purposes, drivers can assume
this is set, or add a driver-specific feature bit.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
I see that as compared to my original patch, you have
added a new flag: VRING_AVAIL_F_NO_URGENT_INTERRUPT
I don't think it's necessary, see below.
As such, I think this patch should be split:
- original patch adding support for urgent descriptors
- a patch adding virtqueue_enable/disable_cb_urgent(_prepare)?
---
drivers/virtio/virtio_ring.c | 75
+---
include/linux/virtio.h | 14
include/uapi/linux/virtio_ring.h | 5 ++-
3 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 4d08f45a..a5188c6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -115,6 +115,7 @@ static inline struct scatterlist *sg_next_arr(struct
scatterlist *sg,
/* Set up an indirect table of descriptors and add it to the queue. */
static inline int vring_add_indirect(struct vring_virtqueue *vq,
+ bool urgent,
struct scatterlist *sgs[],
struct scatterlist *(*next)
(struct scatterlist *, unsigned int *),
@@ -173,6 +174,8 @@ static inline int vring_add_indirect(struct
vring_virtqueue *vq,
/* Use a single buffer which doesn't continue */
head = vq-free_head;
vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
+ if (urgent)
+ vq-vring.desc[head].flags |= VRING_DESC_F_URGENT;
vq-vring.desc[head].addr = virt_to_phys(desc);
/* kmemleak gives a false positive, as it's hidden by virt_to_phys */
kmemleak_ignore(desc);
@@ -185,6 +188,7 @@ static inline int vring_add_indirect(struct
vring_virtqueue *vq,
}
static inline int virtqueue_add(struct virtqueue *_vq,
+ bool urgent,
struct scatterlist *sgs[],
struct scatterlist *(*next)
(struct scatterlist *, unsigned int *),
@@ -227,7 +231,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* If the host supports indirect descriptor tables, and we have multiple
* buffers, then go indirect. FIXME: tune this threshold */
if (vq-indirect total_sg 1 vq-vq.num_free) {
- head = vring_add_indirect(vq, sgs, next, total_sg, total_out,
+ head = vring_add_indirect(vq, urgent, sgs, next, total_sg,
total_out,
total_in,
out_sgs, in_sgs, gfp);
if (likely(head = 0))
@@ -256,6 +260,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
for (n = 0; n out_sgs; n++) {
for (sg = sgs[n]; sg; sg = next(sg, total_out)) {
vq-vring.desc[i].flags = VRING_DESC_F_NEXT;
+ if (urgent) {
+ vq-vring.desc[head].flags |=
VRING_DESC_F_URGENT;
+ urgent = false;
+ }
vq-vring.desc[i].addr = sg_phys(sg);
vq-vring.desc[i].len = sg-length;
prev = i;
@@ -265,6 +273,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
for (; n (out_sgs + in_sgs); n++) {
for (sg = sgs[n]; sg; sg = next(sg, total_in)) {
vq-vring.desc[i].flags =
VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
+ if (urgent) {
+ vq-vring.desc[head].flags |=
VRING_DESC_F_URGENT;
+ urgent = false;
+ }
vq-vring.desc[i].addr = sg_phys(sg);
vq-vring.desc[i].len = sg-length;
prev = i;
@@ -305,6 +317,8 @@ add_head:
/**
* virtqueue_add_sgs - expose buffers to other end
+ * @urgent: in case virtqueue_enable_cb_delayed was called, cause an
interrupt
+ * after this descriptor was completed
* @vq: the struct virtqueue we're talking about.
* @sgs: array of terminated scatterlists.
*
[PATCH net-next RFC 1/3] virtio: support for urgent descriptors
Below should be useful for some experiments Jason is doing.
I thought I'd send it out for early review/feedback.
event idx feature allows us to defer interrupts until
a specific # of descriptors were used.
Sometimes it might be useful to get an interrupt after
a specific descriptor, regardless.
This adds a descriptor flag for this, and an API
to create an urgent output descriptor.
This is still an RFC:
we'll need a feature bit for drivers to detect this,
but we've run out of feature bits for virtio 0.X.
For experimentation purposes, drivers can assume
this is set, or add a driver-specific feature bit.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/virtio/virtio_ring.c | 75 +---
include/linux/virtio.h | 14
include/uapi/linux/virtio_ring.h | 5 ++-
3 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 4d08f45a..a5188c6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -115,6 +115,7 @@ static inline struct scatterlist *sg_next_arr(struct
scatterlist *sg,
/* Set up an indirect table of descriptors and add it to the queue. */
static inline int vring_add_indirect(struct vring_virtqueue *vq,
+bool urgent,
struct scatterlist *sgs[],
struct scatterlist *(*next)
(struct scatterlist *, unsigned int *),
@@ -173,6 +174,8 @@ static inline int vring_add_indirect(struct vring_virtqueue
*vq,
/* Use a single buffer which doesn't continue */
head = vq-free_head;
vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
+ if (urgent)
+ vq-vring.desc[head].flags |= VRING_DESC_F_URGENT;
vq-vring.desc[head].addr = virt_to_phys(desc);
/* kmemleak gives a false positive, as it's hidden by virt_to_phys */
kmemleak_ignore(desc);
@@ -185,6 +188,7 @@ static inline int vring_add_indirect(struct vring_virtqueue
*vq,
}
static inline int virtqueue_add(struct virtqueue *_vq,
+ bool urgent,
struct scatterlist *sgs[],
struct scatterlist *(*next)
(struct scatterlist *, unsigned int *),
@@ -227,7 +231,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* If the host supports indirect descriptor tables, and we have multiple
* buffers, then go indirect. FIXME: tune this threshold */
if (vq-indirect total_sg 1 vq-vq.num_free) {
- head = vring_add_indirect(vq, sgs, next, total_sg, total_out,
+ head = vring_add_indirect(vq, urgent, sgs, next, total_sg,
total_out,
total_in,
out_sgs, in_sgs, gfp);
if (likely(head = 0))
@@ -256,6 +260,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
for (n = 0; n out_sgs; n++) {
for (sg = sgs[n]; sg; sg = next(sg, total_out)) {
vq-vring.desc[i].flags = VRING_DESC_F_NEXT;
+ if (urgent) {
+ vq-vring.desc[head].flags |=
VRING_DESC_F_URGENT;
+ urgent = false;
+ }
vq-vring.desc[i].addr = sg_phys(sg);
vq-vring.desc[i].len = sg-length;
prev = i;
@@ -265,6 +273,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
for (; n (out_sgs + in_sgs); n++) {
for (sg = sgs[n]; sg; sg = next(sg, total_in)) {
vq-vring.desc[i].flags =
VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
+ if (urgent) {
+ vq-vring.desc[head].flags |=
VRING_DESC_F_URGENT;
+ urgent = false;
+ }
vq-vring.desc[i].addr = sg_phys(sg);
vq-vring.desc[i].len = sg-length;
prev = i;
@@ -305,6 +317,8 @@ add_head:
/**
* virtqueue_add_sgs - expose buffers to other end
+ * @urgent: in case virtqueue_enable_cb_delayed was called, cause an interrupt
+ * after this descriptor was completed
* @vq: the struct virtqueue we're talking about.
* @sgs: array of terminated scatterlists.
* @out_num: the number of scatterlists readable by other side
@@ -337,7 +351,7 @@ int virtqueue_add_sgs(struct virtqueue *_vq,
for (sg = sgs[i]; sg; sg = sg_next(sg))
total_in++;
}
- return virtqueue_add(_vq, sgs, sg_next_chained,
+ return virtqueue_add(_vq, false, sgs, sg_next_chained,
total_out, total_in,
