linux-kernel - Re: [PATCH 2/2] block: move request

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] [day] [month] [year] [list]

Date:   Sun, 28 Jun 2020 14:23:07 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Luis Chamberlain <mcgrof@...nel.org>, axboe@...nel.dk,
        damien.lemoal@....com, ming.lei@...hat.com,
        martin.petersen@...cle.com, satyat@...gle.com
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] block: move request_queue member docs to kdoc

On 2020-06-23 15:03, Luis Chamberlain wrote:
>  /**
>   * struct request_queue - block device driver request queue
> + * @queue_ctx: software queue context

To me the description "software queues" is much more clear than
"software queue context". I wouldn't mind if the queue_ctx member
variable would be renamed to make its role more clear.

Please also mention that there is one software queue per CPU.

> + * @queue_hw_ctx: hw dispatch queues

How about mentioning that requests flow from software queues into
hardware queues, from hardware queues to the storage controller and also
that the block driver controls the number of hardware queues?

> + * @queuedata: the queue owner gets to use this for whatever they like.
> + *	ll_rw_blk doesn't touch it.

How about changing "queue owner" into "block driver"? Please leave out
the reference to ll_rw_blk since that source file was removed in 2008.
See also commit a168ee84c90b ("block: first step of splitting ll_rw_blk,
rename it").

> + * @id: ida allocated id for this queue.  Used to index queues from ioctx.

It seems to me that this ID is not only used to associate an ioctx with
a request queue but also to associate a block cgroup with a request
queue? See also blkg_lookup_slowpath().

> + * @bounce_gfp: queue needs bounce pages for pages above this limit
> + * @kobj: queue kobject

Please mention the path of this object, namely /sys/block/${bdev}/queue.

> + * @mq_kobj: mq queue kobject

Please mention the path of this object too, namely /sys/block/${bdev}/mq.

> + * @nr_requests: maximum number of of requests

Double "of"? Please mention that this is the maximum number of requests
per hardware queue. There is one set of tags per hardware queue and each
hardware queue has 'nr_requests' tags. See also queue_requests_store()
and blk_mq_update_nr_requests().

> + * @ksm: Inline crypto capabilities
> + * @nr_zones:
> + * @nr_zones: total number of zones of the device. This is always 0 for regular
> + *	block devices.

"@nr_zones" occurs twice?

> + * @debugfs_mutex: used to protect access to the @ebugfs_dir
>   * @debugfs_mutex: used to protect access to the @debugfs_dir

Double "@debugfs_mutex"?

Thanks,

Bart.