linux-kernel - Re: [PATCH] io-controller: Add io group reference handling for request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4A0D24E6.6010807@cn.fujitsu.com>
Date:	Fri, 15 May 2009 16:16:38 +0800
From:	Gui Jianfeng <guijianfeng@...fujitsu.com>
To:	Andrea Righi <righi.andrea@...il.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

Andrea Righi wrote:
> 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.

  Hi Andrea,

  Thanks for pointing this out. Yes, i think we can determine io group in
  terms of bio->bi_rw. If bio is a READ bio, just taking io group by 
  task_cgroup(). If it's a WRITE bio, getting it from blkio_cgroup.

> 
> -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
>>
> 
> 
> 

-- 
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/