| Message ID | 20210527010134.32448-2-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve I/O priority support | expand |
On 2021/05/27 10:01, Bart Van Assche wrote: > Make the code easier to read by adding more comments. > > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
On 5/27/21 3:01 AM, Bart Van Assche wrote: > Make the code easier to read by adding more comments. > > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/mq-deadline.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/block/mq-deadline.c b/block/mq-deadline.c > index 8eea2cbf2bf4..64cabbc157ea 100644 > --- a/block/mq-deadline.c > +++ b/block/mq-deadline.c > @@ -139,6 +139,9 @@ static void dd_request_merged(struct request_queue *q, struct request *req, > } > } > > +/* > + * Callback function that is invoked after @next has been merged into @req. > + */ > static void dd_merged_requests(struct request_queue *q, struct request *req, > struct request *next) > { > @@ -375,6 +378,8 @@ static struct request *__dd_dispatch_request(struct deadline_data *dd) > } > > /* > + * Called from blk_mq_run_hw_queue() -> __blk_mq_sched_dispatch_requests(). > + * > * One confusing aspect here is that we get called for a specific > * hardware queue, but we may return a request that is for a > * different hardware queue. This is because mq-deadline has shared > @@ -438,6 +443,10 @@ static int dd_init_queue(struct request_queue *q, struct elevator_type *e) > return 0; > } > > +/* > + * Try to merge @bio into an existing request. If @bio has been merged into > + * an existing request, store the pointer to that request into *@rq. > + */ > static int dd_request_merge(struct request_queue *q, struct request **rq, > struct bio *bio) > { > @@ -461,6 +470,10 @@ static int dd_request_merge(struct request_queue *q, struct request **rq, > return ELEVATOR_NO_MERGE; > } > > +/* > + * Attempt to merge a bio into an existing request. This function is called > + * before @bio is associated with a request. > + */ > static bool dd_bio_merge(struct request_queue *q, struct bio *bio, > unsigned int nr_segs) > { > @@ -518,6 +531,9 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, > } > } > > +/* > + * Called from blk_mq_sched_insert_request() or blk_mq_sched_insert_requests(). > + */ > static void dd_insert_requests(struct blk_mq_hw_ctx *hctx, > struct list_head *list, bool at_head) > { > @@ -544,6 +560,8 @@ static void dd_prepare_request(struct request *rq) > } > > /* > + * Callback function called from inside blk_mq_free_request(). > + * > * For zoned block devices, write unlock the target zone of > * completed write requests. Do this while holding the zone lock > * spinlock so that the zone is never unlocked while deadline_fifo_request() > Shouldn't these be kernel-doc comments? Cheers, Hannes
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com?
On 5/26/21 8:01 PM, Bart Van Assche wrote: > Make the code easier to read by adding more comments. > > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/mq-deadline.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/block/mq-deadline.c b/block/mq-deadline.c > index 8eea2cbf2bf4..64cabbc157ea 100644 > --- a/block/mq-deadline.c > +++ b/block/mq-deadline.c > @@ -139,6 +139,9 @@ static void dd_request_merged(struct request_queue *q, struct request *req, > } > } > > +/* > + * Callback function that is invoked after @next has been merged into @req. > + */ > static void dd_merged_requests(struct request_queue *q, struct request *req, > struct request *next) > { > @@ -375,6 +378,8 @@ static struct request *__dd_dispatch_request(struct deadline_data *dd) > } > > /* > + * Called from blk_mq_run_hw_queue() -> __blk_mq_sched_dispatch_requests(). > + * > * One confusing aspect here is that we get called for a specific > * hardware queue, but we may return a request that is for a > * different hardware queue. This is because mq-deadline has shared > @@ -438,6 +443,10 @@ static int dd_init_queue(struct request_queue *q, struct elevator_type *e) > return 0; > } > > +/* > + * Try to merge @bio into an existing request. If @bio has been merged into > + * an existing request, store the pointer to that request into *@rq. > + */ > static int dd_request_merge(struct request_queue *q, struct request **rq, > struct bio *bio) > { > @@ -461,6 +470,10 @@ static int dd_request_merge(struct request_queue *q, struct request **rq, > return ELEVATOR_NO_MERGE; > } > > +/* > + * Attempt to merge a bio into an existing request. This function is called > + * before @bio is associated with a request. > + */ > static bool dd_bio_merge(struct request_queue *q, struct bio *bio, > unsigned int nr_segs) > { > @@ -518,6 +531,9 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, > } > } > > +/* > + * Called from blk_mq_sched_insert_request() or blk_mq_sched_insert_requests(). > + */ > static void dd_insert_requests(struct blk_mq_hw_ctx *hctx, > struct list_head *list, bool at_head) > { > @@ -544,6 +560,8 @@ static void dd_prepare_request(struct request *rq) > } > > /* > + * Callback function called from inside blk_mq_free_request(). > + * > * For zoned block devices, write unlock the target zone of > * completed write requests. Do this while holding the zone lock > * spinlock so that the zone is never unlocked while deadline_fifo_request() > Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
On 5/26/21 11:45 PM, Hannes Reinecke wrote: > On 5/27/21 3:01 AM, Bart Van Assche wrote: >> /* >> + * Callback function called from inside blk_mq_free_request(). >> + * >> * For zoned block devices, write unlock the target zone of >> * completed write requests. Do this while holding the zone lock >> * spinlock so that the zone is never unlocked while deadline_fifo_request() >> > Shouldn't these be kernel-doc comments? Hi Hannes, When using the kernel-doc format it is required to document the meaning of all function arguments. It seems to me that it is easy to guess what the meaning of the function arguments is in the deadline scheduler? Bart.
diff --git a/block/mq-deadline.c b/block/mq-deadline.c index 8eea2cbf2bf4..64cabbc157ea 100644 --- a/block/mq-deadline.c +++ b/block/mq-deadline.c @@ -139,6 +139,9 @@ static void dd_request_merged(struct request_queue *q, struct request *req, } } +/* + * Callback function that is invoked after @next has been merged into @req. + */ static void dd_merged_requests(struct request_queue *q, struct request *req, struct request *next) { @@ -375,6 +378,8 @@ static struct request *__dd_dispatch_request(struct deadline_data *dd) } /* + * Called from blk_mq_run_hw_queue() -> __blk_mq_sched_dispatch_requests(). + * * One confusing aspect here is that we get called for a specific * hardware queue, but we may return a request that is for a * different hardware queue. This is because mq-deadline has shared @@ -438,6 +443,10 @@ static int dd_init_queue(struct request_queue *q, struct elevator_type *e) return 0; } +/* + * Try to merge @bio into an existing request. If @bio has been merged into + * an existing request, store the pointer to that request into *@rq. + */ static int dd_request_merge(struct request_queue *q, struct request **rq, struct bio *bio) { @@ -461,6 +470,10 @@ static int dd_request_merge(struct request_queue *q, struct request **rq, return ELEVATOR_NO_MERGE; } +/* + * Attempt to merge a bio into an existing request. This function is called + * before @bio is associated with a request. + */ static bool dd_bio_merge(struct request_queue *q, struct bio *bio, unsigned int nr_segs) { @@ -518,6 +531,9 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, } } +/* + * Called from blk_mq_sched_insert_request() or blk_mq_sched_insert_requests(). + */ static void dd_insert_requests(struct blk_mq_hw_ctx *hctx, struct list_head *list, bool at_head) { @@ -544,6 +560,8 @@ static void dd_prepare_request(struct request *rq) } /* + * Callback function called from inside blk_mq_free_request(). + * * For zoned block devices, write unlock the target zone of * completed write requests. Do this while holding the zone lock * spinlock so that the zone is never unlocked while deadline_fifo_request()
Make the code easier to read by adding more comments. Cc: Damien Le Moal <damien.lemoal@wdc.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Ming Lei <ming.lei@redhat.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/mq-deadline.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)