> Il giorno 21 dic 2022, alle ore 01:50, Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx> ha scritto:
>
> On 2022/12/20 22:10, Paolo Valente wrote:
>>>> - /*
>>>> - * Does queue (or any parent entity) exceed number of requests that
>>>> - * should be available to it? Heavily limit depth so that it cannot
>>>> - * consume more available requests and thus starve other entities.
>>>> - */
>>>> - if (bfqq && bfqq_request_over_limit(bfqq, limit))
>>>> - depth = 1;
>>>> + for (act_idx = 0; act_idx < bfqd->num_actuators; act_idx++) {
>>>> + struct bfq_queue *bfqq =
>>>> + bic ? bic_to_bfqq(bic, op_is_sync(opf), act_idx) : NULL;
>>>
>>> Commented already: why not add a "if (!bfqq) return NULL;" in
>>> bic_to_bfqq() ?
>>
>> You have probably missed my reply on this. The problem is that your
>> proposal would improve code (only) here, but it would entail the above
>> control for all the other invocations, for which it is useless :(
>
> But then you have *a lot* of "if (bfqd)" tests that are useless elsewhere since
> bic_to_bfqq() never returns NULL.
>
I'm probably misunderstanding your point, sorry. Could you point me
to one of the places where there is the useless control that would go
away if we add your proposed control inside bic_to_bfqq? (of course
apart form the above one, which seems to be the only one to me)
> And for this line, I personally would prefer seeing something like:
>
> struct bfq_queue *bfqq;
>
>
> if (bic)
> bfqd = bic_to_bfqq(bic, op_is_sync(opf), act_idx)
> else
> bfqd = NULL;
>
> Which is a lot simpler to read.
>
>
Ok, as I have your blessing on this point, I'll send a V12 with also
this change.
Thanks,
Paolo
> --
> Damien Le Moal
> Western Digital Research
>
