| Message ID | 1442266096.3494.15.camel@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 14/09/15 22:28, Charles (Chas) Williams wrote: > The xen store preserves this information across module invocations. > If you insmod netfront with two queues and later insmod again with one > queue, the backend will still believe you asked for two queues. Can you rewrite the commit message to be clearer? "If netfront connects with 2 (or more) queues and then reconnects with only 1 queue it fails to delete or rewrite the multi-queue-num-queues key and netback will try to use the wrong number of queues. Always write the num-queues field if the backend has multi-queue support." > --- a/drivers/net/xen-netfront.c > +++ b/drivers/net/xen-netfront.c > @@ -1819,11 +1819,7 @@ again: > goto destroy_ring; > } > > - if (num_queues == 1) { > - err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */ > - if (err) > - goto abort_transaction_no_dev_fatal; > - } else { > + if (xenbus_exists(xbt, dev->nodename, "multi-queue-num-queues")) { > /* Write the number of queues */ > err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues", > "%u", num_queues); Isn't this broken? It looks like it won't write the multi-queue-num-queues key the first time around. It think this should be conditional on multi-queue-max-queues existing (which is written by the backend if multi-queue is supported). David -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-09-15 at 11:59 +0100, David Vrabel wrote: > On 14/09/15 22:28, Charles (Chas) Williams wrote: > > The xen store preserves this information across module invocations. > > If you insmod netfront with two queues and later insmod again with one > > queue, the backend will still believe you asked for two queues. > > Can you rewrite the commit message to be clearer? > > "If netfront connects with 2 (or more) queues and then reconnects with > only 1 queue it fails to delete or rewrite the multi-queue-num-queues > key and netback will try to use the wrong number of queues. > > Always write the num-queues field if the backend has multi-queue support." Yes I can do that. > > > --- a/drivers/net/xen-netfront.c > > +++ b/drivers/net/xen-netfront.c > > @@ -1819,11 +1819,7 @@ again: > > goto destroy_ring; > > } > > > > - if (num_queues == 1) { > > - err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */ > > - if (err) > > - goto abort_transaction_no_dev_fatal; > > - } else { > > + if (xenbus_exists(xbt, dev->nodename, "multi-queue-num-queues")) { > > /* Write the number of queues */ > > err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues", > > "%u", num_queues); > > Isn't this broken? It looks like it won't write the > multi-queue-num-queues key the first time around. > > It think this should be conditional on multi-queue-max-queues existing > (which is written by the backend if multi-queue is supported). You are right. I am testing against the wrong key here. I should have tested for multi-queue-max-queues. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index f821a97..b53a681 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1819,11 +1819,7 @@ again: goto destroy_ring; } - if (num_queues == 1) { - err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */ - if (err) - goto abort_transaction_no_dev_fatal; - } else { + if (xenbus_exists(xbt, dev->nodename, "multi-queue-num-queues")) { /* Write the number of queues */ err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues", "%u", num_queues); @@ -1831,7 +1827,13 @@ again: message = "writing multi-queue-num-queues"; goto abort_transaction_no_dev_fatal; } + } + if (num_queues == 1) { + err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */ + if (err) + goto abort_transaction_no_dev_fatal; + } else { /* Write the keys for each queue */ for (i = 0; i < num_queues; ++i) { queue = &info->queues[i];
The xen store preserves this information across module invocations. If you insmod netfront with two queues and later insmod again with one queue, the backend will still believe you asked for two queues. Signed-off-by: Chas Williams <3chas3@gmail.com> --- drivers/net/xen-netfront.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)