lkml.org 
[lkml]   [2022]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH V10 1/8] block, bfq: split sync bfq_queues on a per-actuator basis
    From
    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.

    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.


    --
    Damien Le Moal
    Western Digital Research

    \
     
     \ /
      Last update: 2023-03-26 23:16    [W:2.501 / U:0.200 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site