Re: [PATCH 2/2] loop: charge i/o per cgroup
From: Johannes Weiner <hannes@cmpxchg.org>
Date: 2020-02-06 15:46:29
Also in:
cgroups, linux-mm, lkml
Hello Dan, On Wed, Feb 05, 2020 at 02:33:48PM -0800, Dan Schatzberg wrote:
quoted hunk
@@ -1925,14 +1990,13 @@ static blk_status_t loop_queue_rq(struct blk_mq_hw_ctx *hctx, } /* always use the first bio's css */ + cmd->blk_css = NULL; #ifdef CONFIG_BLK_CGROUP - if (cmd->use_aio && rq->bio && rq->bio->bi_blkg) { - cmd->css = &bio_blkcg(rq->bio)->css; - css_get(cmd->css); - } else + if (rq->bio && rq->bio->bi_blkg) + cmd->blk_css = &bio_blkcg(rq->bio)->css; #endif - cmd->css = NULL; - kthread_queue_work(&lo->worker, &cmd->work); + + loop_queue_work(lo, cmd); return BLK_STS_OK; }@@ -1942,6 +2006,9 @@ static void loop_handle_cmd(struct loop_cmd *cmd) struct request *rq = blk_mq_rq_from_pdu(cmd); const bool write = op_is_write(req_op(rq)); struct loop_device *lo = rq->q->queuedata; +#ifdef CONFIG_MEMCG + struct cgroup_subsys_state *mem_css; +#endif int ret = 0; if (write && (lo->lo_flags & LO_FLAGS_READ_ONLY)) {@@ -1949,8 +2016,24 @@ static void loop_handle_cmd(struct loop_cmd *cmd) goto failed; } + if (cmd->blk_css) { +#ifdef CONFIG_MEMCG + mem_css = cgroup_get_e_css(cmd->blk_css->cgroup, + &memory_cgrp_subsys); + memalloc_use_memcg(mem_cgroup_from_css(mem_css)); +#endif + kthread_associate_blkcg(cmd->blk_css); + } + ret = do_req_filebacked(lo, rq); - failed: + + if (cmd->blk_css) { + kthread_associate_blkcg(NULL); +#ifdef CONFIG_MEMCG + memalloc_unuse_memcg(); +#endif
cgroup_get_e_css() acquires a reference, it looks like you're missing a css_put() here. I also wonder why you look up blk_css and mem_css in separate places. Since you already renamed cmd->css to cmd->blk_css, can you also add cmd->mem_css and pair up their lookup and refcounting? This should make loop_handle_cmd() a bit more straight-forward: if (cmd->blk_css) kthread_associate_blkcg(cmd->blk_css); if (cmd->mem_css) memalloc_use_memcg(mem_cgroup_from_css(mem_css)); ret = do_req_filebacked(lo, rq); memalloc_unuse_memcg(); kthread_associate_blkcg(NULL); All these functions have dummy implementations for !CONFIG_BLK_CGROUP, !CONFIG_MEMCG etc., so it shouldn't require any additional ifdefs.