Re: [PATCH V6 1/8] block, bfq: split sync bfq_queues on a per-actuator basis

[Paolo Valente]

> 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