Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
IronPort-SDR: s82+dmxXAheRzcKuD48PJi4BwAwOeW3ijagbny9juf53ntL63RVTF79+PVqdqyrFdi5hxH9aEo
 agOzqLizxqu2IfqRUtHpjAPrUo6q+JxRmfrorD0aiDqOpJnR9cW0zthYOS9eAtS7dWwatQkmqf
 hFaln7kRmuTIOrhVXlIvdDZuIQKmIzPQj4BL0s6WOOztSr21xTSgnlzJ7L+NQ4+0K+kbwqe1q/
 4vHCjDe7d5DCQzPvQNERXXbChUb4TMGK0DOVg9cd3xzsRUSCaoZOiZLo1WKFOuOl4Pk+hcHuwO
 MuY=
IronPort-SDR: NAc52oDWYeTmWD22lcJw0oXWon8G3OenVuOpa0uy+OPBHA1DVTcncTZzL6WpYdubOzn6zVAnMF
 gLfVcyiXLnPeS6X861EoH6pylxMSsuhfBTljQV1X9gJdt1B1ge+icjP41RhvvpN71OXCbJOrDU
 ytWTAPdjbRDv+AYaw+XV9TGUoQZgUfx8x3XxKts3PWs2QQmmm2OpDTtpm9MLo6f+kUWVqh8aXb
 52coQ7xiENRKhM79G4Zd7ccKvzC+LMSlHOaI4G/eOudmrxHsnTwGT6a2K567d4Bb5yMA2V/RFh
 /Sw=
WDCIronportException: Internal
Message-ID: <958736d1-918b-d1a0-d6f2-0841f4470803@opensource.wdc.com>
Date:   Fri, 16 Dec 2022 10:36:56 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.1
Subject: Re: [PATCH V10 8/8] block, bfq: balance I/O injection among
 underutilized actuators
Content-Language: en-US
To:     Paolo Valente <paolo.valente@linaro.org>,
        Jens Axboe <axboe@kernel.dk>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        arie.vanderhoeven@seagate.com, rory.c.chen@seagate.com,
        glen.valante@linaro.org, Davide Zini <davidezini2@gmail.com>
References: <20221209094442.36896-1-paolo.valente@linaro.org>
 <20221209094442.36896-9-paolo.valente@linaro.org>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital Research
In-Reply-To: <20221209094442.36896-9-paolo.valente@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/9/22 18:44, Paolo Valente wrote:
> From: Davide Zini <davidezini2@gmail.com>
> 
> Upon the invocation of its dispatch function, BFQ returns the next I/O
> request of the in-service bfq_queue, unless some exception holds. One
> such exception is that there is some underutilized actuator, different
> from the actuator for which the in-service queue contains I/O, and
> that some other bfq_queue happens to contain I/O for such an
> actuator. In this case, the next I/O request of the latter bfq_queue,
> and not of the in-service bfq_queue, is returned (I/O is injected from
> that bfq_queue). To find such an actuator, a linear scan, in
> increasing index order, is performed among actuators.
> 
> Performing a linear scan entails a prioritization among actuators: an
> underutilized actuator may be considered for injection only if all
> actuators with a lower index are currently fully utilized, or if there
> is no pending I/O for any lower-index actuator that happens to be
> underutilized.
> 
> This commits breaks this prioritization and tends to distribute
> injection uniformly across actuators. This is obtained by adding the
> following condition to the linear scan: even if an actuator A is
> underutilized, A is however skipped if its load is higher than that of
> the next actuator.
> 
> Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
> Signed-off-by: Davide Zini <davidezini2@gmail.com>
> ---
>  block/bfq-iosched.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 527def05ce44..0ec8310331e1 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -4830,10 +4830,16 @@ bfq_find_active_bfqq_for_actuator(struct bfq_data *bfqd, int idx)
>  
>  /*
>   * Perform a linear scan of each actuator, until an actuator is found
> - * for which the following two conditions hold: the load of the
> - * actuator is below the threshold (see comments on actuator_load_threshold
> - * for details), and there is a queue that contains I/O for that
> - * actuator. On success, return that queue.
> + * for which the following three conditions hold: the load of the
> + * actuator is below the threshold (see comments on
> + * actuator_load_threshold for details) and lower than that of the
> + * next actuator (comments on this extra condition below), and there
> + * is a queue that contains I/O for that actuator. On success, return
> + * that queue.
> + *
> + * Performing a plain linear scan entails a prioritization among
> + * actuators. The extra condition above breaks this prioritization and
> + * tends to distribute injection uniformly across actuators.
>   */
>  static struct bfq_queue *
>  bfq_find_bfqq_for_underused_actuator(struct bfq_data *bfqd)
> @@ -4841,7 +4847,9 @@ bfq_find_bfqq_for_underused_actuator(struct bfq_data *bfqd)
>  	int i;
>  
>  	for (i = 0 ; i < bfqd->num_actuators; i++) {
> -		if (bfqd->rq_in_driver[i] < bfqd->actuator_load_threshold) {
> +		if (bfqd->rq_in_driver[i] < bfqd->actuator_load_threshold &&
> +		    (i == bfqd->num_actuators - 1 ||
> +		     bfqd->rq_in_driver[i] < bfqd->rq_in_driver[i+1])) {
>  			struct bfq_queue *bfqq =
>  				bfq_find_active_bfqq_for_actuator(bfqd, i);
>  

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

-- 
Damien Le Moal
Western Digital Research