linux-kernel - Re: [PATCH 7/8] mq-deadline: add blk-mq adaptation of the deadline IO scheduler

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 16 Feb 2017 08:35:02 -0700
From:   Jens Axboe <axboe@...com>
To:     Paolo Valente <paolo.valente@...aro.org>
CC:     Jens Axboe <axboe@...nel.dk>, <linux-block@...r.kernel.org>,
        Linux-Kernal <linux-kernel@...r.kernel.org>, <osandov@...com>
Subject: Re: [PATCH 7/8] mq-deadline: add blk-mq adaptation of the deadline IO
 scheduler

On 02/16/2017 03:46 AM, Paolo Valente wrote:
> 
>> Il giorno 17 dic 2016, alle ore 01:12, Jens Axboe <axboe@...com> ha scritto:
>>
>> This is basically identical to deadline-iosched, except it registers
>> as a MQ capable scheduler. This is still a single queue design.
>>
>> Signed-off-by: Jens Axboe <axboe@...com>
> ...
>> +
>> +static void dd_merged_requests(struct request_queue *q, struct request *req,
>> +			       struct request *next)
>> +{
>> +	/*
>> +	 * if next expires before rq, assign its expire time to rq
>> +	 * and move into next position (next will be deleted) in fifo
>> +	 */
>> +	if (!list_empty(&req->queuelist) && !list_empty(&next->queuelist)) {
>> +		if (time_before((unsigned long)next->fifo_time,
>> +				(unsigned long)req->fifo_time)) {
>> +			list_move(&req->queuelist, &next->queuelist);
>> +			req->fifo_time = next->fifo_time;
>> +		}
>> +	}
>> +
> 
> Jens,
> while trying to imagine the possible causes of Bart's hang with
> bfq-mq, I've bumped into the following doubt: in the above function
> (in my case, in bfq-mq-'s equivalent of the above function), are
> we sure that neither req or next could EVER be in dd->dispatch instead
> of dd->fifo_list?  I've tried to verify it, but, although I think it has never
> happened in my tests, I was not able to make sure that no unlucky
> combination may ever happen (considering also the use of
> blk_rq_is_passthrough too, to decide where to put a new request).
> 
> I'm making a blunder, right?

If a request goes into dd->dispatch, it's going to be found for merging.
Hence we can never call the above on the request.

-- 
Jens Axboe