lkml.org 
[lkml]   [2021]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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.

    \
     
     \ /
      Last update: 2021-06-28 16:17    [W:2.870 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site