Messages in this thread Patch in this message | | | Date | Sun, 27 Mar 2022 19:02:46 -0700 | From | Bart Van Assche <> | Subject | Re: [PATCH] block: use dedicated list iterator variable |
| |
On 3/27/22 14:47, Jakob Koschel wrote: > To move the list iterator variable into the list_for_each_entry_*() > macro in the future it should be avoided to use the list iterator > variable after the loop body. > > To *never* use the list iterator variable after the loop it was > concluded to use a separate iterator variable instead of a > found boolean [1]. > > Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ > Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com> > --- > block/blk-mq.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 8e659dc5fcf3..455fdd488f3c 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -4466,12 +4466,14 @@ static bool blk_mq_elv_switch_none(struct list_head *head, > static void blk_mq_elv_switch_back(struct list_head *head, > struct request_queue *q) > { > - struct blk_mq_qe_pair *qe; > + struct blk_mq_qe_pair *qe = NULL; > + struct blk_mq_qe_pair *iter; > struct elevator_type *t = NULL; > > - list_for_each_entry(qe, head, node) > - if (qe->q == q) { > - t = qe->type; > + list_for_each_entry(iter, head, node) > + if (iter->q == q) { > + t = iter->type; > + qe = iter; > break; > }
The current code looks much more readable to me than with the above patch applied. How about replacing the above patch with the patch below?
Thanks,
Bart.
diff --git a/block/blk-mq.c b/block/blk-mq.c index 9c784262fd6b..c9435be54667 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -4451,21 +4451,28 @@ static bool blk_mq_elv_switch_none(struct list_head *head, return true; }
-static void blk_mq_elv_switch_back(struct list_head *head, - struct request_queue *q) +static struct blk_mq_qe_pair *blk_lookup_qe_pair(struct list_head *head, + struct request_queue *q) { struct blk_mq_qe_pair *qe; - struct elevator_type *t = NULL;
list_for_each_entry(qe, head, node) - if (qe->q == q) { - t = qe->type; - break; - } + if (qe->q == q) + return qe; + + return NULL; +} + +static void blk_mq_elv_switch_back(struct list_head *head, + struct request_queue *q) +{ + struct blk_mq_qe_pair *qe = blk_lookup_qe_pair(head, q); + struct elevator_type *t;
- if (!t) + if (!qe) return;
+ t = qe->type; list_del(&qe->node); kfree(qe);
| |