Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] blk-cgroup: Fix the recursive blkg rwstat
To:     Xunlei Pang <xlpang@linux.alibaba.com>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        tj@kernel.org
References: <1614932007-97224-1-git-send-email-xlpang@linux.alibaba.com>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <4cc104d1-9aa3-a838-b786-9a808dd85945@kernel.dk>
Date:   Fri, 5 Mar 2021 11:32:05 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <1614932007-97224-1-git-send-email-xlpang@linux.alibaba.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/5/21 1:13 AM, Xunlei Pang wrote:
> The current blkio.throttle.io_service_bytes_recursive doesn't
> work correctly.
> 
> As an example, for the following blkcg hierarchy:
>  (Made 1GB READ in test1, 512MB READ in test2)
>      test
>     /    \
>  test1   test2
> 
> $ head -n 1 test/test1/blkio.throttle.io_service_bytes_recursive
> 8:0 Read 1073684480
> $ head -n 1 test/test2/blkio.throttle.io_service_bytes_recursive
> 8:0 Read 537448448
> $ head -n 1 test/blkio.throttle.io_service_bytes_recursive
> 8:0 Read 537448448
> 
> Clearly, above data of "test" reflects "test2" not "test1"+"test2".
> 
> Do the correct summary in blkg_rwstat_recursive_sum().

LGTM, Tejun?

> 
> Signed-off-by: Xunlei Pang <xlpang@linux.alibaba.com>
> ---
>  block/blk-cgroup-rwstat.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-cgroup-rwstat.c b/block/blk-cgroup-rwstat.c
> index 85d5790..3304e84 100644
> --- a/block/blk-cgroup-rwstat.c
> +++ b/block/blk-cgroup-rwstat.c
> @@ -109,6 +109,7 @@ void blkg_rwstat_recursive_sum(struct blkcg_gq *blkg, struct blkcg_policy *pol,
>  
>  	lockdep_assert_held(&blkg->q->queue_lock);
>  
> +	memset(sum, 0, sizeof(*sum));
>  	rcu_read_lock();
>  	blkg_for_each_descendant_pre(pos_blkg, pos_css, blkg) {
>  		struct blkg_rwstat *rwstat;
> @@ -122,7 +123,7 @@ void blkg_rwstat_recursive_sum(struct blkcg_gq *blkg, struct blkcg_policy *pol,
>  			rwstat = (void *)pos_blkg + off;
>  
>  		for (i = 0; i < BLKG_RWSTAT_NR; i++)
> -			sum->cnt[i] = blkg_rwstat_read_counter(rwstat, i);
> +			sum->cnt[i] += blkg_rwstat_read_counter(rwstat, i);
>  	}
>  	rcu_read_unlock();
>  }
> 


-- 
Jens Axboe