Re: [RFC][PATCH 02/11] blkiocg async: The main part of iotrack

[Andrea Righi <righi.andrea@gmail.com>]

On Fri, Jul 09, 2010 at 07:09:31PM -0400, Munehiro Ikeda wrote:
> KAMEZAWA Hiroyuki wrote, on 07/09/2010 03:38 AM:
> >On Thu, 08 Jul 2010 23:15:30 -0400
> >Munehiro Ikeda<m-ikeda@ds.jp.nec.com>  wrote:
> >
> >>Signed-off-by: Hirokazu Takahashi<taka@valinux.co.jp>
> >>Signed-off-by: Ryo Tsuruta<ryov@valinux.co.jp>
> >>Signed-off-by: Andrea Righi<arighi@develer.com>
> >>Signed-off-by: Munehiro "Muuhh" Ikeda<m-ikeda@ds.jp.nec.com>
> >>---
> >>  block/Kconfig.iosched       |    8 +++
> >>  block/Makefile              |    1 +
> >>  block/blk-iotrack.c         |  129 +++++++++++++++++++++++++++++++++++++++++++
> >>  include/linux/blk-iotrack.h |   62 +++++++++++++++++++++
> >>  include/linux/page_cgroup.h |   25 ++++++++
> >>  init/Kconfig                |    2 +-
> >>  mm/page_cgroup.c            |   91 +++++++++++++++++++++++++++---
> >>  7 files changed, 309 insertions(+), 9 deletions(-)
> >>  create mode 100644 block/blk-iotrack.c
> >>  create mode 100644 include/linux/blk-iotrack.h
> 
> (snip)
> 
> >>+/**
> >>+ * blk_iotrack_set_owner() - set the owner ID of a page.
> >>+ * @page:	the page we want to tag
> >>+ * @mm:		the mm_struct of a page owner
> >>+ *
> >>+ * Make a given page have the blkio-cgroup ID of the owner of this page.
> >>+ */
> >>+int blk_iotrack_set_owner(struct page *page, struct mm_struct *mm)
> >>+{
> >>+	struct blkio_cgroup *blkcg;
> >>+	unsigned short id = 0;	/* 0: default blkio_cgroup id */
> >>+
> >>+	if (blk_iotrack_disabled())
> >>+		return 0;
> >>+	if (!mm)
> >>+		goto out;
> >>+
> >>+	rcu_read_lock();
> >>+	blkcg = task_to_blkio_cgroup(rcu_dereference(mm->owner));
> >>+	if (likely(blkcg))
> >>+		id = css_id(&blkcg->css);
> >>+	rcu_read_unlock();
> >>+out:
> >>+	return page_cgroup_set_owner(page, id);
> >>+}
> >>+
> >
> >I think this is bad. I/O should be charged against threads i.e. "current",
> >because CFQ/blockio-cgroup provides per-thread control.
> >mm->owner is not suitable, I think.
> 
> OK, thanks.

BTW, this should be automatically fixed if you remove anonymous pages
tracking.

-Andrea