On 9/30/19 1:48 PM, André Almeida wrote:
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 0bf056de5cc3..d0aab34972b7 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -11,12 +11,85 @@ struct blk_flush_queue;
>
> /**
> * struct blk_mq_hw_ctx - State for a hardware queue facing the hardware
> block device
> + *
> + * @lock: Lock for accessing dispatch queue
> + * @dispatch: Queue of dispatched requests, waiting for workers to
> send them
> + * to the hardware.
> + * @state: BLK_MQ_S_* flags. Define the state of the hw queue (active,
> + * scheduled to restart, stopped).
> + *
> + * @run_work: Worker for dispatch scheduled requests to
> hardware.
> + * BLK_MQ_CPU_WORK_BATCH workers will be assigned to a CPU,
> + * then the following ones will be assigned to
> + * the next_cpu.
> + * @cpumask: Map of available CPUs.
> + * @next_cpu: Index of CPU/workqueue to be used in the next
> batch
> + * of workers.
> + * @next_cpu_batch: Counter of how many works letf in the batch before
> + * changing to the next CPU. A batch has the size of
> + * BLK_MQ_CPU_WORK_BATCH.
> + *
> + * @flags: BLK_MQ_F_* flags. Define the behaviour of the queue.
> + *
> + * @sched_data: Data to support schedulers.
> + * @queue: Queue of request to be dispatched.
> + * @fq: Queue of requests to be flushed from memory to storage.
> + *
> + * @driver_data: Device driver specific queue.
> + *
> + * @ctx_map: Bitmap for each software queue. If bit is on, there is a
> + * pending request.
> + *
> + * @dispatch_from: Queue to be used when there is no scheduler was selected.
> + * @dispatch_busy: Number used by blk_mq_update_dispatch_busy() to decide
> + * if the hw_queue is busy using Exponential Weighted Moving
> + * Average algorithm.
> + *
> + * @type: HCTX_TYPE_* flags. Type of hardware queue.
> + * @nr_ctx: Number of software queues.
> + * @ctxs: Array of software queues.
> + *
> + * @dispatch_wait_lock: Lock for dispatch_wait queue.
> + * @dispatch_wait: Waitqueue for dispatched requests. Request here will
> + * be processed when
> + * percpu_ref_is_zero(&q->q_usage_counter) evaluates true
> + * for q as a request_queue.
> + * @wait_index: Index of next wait queue to be used.
> + *
> + * @tags: Request tags.
> + * @sched_tags: Request tags for schedulers.
> + *
> + * @queued: Number of queued requests.
> + * @run: Number of dispatched requests.
> + * @dispatched: Number of dispatch requests by queue.
> + *
> + * @numa_node: NUMA node index of this hardware queue.
> + * @queue_num: Index of this hardware queue.
> + *
> + * @nr_active: Number of active tags.
> + *
> + * @cpuhp_dead: List to store request if some CPU die.
> + * @kobj: Kernel object for sysfs.
> + *
> + * @poll_considered: Count times blk_poll() was called.
> + * @poll_invoked: Count how many requests blk_poll() polled.
> + * @poll_success: Count how many polled requests were completed.
> + *
> + * @debugfs_dir: debugfs directory for this hardware queue. Named
> + * as cpu<cpu_number>.
> + * @sched_debugfs_dir: debugfs directory for the scheduler.
> + *
> + * @hctx_list: List of all hardware queues.
> + *
> + * @srcu: Sleepable RCU. Use as lock when type of the hardware queue is
> + * blocking (BLK_MQ_F_BLOCKING). Must be the last member - see
> + * also blk_mq_hw_ctx_size().
> */
> struct blk_mq_hw_ctx {
> struct {
> spinlock_t lock;
> struct list_head dispatch;
> - unsigned long state; /* BLK_MQ_S_* flags */
> + unsigned long state;
> } ____cacheline_aligned_in_smp;
>
> struct delayed_work run_work;
> @@ -24,7 +97,7 @@ struct blk_mq_hw_ctx {
> int next_cpu;
> int next_cpu_batch;
>
> - unsigned long flags; /* BLK_MQ_F_* flags */
> + unsigned long flags;
>
> void *sched_data;
> struct request_queue *queue;
> @@ -72,41 +145,73 @@ struct blk_mq_hw_ctx {
>
> struct list_head hctx_list;
>
> - /* Must be the last member - see also blk_mq_hw_ctx_size(). */
> struct srcu_struct srcu[0];
> };
I like improving how much is documented, but I absolutely don't like how
everything is pulled into one section above the struct instead of being
documented inline instead.
I realize this probably makes it easier to make nice external
documentation, but imho that's absolutely secondary to having the
documentation being right there where you read the code.
> @@ -142,81 +253,100 @@ typedef bool (busy_fn)(struct request_queue *);
> typedef void (complete_fn)(struct request *);
> typedef void (cleanup_rq_fn)(struct request *);
>
> -
> +/**
> + * struct blk_mq_ops - list of callback functions for blk-mq drivers
> + */
> struct blk_mq_ops {
> - /*
> - * Queue request
> + /**
> + * @queue_rq: Queue a new request from block IO.
> */
> queue_rq_fn *queue_rq;
>
> - /*
> - * If a driver uses bd->last to judge when to submit requests to
> - * hardware, it must define this function. In case of errors that
> - * make us stop issuing further requests, this hook serves the
> + /**
> + * @commit_rqs: If a driver uses bd->last to judge when to submit
> + * requests to hardware, it must define this function. In case of errors
> + * that make us stop issuing further requests, this hook serves the
> * purpose of kicking the hardware (which the last request otherwise
> * would have done).
> */
> commit_rqs_fn *commit_rqs;
Stuff like this is MUCH better. Why isn't all of it done like this?
--
Jens Axboe
