Re: [PATCH v2 3/3] xen-blkfront: dynamic configuration of per-vbd resources

[Roger Pau Monné]

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 
struct.

Roger.