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: <20090515140930.GC19350@redhat.com>
Date:	Fri, 15 May 2009 10:09:30 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Gui Jianfeng <guijianfeng@...fujitsu.com>
Cc:	Andrea Righi <righi.andrea@...il.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 04:16:38PM +0800, Gui Jianfeng wrote:
> 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.
> 

Gui, we are already doing it. page tracking functionality is used only
for async IO and for all sync IO, we are using submitting tasks's group
to determine io group bio belongs to.

	if (elv_bio_sync(bio)) {
		/* sync io. Determine cgroup from submitting task
		 * context.*/
                cgroup = task_cgroup(current, io_subsys_id);
                return cgroup;
        }

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