Messages in this thread | | | Date | Mon, 28 Jun 2021 10:17:18 -0400 | From | Dan Schatzberg <> | Subject | Re: [PATCH 3/3] loop: Charge i/o to mem and blk cg |
| |
Hi Michal,
On Fri, Jun 25, 2021 at 05:01:03PM +0200, Michal Koutný wrote: > Hi. > > On Thu, Jun 10, 2021 at 10:39:44AM -0700, Dan Schatzberg <schatzberg.dan@gmail.com> wrote: > > The current code only associates with the existing blkcg when aio is > > used to access the backing file. This patch covers all types of i/o to > > the backing file and also associates the memcg so if the backing file is > > on tmpfs, memory is charged appropriately. > > > > This patch also exports cgroup_get_e_css and int_active_memcg so it > > can be used by the loop module. > > Wouldn't it be clearer to export (not explicitly inlined anymore) > set_active_memcg() instead of the int_active_memcg that's rather an > implementation detail?
Agreed that exporting int_active_memcg is an implementation detail, but would this prevent set_active_memcg from being inlined? Is that desireable?
> > > @@ -2111,13 +2112,18 @@ static blk_status_t loop_queue_rq(struct blk_mq_hw_ctx *hctx, > > } > > > > /* always use the first bio's css */ > > + cmd->blkcg_css = NULL; > > + cmd->memcg_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->blkcg_css = &bio_blkcg(rq->bio)->css; > > +#ifdef CONFIG_MEMCG > > + cmd->memcg_css = > > + cgroup_get_e_css(cmd->blkcg_css->cgroup, > > + &memory_cgrp_subsys); > > +#endif > > + } > > #endif > > - cmd->css = NULL; > > loop_queue_work(lo, cmd); > > I see you dropped the cmd->blkcg_css reference (while rq is handled). Is > it intentional?
Yes it is intentional. All requests (not just aio) go through the loop worker which grabs the blkcg reference in loop_queue_work() on construction. So I believe grabbing a reference per request is unnecessary.
| |