Re: [PATCH v2 3/3] xen-blkfront: dynamic configuration of per-vbd resources
From: Roger Pau Monné
Date: Tue Jul 26 2016 - 11:51:34 EST
On Tue, Jul 26, 2016 at 04:58:10PM +0800, Bob Liu wrote:
> On 07/26/2016 04:44 PM, Roger Pau Monné wrote:
> > On Tue, Jul 26, 2016 at 01:19:37PM +0800, Bob Liu wrote:
> >> The current VBD layer reserves buffer space for each attached device based on
> >> three statically configured settings which are read at boot time.
> >> * max_indirect_segs: Maximum amount of segments.
> >> * max_ring_page_order: Maximum order of pages to be used for the shared ring.
> >> * max_queues: Maximum of queues(rings) to be used.
> >> But the storage backend, workload, and guest memory result in very different
> >> tuning requirements. It's impossible to centrally predict application
> >> characteristics so it's best to leave allow the settings can be dynamiclly
> >> adjusted based on workload inside the Guest.
> >> Usage:
> >> Show current values:
> >> cat /sys/devices/vbd-xxx/max_indirect_segs
> >> cat /sys/devices/vbd-xxx/max_ring_page_order
> >> cat /sys/devices/vbd-xxx/max_queues
> >> Write new values:
> >> echo <new value> > /sys/devices/vbd-xxx/max_indirect_segs
> >> echo <new value> > /sys/devices/vbd-xxx/max_ring_page_order
> >> echo <new value> > /sys/devices/vbd-xxx/max_queues
> >> Signed-off-by: Bob Liu <bob.liu@xxxxxxxxxx>
> >> --
> >> v2: Rename to max_ring_page_order and rm the waiting code suggested by Roger.
> >> ---
> >> drivers/block/xen-blkfront.c | 275 +++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 269 insertions(+), 6 deletions(-)
> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >> index 1b4c380..ff5ebe5 100644
> >> --- a/drivers/block/xen-blkfront.c
> >> +++ b/drivers/block/xen-blkfront.c
> >> @@ -212,6 +212,11 @@ struct blkfront_info
> >> /* Save uncomplete reqs and bios for migration. */
> >> struct list_head requests;
> >> struct bio_list bio_list;
> >> + /* For dynamic configuration. */
> >> + unsigned int reconfiguring:1;
> >> + int new_max_indirect_segments;
> > Can't you just use max_indirect_segments? Is it really needed to introduce a
> > new struct member?
> >> + int max_ring_page_order;
> >> + int max_queues;
> Do you mean also get rid of these two new struct members?
> I'll think about that.
Oh no, those two are fine, and AFAICT are needed because now every blkfront
instance can have it's own max number of queues or ring pages. What I think
can be removed is the introduction of new_max_indirect_segments, and instead
just use the already available max_indirect_segments field in that same