[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <EEFE8587-7834-4686-B3C3-E2375668E680@linaro.org>
Date: Tue, 6 Dec 2022 09:04:45 +0100
From: Paolo Valente <paolo.valente@...aro.org>
To: Damien Le Moal <damien.lemoal@...nsource.wdc.com>
Cc: Jens Axboe <axboe@...nel.dk>,
linux-block <linux-block@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Arie van der Hoeven <arie.vanderhoeven@...gate.com>,
Rory Chen <rory.c.chen@...gate.com>,
Gabriele Felici <felicigb@...il.com>,
Carmine Zaccagnino <carmine@...minezacc.com>
Subject: Re: [PATCH V6 1/8] block, bfq: split sync bfq_queues on a
per-actuator basis
> Il giorno 21 nov 2022, alle ore 01:18, Damien Le Moal <damien.lemoal@...nsource.wdc.com> 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
Powered by blists - more mailing lists