linux-kernel - Re: [PATCH 5/7] blk-mq-sched: add framework for MQ capable IO schedulers

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: <cb5d74ae-6f27-a1c4-4e8b-93398f556b94@sandisk.com>
Date:   Wed, 14 Dec 2016 11:31:46 +0100
From:   Bart Van Assche <bart.vanassche@...disk.com>
To:     Jens Axboe <axboe@...com>, <axboe@...nel.dk>,
        <linux-block@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     <paolo.valente@...aro.org>, <osandov@...com>
Subject: Re: [PATCH 5/7] blk-mq-sched: add framework for MQ capable IO
 schedulers

On 12/13/2016 04:14 PM, Jens Axboe wrote:
> On 12/13/2016 06:56 AM, Bart Van Assche wrote:
>> On 12/08/2016 09:13 PM, Jens Axboe wrote:
>>> +struct request *blk_mq_sched_alloc_shadow_request(struct request_queue *q,
>>> +						  struct blk_mq_alloc_data *data,
>>> +						  struct blk_mq_tags *tags,
>>> +						  atomic_t *wait_index)
>>> +{
>>
>> Using the word "shadow" in the function name suggests to me that there 
>> is a shadow request for every request and a request for every shadow 
>> request. However, my understanding from the code is that there can be 
>> requests without shadow requests (for e.g. a flush) and shadow requests 
>> without requests. Shouldn't the name of this function reflect that, e.g. 
>> by using "sched" or "elv" in the function name instead of "shadow"?
> 
> Shadow might not be the best name. Most do have shadows though, it's
> only the rare exception like the flush, that you mention. I'll see if I
> can come up with a better name.

Hello Jens,

One aspect of this patch series that might turn out to be a maintenance
burden is the copying between original and shadow requests. It is easy
to overlook that rq_copy() has to be updated if a field would ever be
added to struct request. Additionally, having to allocate two requests
structures per I/O instead of one will have a runtime overhead. Do you
think the following approach would work?
- Instead of using two request structures per I/O, only use a single
  request structure.
- Instead of storing one tag in the request structure, store two tags
  in that structure. One tag comes from the I/O scheduler tag set
  (size: nr_requests) and the other from the tag set associated with
  the block driver (size: HBA queue depth).
- Only add a request to the hctx dispatch list after a block driver tag
  has been assigned. This means that an I/O scheduler must keep a
  request structure on a list it manages itself as long as no block
  driver tag has been assigned.
- sysfs_list_show() is modified such that it shows both tags.

Thanks,

Bart.