linux-kernel - Re: [PATCH v3] xen-blkfront: dynamic configuration of per-vbd resources

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160728074858.5iepk2n6h746okx6@mac>
Date:	Thu, 28 Jul 2016 09:49:07 +0200
From:	Roger Pau Monné <roger.pau@...rix.com>
To:	Bob Liu <bob.liu@...cle.com>
CC:	<linux-kernel@...r.kernel.org>, <xen-devel@...ts.xenproject.org>,
	<konrad.wilk@...cle.com>
Subject: Re: [PATCH v3] xen-blkfront: dynamic configuration of per-vbd
 resources

On Thu, Jul 28, 2016 at 07:05:05AM +0800, Bob Liu wrote:
> 
> 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().

Right, stopping the queues doesn't seem to be an issue?

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

I'm afraid I don't know much about the new blk-mq API, you will have to ask 
the blk-mq maintainers for solutions, since this was possible with the 
previous API (and I would consider a regression that the new blk-mq API 
doesn't allow it).

Roger.