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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <e0ef6987-adb9-c53d-d540-1d50cdc967bc@fb.com>
Date:   Mon, 16 Jan 2017 18:47:18 -0800
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 12/22/2016 09:07 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>
>> ---
> 
> ...
> 
>> diff --git a/block/mq-deadline.c b/block/mq-deadline.c
>> new file mode 100644
>> index 000000000000..3cb9de21ab21
>> --- /dev/null
>> +++ b/block/mq-deadline.c
>> ...
>> +/*
>> + * remove rq from rbtree and fifo.
>> + */
>> +static void deadline_remove_request(struct request_queue *q, struct request *rq)
>> +{
>> +	struct deadline_data *dd = q->elevator->elevator_data;
>> +
>> +	list_del_init(&rq->queuelist);
>> +
>> +	/*
>> +	 * We might not be on the rbtree, if we are doing an insert merge
>> +	 */
>> +	if (!RB_EMPTY_NODE(&rq->rb_node))
>> +		deadline_del_rq_rb(dd, rq);
>> +
> 
> I've been scratching my head on the last three instructions, but at no
> avail.  If I understand correctly, the
> list_del_init(&rq->queue list);
> removes rq from the fifo list.  But, if so, I don't understand how it
> could be possible that rq has not been added to the rb_tree too.
>
> Another interpretation that I tried is that the above three lines
> handle correctly the following case where rq has not been inserted at
> all into deadline fifo queue and rb tree: when dd_insert_request was
> executed for rq, blk_mq_sched_try_insert_merge succeeded.  Yet, the
> list_del_init(&rq->queue list);
> does not seem to make sense.
> 
> Could you please shed some light on this for me?

I think you are correct, we don't need to touch ->queuelist for the case
where RB_EMPTY_NODE() is true. Minor detail, the list is already empty,
so it does no harm.

-- 
Jens Axboe