Messages in this thread | | | Date | Thu, 28 Jul 2016 07:05:05 +0800 | From | Bob Liu <> | Subject | Re: [PATCH v3] xen-blkfront: dynamic configuration of per-vbd resources |
| |
On 07/27/2016 10:24 PM, Roger Pau Monné wrote: > On Wed, Jul 27, 2016 at 07:21:05PM +0800, Bob Liu wrote: >> >> On 07/27/2016 06:59 PM, Roger Pau Monné wrote: >>> On Wed, Jul 27, 2016 at 11:21:25AM +0800, Bob Liu wrote: >>> [...] >>>> +static ssize_t dynamic_reconfig_device(struct blkfront_info *info, ssize_t count) >>>> +{ >>>> + /* >>>> + * Prevent new requests even to software request queue. >>>> + */ >>>> + blk_mq_freeze_queue(info->rq); >>>> + >>>> + /* >>>> + * Guarantee no uncompleted reqs. >>>> + */ >>> >>> I'm also wondering, why do you need to guarantee that there are no >>> uncompleted requests? The resume procedure is going to call blkif_recover >>> that will take care of requeuing any unfinished requests that are on the >>> ring. >>> >> >> Because there may have requests in the software request queue with more segments than >> we can handle(if info->max_indirect_segments is reduced). >> >> The blkif_recover() can't handle this since blk-mq was introduced, >> because there is no way to iterate the sw-request queues(blk_fetch_request() can't be used by blk-mq). >> >> So there is a bug in blkif_recover(), I was thinking implement the suspend function of blkfront_driver like: > > Hm, this is a regression and should be fixed ASAP. I'm still not sure I > follow, don't blk_queue_max_segments change the number of segments the > requests on the queue are going to have? So that you will only have to > re-queue the requests already on the ring? >
That's not enough, request queues were split to software queues and hardware queues since blk-mq was introduced. We need to consider two more things: * Stop new requests be added to software queues before blk_queue_max_segments() is called(still using old 'max-indirect-segments'). I didn't see other way except call blk_mq_freeze_queue().
* Requests already in software queues but with old 'max-indirect-segments' also have to be re-queued based on new 'max-indirect-segments'. Neither blk-mq API can do this.
> Waiting for the whole queue to be flushed before suspending is IMHO not > acceptable, it introduces an unbounded delay during migration if the backend > is slow for some reason. >
Right, I also hope there is better solution.
-- Regards, -Bob
| |