> Il giorno 21 nov 2022, alle ore 01:18, Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx> ha scritto:
...
> That said, creating 2 helpers
> bic_to_async_bfqq() and bic_to_sync_bfqq() without that second argument
> would make the code more readable and less error prone I think.
>
I'm not sure yet about this, because, in some other place, the function is invoked like this
bic_to_bfqq(bic, is_sync, act_idx);
where is_sync is a bool. With your proposal, we would lose the
wrapper and have to turn the above line into an if/else.
So, if ok for you, I'd leave this further change for a second time, as
it is somehow unrelated with the main goal of this patch.
...
>>
>> if (bfqq && bic->stable_merge_bfqq == bfqq) {
>> /*
>> @@ -672,9 +677,9 @@ static void bfq_limit_depth(blk_opf_t opf, struct blk_mq_alloc_data *data)
>> {
>> struct bfq_data *bfqd = data->q->elevator->elevator_data;
>> struct bfq_io_cq *bic = bfq_bic_lookup(data->q);
>> - struct bfq_queue *bfqq = bic ? bic_to_bfqq(bic, op_is_sync(opf)) : NULL;
>
> Given that you are repeating this same pattern many times, the test for
> bic != NULL should go into the bic_to_bfqq() helper.
Actually, this would improve code here, but it would entail a useless control
for all the other invocations :(
As for your other replies, I have applied all the other suggestions in
this reply of yours.
Thanks,
Paolo
