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

[Munehiro Ikeda <m-ikeda@ds.jp.nec.com>]

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.


-- 
IKEDA, Munehiro
   NEC Corporation of America
     m-ikeda@ds.jp.nec.com