On 11/14/2017 04:23 PM, Shaohua Li wrote:
> On Sun, Nov 12, 2017 at 02:26:12PM -0800, Tejun Heo wrote:
>> blkcg accounting is currently bio based, which is silly for request
>> based request_queues. This is silly as the number of bios doesn't
>> have much to do with the actual number of IOs issued to the underlying
>> device (can be significantly higher or lower) and may change depending
>> on the implementation details on how the bios are issued (e.g. from
>> the recent split-bios-while-issuing change).
>>
>> request based request_queues have QUEUE_FLAG_IO_STAT set by default
>> which controls the gendisk accounting. Do cgroup accounting for those
>> request_queues together with gendisk accounting on request completion.
>>
>> This makes cgroup accounting consistent with gendisk accounting and
>> what's happening on the system.
>>
>> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
>> ---
>> block/blk-core.c | 3 +++
>> include/linux/blk-cgroup.h | 18 +++++++++++++++++-
>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 048be4a..ad23b96 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -2429,6 +2429,7 @@ void blk_account_io_completion(struct request *req, unsigned int bytes)
>> cpu = part_stat_lock();
>> part = req->part;
>> part_stat_add(cpu, part, sectors[rw], bytes >> 9);
>> + blkcg_account_io_completion(req, bytes);
>> part_stat_unlock();
>> }
>> }
>> @@ -2454,6 +2455,8 @@ void blk_account_io_done(struct request *req)
>> part_round_stats(req->q, cpu, part);
>> part_dec_in_flight(req->q, part, rw);
>>
>> + blkcg_account_io_done(req);
>> +
>> hd_struct_put(part);
>> part_stat_unlock();
>> }
>> diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
>> index 96eed0f..f2f9691 100644
>> --- a/include/linux/blk-cgroup.h
>> +++ b/include/linux/blk-cgroup.h
>> @@ -715,7 +715,8 @@ static inline bool blkcg_bio_issue_check(struct request_queue *q,
>>
>> throtl = blk_throtl_bio(q, blkg, bio);
>>
>> - if (!throtl) {
>> + /* if @q does io stat, blkcg stats are updated together with them */
>> + if (!blk_queue_io_stat(q) && !throtl) {
>
> Reviewed-by: Shaohua Li <shli@xxxxxxxxxx>
>
> One nitpick, can we use q->request_fn to determine request based queue? I think
> that is more reliable and usual way for this.
That won't work for mq - but there is a helper for this, queue_is_rq_based().
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
