[PATCH 0/2] block: kdocify the request_queue

[PATCH 0/2] block: kdocify the request_queue

[Luis Chamberlain]

Bart suggested we add docs for the degugfs_mutex, while at it I noticed
we don't have a nice way to add docs for members of the request_queue so
I figured I'd add one as I drive by.

Come bike shed with me.

Luis Chamberlain (2):
  block: add initial kdoc over the request_queue
  block: move request_queue member docs to kdoc

 include/linux/blkdev.h | 159 ++++++++++++++++-------------------------
 1 file changed, 60 insertions(+), 99 deletions(-)

-- 
2.26.2

[PATCH 1/2] block: add initial kdoc over the request_queue

[Luis Chamberlain]

We start off with an initial description of the request_queue data
structure, followed by describing the purpose of the debugfs_mutex
debugfs_dir, and blk_trace.

Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 include/linux/blkdev.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 70461b347169..ea319c2b0593 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -394,6 +394,26 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
 
 #endif /* CONFIG_BLK_DEV_ZONED */
 
+/**
+ * struct request_queue - block device driver request queue
+ * @debugfs_mutex: used to protect access to the @debugfs_dir
+ * @blk_trace: used by blktrace to keep track of setup / tracing
+ * @debugfs_dir: directory created to place debugfs information. This is always
+ *	created for make_request and request-based block drivers upon
+ *	initialization. blktrace requires for this directory to be created,
+ *	and so it will be created on demand if its block driver type does not
+ *	create it opon initialization.
+ *
+ * The request_queue is used to manage incoming block layer device driver
+ * requests. We have three main type of block driver types which end up making
+ * use of the request_queue:
+ *
+ *   o make_request block drivers (multiqueue)
+ *   o request-based block drivers
+ *   o custom solutions such as scsi-generic
+ *
+ * All partitions share the same request_queue data structure.
+ */
 struct request_queue {
 	struct request		*last_merge;
 	struct elevator_queue	*elevator;
-- 
2.26.2

Re: [PATCH 1/2] block: add initial kdoc over the request_queue

[Damien Le Moal]

On 2020/06/24 7:03, Luis Chamberlain wrote:
> We start off with an initial description of the request_queue data
> structure, followed by describing the purpose of the debugfs_mutex
> debugfs_dir, and blk_trace.
> 
> Suggested-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  include/linux/blkdev.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 70461b347169..ea319c2b0593 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -394,6 +394,26 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
>  
>  #endif /* CONFIG_BLK_DEV_ZONED */
>  
> +/**
> + * struct request_queue - block device driver request queue
> + * @debugfs_mutex: used to protect access to the @debugfs_dir
> + * @blk_trace: used by blktrace to keep track of setup / tracing
> + * @debugfs_dir: directory created to place debugfs information. This is always
> + *	created for make_request and request-based block drivers upon
> + *	initialization. blktrace requires for this directory to be created,

s/requires for/requires/ (may be, not sure of my English grammar on this one :))

> + *	and so it will be created on demand if its block driver type does not
> + *	create it opon initialization.

s/opon/upon/

> + *
> + * The request_queue is used to manage incoming block layer device driver
> + * requests. We have three main type of block driver types which end up making

We have three main types of block drivers which...

> + * use of the request_queue:
> + *
> + *   o make_request block drivers (multiqueue)
> + *   o request-based block drivers

Isn't this second one BIO based drivers, like device-mapper BIO targets ?

> + *   o custom solutions such as scsi-generic

The SG driver does not create a request queue, it uses the one of the scsi
device created by the scsi stack. Is this what you mean here ? Saying
scsi-generic kind of implies sg driver.

> + *
> + * All partitions share the same request_queue data structure.
> + */
>  struct request_queue {
>  	struct request		*last_merge;
>  	struct elevator_queue	*elevator;
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] block: add initial kdoc over the request_queue

[Johannes Thumshirn]

On 24/06/2020 00:04, Luis Chamberlain wrote:
> + *   o make_request block drivers (multiqueue)

I don't think the differentiation between singlequeue and 
multiqueue is usefull, given that the single queue block layer
is gone for quite some time now.

Re: [PATCH 1/2] block: add initial kdoc over the request_queue

[Bart Van Assche]

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

Since request queues are used as a communication channel between the
block layer core and block drivers and since the block layer core
creates request queues, I propose to leave out the word "driver".

> + * @debugfs_mutex: used to protect access to the @debugfs_dir
> + * @blk_trace: used by blktrace to keep track of setup / tracing
> + * @debugfs_dir: directory created to place debugfs information. This is always
> + *	created for make_request and request-based block drivers upon
> + *	initialization. blktrace requires for this directory to be created,
> + *	and so it will be created on demand if its block driver type does not
> + *	create it opon initialization.
> + *
> + * The request_queue is used to manage incoming block layer device driver
> + * requests. We have three main type of block driver types which end up making
> + * use of the request_queue:

"incoming" is correct from the perspective of the block driver but not
from the perspective of the block layer core. How about describing
request queues as a communication channel between the block layer core
and block drivers?

Thanks,

Bart.

[PATCH 2/2] block: move request_queue member docs to kdoc

[Luis Chamberlain]

Now that we have a template, expand on the kdoc form for
the request_queue data structure with documentation from
the rest of the members, *only* for information we already
had.

This does not add any new documentation. This just shifts
documentation to kdoc form.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 include/linux/blkdev.h | 139 ++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 99 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ea319c2b0593..d30bfef893b9 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -396,8 +396,38 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
 
 /**
  * struct request_queue - block device driver request queue
+ * @queue_ctx: software queue context
+ * @queue_hw_ctx: hw dispatch queues
+ * @queuedata: the queue owner gets to use this for whatever they like.
+ *	ll_rw_blk doesn't touch it.
+ * @queue_flags: various queue flags, see %QUEUE_* below
+ * @pm_only: Number of contexts that have called blk_set_pm_only(). If this
+ *	counter is above zero then only %RQF_PM and %RQF_PREEMPT requests are
+ *	processed.
+ * @id: ida allocated id for this queue.  Used to index queues from ioctx.
+ * @bounce_gfp: queue needs bounce pages for pages above this limit
+ * @kobj: queue kobject
+ * @mq_kobj: mq queue kobject
+ * @nr_requests: maximum number of of requests
+ * @ksm: Inline crypto capabilities
+ * @nr_zones:
+ * @nr_zones: total number of zones of the device. This is always 0 for regular
+ *	block devices.
+ * @conv_zones_bitmap: bitmap of nr_zones bits which indicates if a zone
+ *	is conventional (bit set) or sequential (bit clear).
+ * @seq_zones_wlock: bitmap of nr_zones bits which indicates if a zone
+ *	is write locked, that is, if a write request targeting the zone was
+ *	dispatched.
+ * @debugfs_mutex: used to protect access to the @ebugfs_dir
  * @debugfs_mutex: used to protect access to the @debugfs_dir
  * @blk_trace: used by blktrace to keep track of setup / tracing
+ * @fq: for flush operations
+ * @td: throttle data
+ * @unused_hctx_list: list used for reusing dead hctx instance in case of
+ *	updating nr_hw_queues.
+ * @unused_hctx_lock: used to protect the @unused_hctx_list
+ * @mq_freeze_lock: protects concurrent access to q_usage_counter by
+ *	percpu_ref_kill() and percpu_ref_reinit().
  * @debugfs_dir: directory created to place debugfs information. This is always
  *	created for make_request and request-based block drivers upon
  *	initialization. blktrace requires for this directory to be created,
@@ -413,67 +443,35 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
  *   o custom solutions such as scsi-generic
  *
  * All partitions share the same request_queue data structure.
+ *
+ * Zoned block device dispatch control is managed by the fields @nr_zones,
+ * @conv_zones_bitmap and @seq_zones_wlock. These fields are fields are
+ * initialized by the low level device driver (e.g. scsi/sd.c).  Stacking
+ * drivers (device mappers) may or may not initialize these fields.
+ * Reads of this information must be protected with blk_queue_enter() /
+ * blk_queue_exit(). Modifying this information is only allowed while
+ * no requests are being processed. See also blk_mq_freeze_queue() and
+ * blk_mq_unfreeze_queue().
  */
 struct request_queue {
 	struct request		*last_merge;
 	struct elevator_queue	*elevator;
-
 	struct blk_queue_stats	*stats;
 	struct rq_qos		*rq_qos;
-
 	make_request_fn		*make_request_fn;
-
 	const struct blk_mq_ops	*mq_ops;
-
-	/* sw queues */
 	struct blk_mq_ctx __percpu	*queue_ctx;
-
 	unsigned int		queue_depth;
-
-	/* hw dispatch queues */
 	struct blk_mq_hw_ctx	**queue_hw_ctx;
 	unsigned int		nr_hw_queues;
-
 	struct backing_dev_info	*backing_dev_info;
-
-	/*
-	 * The queue owner gets to use this for whatever they like.
-	 * ll_rw_blk doesn't touch it.
-	 */
 	void			*queuedata;
-
-	/*
-	 * various queue flags, see QUEUE_* below
-	 */
 	unsigned long		queue_flags;
-	/*
-	 * Number of contexts that have called blk_set_pm_only(). If this
-	 * counter is above zero then only RQF_PM and RQF_PREEMPT requests are
-	 * processed.
-	 */
 	atomic_t		pm_only;
-
-	/*
-	 * ida allocated id for this queue.  Used to index queues from
-	 * ioctx.
-	 */
 	int			id;
-
-	/*
-	 * queue needs bounce pages for pages above this limit
-	 */
 	gfp_t			bounce_gfp;
-
 	spinlock_t		queue_lock;
-
-	/*
-	 * queue kobject
-	 */
 	struct kobject kobj;
-
-	/*
-	 * mq queue kobject
-	 */
 	struct kobject *mq_kobj;
 
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
@@ -485,66 +483,32 @@ struct request_queue {
 	int			rpm_status;
 	unsigned int		nr_pending;
 #endif
-
-	/*
-	 * queue settings
-	 */
-	unsigned long		nr_requests;	/* Max # of requests */
-
+	unsigned long		nr_requests;
 	unsigned int		dma_pad_mask;
 	unsigned int		dma_alignment;
-
 #ifdef CONFIG_BLK_INLINE_ENCRYPTION
-	/* Inline crypto capabilities */
 	struct blk_keyslot_manager *ksm;
 #endif
-
 	unsigned int		rq_timeout;
 	int			poll_nsec;
-
 	struct blk_stat_callback	*poll_cb;
 	struct blk_rq_stat	poll_stat[BLK_MQ_POLL_STATS_BKTS];
-
 	struct timer_list	timeout;
 	struct work_struct	timeout_work;
-
 	struct list_head	icq_list;
 #ifdef CONFIG_BLK_CGROUP
 	DECLARE_BITMAP		(blkcg_pols, BLKCG_MAX_POLS);
 	struct blkcg_gq		*root_blkg;
 	struct list_head	blkg_list;
 #endif
-
 	struct queue_limits	limits;
-
 	unsigned int		required_elevator_features;
-
 #ifdef CONFIG_BLK_DEV_ZONED
-	/*
-	 * Zoned block device information for request dispatch control.
-	 * nr_zones is the total number of zones of the device. This is always
-	 * 0 for regular block devices. conv_zones_bitmap is a bitmap of nr_zones
-	 * bits which indicates if a zone is conventional (bit set) or
-	 * sequential (bit clear). seq_zones_wlock is a bitmap of nr_zones
-	 * bits which indicates if a zone is write locked, that is, if a write
-	 * request targeting the zone was dispatched. All three fields are
-	 * initialized by the low level device driver (e.g. scsi/sd.c).
-	 * Stacking drivers (device mappers) may or may not initialize
-	 * these fields.
-	 *
-	 * Reads of this information must be protected with blk_queue_enter() /
-	 * blk_queue_exit(). Modifying this information is only allowed while
-	 * no requests are being processed. See also blk_mq_freeze_queue() and
-	 * blk_mq_unfreeze_queue().
-	 */
 	unsigned int		nr_zones;
 	unsigned long		*conv_zones_bitmap;
 	unsigned long		*seq_zones_wlock;
 #endif /* CONFIG_BLK_DEV_ZONED */
 
-	/*
-	 * sg stuff
-	 */
 	unsigned int		sg_timeout;
 	unsigned int		sg_reserved_size;
 	int			node;
@@ -552,59 +516,36 @@ struct request_queue {
 #ifdef CONFIG_BLK_DEV_IO_TRACE
 	struct blk_trace __rcu	*blk_trace;
 #endif
-	/*
-	 * for flush operations
-	 */
 	struct blk_flush_queue	*fq;
-
 	struct list_head	requeue_list;
 	spinlock_t		requeue_lock;
 	struct delayed_work	requeue_work;
-
 	struct mutex		sysfs_lock;
 	struct mutex		sysfs_dir_lock;
-
-	/*
-	 * for reusing dead hctx instance in case of updating
-	 * nr_hw_queues
-	 */
 	struct list_head	unused_hctx_list;
 	spinlock_t		unused_hctx_lock;
-
 	int			mq_freeze_depth;
-
 #if defined(CONFIG_BLK_DEV_BSG)
 	struct bsg_class_device bsg_dev;
 #endif
 
 #ifdef CONFIG_BLK_DEV_THROTTLING
-	/* Throttle data */
 	struct throtl_data *td;
 #endif
 	struct rcu_head		rcu_head;
 	wait_queue_head_t	mq_freeze_wq;
-	/*
-	 * Protect concurrent access to q_usage_counter by
-	 * percpu_ref_kill() and percpu_ref_reinit().
-	 */
 	struct mutex		mq_freeze_lock;
 	struct percpu_ref	q_usage_counter;
-
 	struct blk_mq_tag_set	*tag_set;
 	struct list_head	tag_set_list;
 	struct bio_set		bio_split;
-
 	struct dentry		*debugfs_dir;
-
 #ifdef CONFIG_BLK_DEBUG_FS
 	struct dentry		*sched_debugfs_dir;
 	struct dentry		*rqos_debugfs_dir;
 #endif
-
 	bool			mq_sysfs_init_done;
-
 	size_t			cmd_size;
-
 #define BLK_MAX_WRITE_HINTS	5
 	u64			write_hints[BLK_MAX_WRITE_HINTS];
 };
-- 
2.26.2

Re: [PATCH 2/2] block: move request_queue member docs to kdoc

[Damien Le Moal]

On 2020/06/24 7:03, Luis Chamberlain wrote:
> Now that we have a template, expand on the kdoc form for
> the request_queue data structure with documentation from
> the rest of the members, *only* for information we already
> had.
> 
> This does not add any new documentation. This just shifts
> documentation to kdoc form.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  include/linux/blkdev.h | 139 ++++++++++++-----------------------------
>  1 file changed, 40 insertions(+), 99 deletions(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ea319c2b0593..d30bfef893b9 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -396,8 +396,38 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
>  
>  /**
>   * struct request_queue - block device driver request queue
> + * @queue_ctx: software queue context
> + * @queue_hw_ctx: hw dispatch queues
> + * @queuedata: the queue owner gets to use this for whatever they like.
> + *	ll_rw_blk doesn't touch it.
> + * @queue_flags: various queue flags, see %QUEUE_* below
> + * @pm_only: Number of contexts that have called blk_set_pm_only(). If this
> + *	counter is above zero then only %RQF_PM and %RQF_PREEMPT requests are
> + *	processed.
> + * @id: ida allocated id for this queue.  Used to index queues from ioctx.
> + * @bounce_gfp: queue needs bounce pages for pages above this limit
> + * @kobj: queue kobject
> + * @mq_kobj: mq queue kobject
> + * @nr_requests: maximum number of of requests
> + * @ksm: Inline crypto capabilities
> + * @nr_zones:

Above line is not needed, no ?

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

May be: "total number of zones for a zoned block device. This is..."

> + * @conv_zones_bitmap: bitmap of nr_zones bits which indicates if a zone
> + *	is conventional (bit set) or sequential (bit clear).
> + * @seq_zones_wlock: bitmap of nr_zones bits which indicates if a zone
> + *	is write locked, that is, if a write request targeting the zone was
> + *	dispatched.
> + * @debugfs_mutex: used to protect access to the @ebugfs_dir
>   * @debugfs_mutex: used to protect access to the @debugfs_dir
>   * @blk_trace: used by blktrace to keep track of setup / tracing
> + * @fq: for flush operations
> + * @td: throttle data
> + * @unused_hctx_list: list used for reusing dead hctx instance in case of
> + *	updating nr_hw_queues.
> + * @unused_hctx_lock: used to protect the @unused_hctx_list
> + * @mq_freeze_lock: protects concurrent access to q_usage_counter by
> + *	percpu_ref_kill() and percpu_ref_reinit().
>   * @debugfs_dir: directory created to place debugfs information. This is always
>   *	created for make_request and request-based block drivers upon
>   *	initialization. blktrace requires for this directory to be created,
> @@ -413,67 +443,35 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
>   *   o custom solutions such as scsi-generic
>   *
>   * All partitions share the same request_queue data structure.
> + *
> + * Zoned block device dispatch control is managed by the fields @nr_zones,
> + * @conv_zones_bitmap and @seq_zones_wlock. These fields are fields are> + * initialized by the low level device driver (e.g. scsi/sd.c).  Stacking
> + * drivers (device mappers) may or may not initialize these fields.
> + * Reads of this information must be protected with blk_queue_enter() /
> + * blk_queue_exit(). Modifying this information is only allowed while
> + * no requests are being processed. See also blk_mq_freeze_queue() and
> + * blk_mq_unfreeze_queue().

Ooops... This is outdated... This should be:

* Dispatch of write commands to zoned block devices is controlled using the
* fields @nr_zones, @conv_zones_bitmap and @seq_zones_wlock. These fields are
* initialized using the blk_revalidate_disk_zones() function from the level
* request based device drivers (e.g. scsi/sd.c).  BIO based drivers, if needed,
* can initialize these fields directly. Accesses to these fields must be
* protected with blk_queue_enter() / blk_queue_exit(). Modification of the
* @nr_zones field and reallocation of the @conv_zones_bitmap and
* @seq_zones_wlock bitmaps is only allowed while no requests are being
* processed. See also blk_mq_freeze_queue() and blk_mq_unfreeze_queue().

>   */
>  struct request_queue {
>  	struct request		*last_merge;
>  	struct elevator_queue	*elevator;
> -
>  	struct blk_queue_stats	*stats;
>  	struct rq_qos		*rq_qos;
> -
>  	make_request_fn		*make_request_fn;
> -
>  	const struct blk_mq_ops	*mq_ops;
> -
> -	/* sw queues */
>  	struct blk_mq_ctx __percpu	*queue_ctx;
> -
>  	unsigned int		queue_depth;
> -
> -	/* hw dispatch queues */
>  	struct blk_mq_hw_ctx	**queue_hw_ctx;
>  	unsigned int		nr_hw_queues;
> -
>  	struct backing_dev_info	*backing_dev_info;
> -
> -	/*
> -	 * The queue owner gets to use this for whatever they like.
> -	 * ll_rw_blk doesn't touch it.
> -	 */
>  	void			*queuedata;
> -
> -	/*
> -	 * various queue flags, see QUEUE_* below
> -	 */
>  	unsigned long		queue_flags;
> -	/*
> -	 * Number of contexts that have called blk_set_pm_only(). If this
> -	 * counter is above zero then only RQF_PM and RQF_PREEMPT requests are
> -	 * processed.
> -	 */
>  	atomic_t		pm_only;
> -
> -	/*
> -	 * ida allocated id for this queue.  Used to index queues from
> -	 * ioctx.
> -	 */
>  	int			id;
> -
> -	/*
> -	 * queue needs bounce pages for pages above this limit
> -	 */
>  	gfp_t			bounce_gfp;
> -
>  	spinlock_t		queue_lock;
> -
> -	/*
> -	 * queue kobject
> -	 */
>  	struct kobject kobj;
> -
> -	/*
> -	 * mq queue kobject
> -	 */
>  	struct kobject *mq_kobj;
>  
>  #ifdef  CONFIG_BLK_DEV_INTEGRITY
> @@ -485,66 +483,32 @@ struct request_queue {
>  	int			rpm_status;
>  	unsigned int		nr_pending;
>  #endif
> -
> -	/*
> -	 * queue settings
> -	 */
> -	unsigned long		nr_requests;	/* Max # of requests */
> -
> +	unsigned long		nr_requests;
>  	unsigned int		dma_pad_mask;
>  	unsigned int		dma_alignment;
> -
>  #ifdef CONFIG_BLK_INLINE_ENCRYPTION
> -	/* Inline crypto capabilities */
>  	struct blk_keyslot_manager *ksm;
>  #endif
> -
>  	unsigned int		rq_timeout;
>  	int			poll_nsec;
> -
>  	struct blk_stat_callback	*poll_cb;
>  	struct blk_rq_stat	poll_stat[BLK_MQ_POLL_STATS_BKTS];
> -
>  	struct timer_list	timeout;
>  	struct work_struct	timeout_work;
> -
>  	struct list_head	icq_list;
>  #ifdef CONFIG_BLK_CGROUP
>  	DECLARE_BITMAP		(blkcg_pols, BLKCG_MAX_POLS);
>  	struct blkcg_gq		*root_blkg;
>  	struct list_head	blkg_list;
>  #endif
> -
>  	struct queue_limits	limits;
> -
>  	unsigned int		required_elevator_features;
> -
>  #ifdef CONFIG_BLK_DEV_ZONED
> -	/*
> -	 * Zoned block device information for request dispatch control.
> -	 * nr_zones is the total number of zones of the device. This is always
> -	 * 0 for regular block devices. conv_zones_bitmap is a bitmap of nr_zones
> -	 * bits which indicates if a zone is conventional (bit set) or
> -	 * sequential (bit clear). seq_zones_wlock is a bitmap of nr_zones
> -	 * bits which indicates if a zone is write locked, that is, if a write
> -	 * request targeting the zone was dispatched. All three fields are
> -	 * initialized by the low level device driver (e.g. scsi/sd.c).
> -	 * Stacking drivers (device mappers) may or may not initialize
> -	 * these fields.
> -	 *
> -	 * Reads of this information must be protected with blk_queue_enter() /
> -	 * blk_queue_exit(). Modifying this information is only allowed while
> -	 * no requests are being processed. See also blk_mq_freeze_queue() and
> -	 * blk_mq_unfreeze_queue().
> -	 */
>  	unsigned int		nr_zones;
>  	unsigned long		*conv_zones_bitmap;
>  	unsigned long		*seq_zones_wlock;
>  #endif /* CONFIG_BLK_DEV_ZONED */
>  
> -	/*
> -	 * sg stuff
> -	 */
>  	unsigned int		sg_timeout;
>  	unsigned int		sg_reserved_size;
>  	int			node;
> @@ -552,59 +516,36 @@ struct request_queue {
>  #ifdef CONFIG_BLK_DEV_IO_TRACE
>  	struct blk_trace __rcu	*blk_trace;
>  #endif
> -	/*
> -	 * for flush operations
> -	 */
>  	struct blk_flush_queue	*fq;
> -
>  	struct list_head	requeue_list;
>  	spinlock_t		requeue_lock;
>  	struct delayed_work	requeue_work;
> -
>  	struct mutex		sysfs_lock;
>  	struct mutex		sysfs_dir_lock;
> -
> -	/*
> -	 * for reusing dead hctx instance in case of updating
> -	 * nr_hw_queues
> -	 */
>  	struct list_head	unused_hctx_list;
>  	spinlock_t		unused_hctx_lock;
> -
>  	int			mq_freeze_depth;
> -
>  #if defined(CONFIG_BLK_DEV_BSG)
>  	struct bsg_class_device bsg_dev;
>  #endif
>  
>  #ifdef CONFIG_BLK_DEV_THROTTLING
> -	/* Throttle data */
>  	struct throtl_data *td;
>  #endif
>  	struct rcu_head		rcu_head;
>  	wait_queue_head_t	mq_freeze_wq;
> -	/*
> -	 * Protect concurrent access to q_usage_counter by
> -	 * percpu_ref_kill() and percpu_ref_reinit().
> -	 */
>  	struct mutex		mq_freeze_lock;
>  	struct percpu_ref	q_usage_counter;
> -
>  	struct blk_mq_tag_set	*tag_set;
>  	struct list_head	tag_set_list;
>  	struct bio_set		bio_split;
> -
>  	struct dentry		*debugfs_dir;
> -
>  #ifdef CONFIG_BLK_DEBUG_FS
>  	struct dentry		*sched_debugfs_dir;
>  	struct dentry		*rqos_debugfs_dir;
>  #endif
> -
>  	bool			mq_sysfs_init_done;
> -
>  	size_t			cmd_size;
> -
>  #define BLK_MAX_WRITE_HINTS	5
>  	u64			write_hints[BLK_MAX_WRITE_HINTS];
>  };
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] block: move request_queue member docs to kdoc

[Johannes Thumshirn]

On 24/06/2020 00:03, Luis Chamberlain wrote:
> + * @debugfs_mutex: used to protect access to the @ebugfs_dir
>   * @debugfs_mutex: used to protect access to the @debugfs_dir

This line is duplicated and one of dups has a typo 'ebugfs_dir'

Re: [PATCH 2/2] block: move request_queue member docs to kdoc

[Bart Van Assche]

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.