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: <20090515140643.GB19350@redhat.com>
Date:	Fri, 15 May 2009 10:06:43 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Andrea Righi <righi.andrea@...il.com>
Cc:	Gui Jianfeng <guijianfeng@...fujitsu.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 09:48:40AM +0200, 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.
> 

True.

> 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().
> 

Yes, may be in some cases we can avoid setting page owner. I will get
to it once I have got functionality going well. In the mean time if
you have a patch for it, it will be great.

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

Right now I am assuming that all the sync IO will belong to task
submitting the bio hence use task_cgroup() for that. Only for async
IO, I am trying to use page tracking functionality to determine the owner.
Look at elv_bio_sync(bio).

You seem to be saying that there are cases where even for sync IO, we
can't use submitting task's context and need to rely on page tracking
functionlity? In case of getting page (read) from swap, will it not happen
in the context of process who will take a page fault and initiate the
swap read?

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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ