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.
