[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090515074839.GA3340@linux>
Date: Fri, 15 May 2009 09:48:40 +0200
From: Andrea Righi <righi.andrea@...il.com>
To: Gui Jianfeng <guijianfeng@...fujitsu.com>
Cc: Vivek Goyal <vgoyal@...hat.com>,
Nauman Rafique <nauman@...gle.com>, dpshah@...gle.com,
lizf@...fujitsu.com, mikew@...gle.com, fchecconi@...il.com,
paolo.valente@...more.it, jens.axboe@...cle.com,
ryov@...inux.co.jp, fernando@....ntt.co.jp, s-uchida@...jp.nec.com,
taka@...inux.co.jp, jmoyer@...hat.com, dhaval@...ux.vnet.ibm.com,
balbir@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org,
containers@...ts.linux-foundation.org, agk@...hat.com,
dm-devel@...hat.com, snitzer@...hat.com, m-ikeda@...jp.nec.com,
akpm@...ux-foundation.org
Subject: Re: [PATCH] io-controller: Add io group reference handling for
request
On Fri, May 15, 2009 at 01:15:24PM +0800, Gui Jianfeng wrote:
> Vivek Goyal wrote:
> ...
> > }
> > @@ -1462,20 +1462,27 @@ struct io_cgroup *get_iocg_from_bio(stru
> > /*
> > * Find the io group bio belongs to.
> > * If "create" is set, io group is created if it is not already present.
> > + * If "curr" is set, io group is information is searched for current
> > + * task and not with the help of bio.
> > + *
> > + * FIXME: Can we assume that if bio is NULL then lookup group for current
> > + * task and not create extra function parameter ?
> > *
> > - * Note: There is a narrow window of race where a group is being freed
> > - * by cgroup deletion path and some rq has slipped through in this group.
> > - * Fix it.
> > */
> > -struct io_group *io_get_io_group_bio(struct request_queue *q, struct bio *bio,
> > - int create)
> > +struct io_group *io_get_io_group(struct request_queue *q, struct bio *bio,
> > + int create, int curr)
>
> Hi Vivek,
>
> IIUC we can get rid of curr, and just determine iog from bio. If bio is not NULL,
> get iog from bio, otherwise get it from current task.
Consider also that get_cgroup_from_bio() is much more slow than
task_cgroup() and need to lock/unlock_page_cgroup() in
get_blkio_cgroup_id(), while task_cgroup() is rcu protected.
BTW another optimization could be to use the blkio-cgroup functionality
only for dirty pages and cut out some blkio_set_owner(). For all the
other cases IO always occurs in the same context of the current task,
and you can use task_cgroup().
However, this is true only for page cache pages, for IO generated by
anonymous pages (swap) you still need the page tracking functionality
both for reads and writes.
-Andrea
>
> > {
> > struct cgroup *cgroup;
> > struct io_group *iog;
> > struct elv_fq_data *efqd = &q->elevator->efqd;
> >
> > rcu_read_lock();
> > - cgroup = get_cgroup_from_bio(bio);
> > +
> > + if (curr)
> > + cgroup = task_cgroup(current, io_subsys_id);
> > + else
> > + cgroup = get_cgroup_from_bio(bio);
> > +
> > if (!cgroup) {
> > if (create)
> > iog = efqd->root_group;
> > @@ -1500,7 +1507,7 @@ out:
> > rcu_read_unlock();
> > return iog;
> > }
>
> --
> Regards
> Gui Jianfeng
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists