All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Justin TerAvest <teravest@google.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	m-ikeda@ds.jp.nec.com, jaxboe@fusionio.com,
	linux-kernel@vger.kernel.org, ryov@valinux.co.jp,
	taka@valinux.co.jp, righi.andrea@gmail.com,
	guijianfeng@cn.fujitsu.com, balbir@linux.vnet.ibm.com,
	ctalbott@google.com, nauman@google.com, mrubin@google.com,
	linux-fsdevel@vger.kernel.org,
	Chris Mason <chris.mason@oracle.com>
Subject: Re: [RFC] Storing cgroup id in page->private (Was: Re: [RFC] [PATCH 0/6] Provide cgroup isolation for buffered writes.)
Date: Thu, 10 Mar 2011 14:41:06 -0500	[thread overview]
Message-ID: <20110310194106.GH29464@redhat.com> (raw)
In-Reply-To: <20110310191115.GG29464@redhat.com>

On Thu, Mar 10, 2011 at 02:11:15PM -0500, Vivek Goyal wrote:
> On Thu, Mar 10, 2011 at 10:57:52AM -0800, Justin TerAvest wrote:
> > On Thu, Mar 10, 2011 at 10:15 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > > On Thu, Mar 10, 2011 at 10:08:03AM -0800, Justin TerAvest wrote:
> > >
> > > [..]
> > >> > I don't like to increase size of page_cgroup but I think you can record
> > >> > information without increasing size of page_cgroup.
> > >> >
> > >> > A) As Andrea did, encode it to pc->flags.
> > >> >   But I'm afraid that there is a racy case because memory cgroup uses some
> > >> >   test_and_set() bits.
> > >> > B) I wonder why the information cannot be recorded in page->private.
> > >> >   When page has buffers, you can record the information to buffer struct.
> > >> >   About swapio (if you take care of), you can record information to bio.
> > >>
> > >> Hi Kame,
> > >>
> > >> I'm concerned that by using something like buffer_heads stored in
> > >> page->private, we will only be supported on some filesystems and not
> > >> others. In addition, I'm not sure if all filesystems attach buffer
> > >> heads at the same time; if page->private is modified in the flusher
> > >> thread, we might not be able to determine the thread that dirtied the
> > >> page in the first place.
> > >
> > > I think the person who dirtied the page can store the information in
> > > page->private (assuming buffer heads were not generated) and if flusher
> > > thread later ends up generating buffer heads and ends up modifying
> > > page->private, this can be copied in buffer heads?
> > 
> > This scares me a bit.
> > 
> > As I understand it, fs/ code expects total ownership of page->private.
> > This adds a responsibility for every user to copy the data through and
> > store it in the buffer head (or anything else). btrfs seems to do
> > something entirely different in some cases and store a different kind
> > of value.
> 
> If filesystems are using page->private for some other purpose also, then
> I guess we have issues. 
> 
> I am ccing linux-fsdevel to have some feedback on the idea of trying
> to store cgroup id of page dirtying thread in page->private and/or buffer
> head for tracking which group originally dirtied the page in IO controller
> during writeback.

A quick "grep" showed that btrfs, ceph and logfs are using page->private
for other purposes also.

I was under the impression that either page->private is null or it 
points to buffer heads for the writeback case. So storing the info
directly in either buffer head directly or first in page->private and
then transferring it to buffer heads would have helped. 

Thanks
Vivek

  reply	other threads:[~2011-03-10 19:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-08 21:20 [RFC] [PATCH 0/6] Provide cgroup isolation for buffered writes Justin TerAvest
2011-03-08 21:20 ` [PATCH 1/6] Add IO cgroup tracking " Justin TerAvest
2011-03-08 21:20 ` [PATCH 2/6] Make async queues per cgroup Justin TerAvest
2011-03-08 21:20 ` [PATCH 3/6] Modify CFQ to use IO tracking information Justin TerAvest
2011-03-08 21:20 ` [PATCH 4/6] With per-cgroup async, don't special case queues Justin TerAvest
2011-03-08 21:20 ` [PATCH 5/6] Add stat for per cgroup writeout done by flusher Justin TerAvest
2011-03-08 21:20 ` [PATCH 6/6] Per cgroup request descriptor counts Justin TerAvest
2011-03-08 22:43 ` [RFC] [PATCH 0/6] Provide cgroup isolation for buffered writes Vivek Goyal
2011-03-08 22:50   ` Vivek Goyal
2011-03-09 18:04     ` Justin TerAvest
2011-03-11  2:47       ` Vivek Goyal
2011-03-11 16:07         ` Justin TerAvest
2011-03-11 16:39           ` Vivek Goyal
2011-03-15 16:41             ` Justin TerAvest
2011-03-15 18:31               ` Vivek Goyal
2011-03-09  5:22 ` KAMEZAWA Hiroyuki
2011-03-09 15:19   ` Vivek Goyal
2011-03-09 18:05   ` Justin TerAvest
2011-03-10 18:08   ` Justin TerAvest
2011-03-10 18:15     ` Vivek Goyal
2011-03-10 18:57       ` Justin TerAvest
2011-03-10 19:11         ` [RFC] Storing cgroup id in page->private (Was: Re: [RFC] [PATCH 0/6] Provide cgroup isolation for buffered writes.) Vivek Goyal
2011-03-10 19:41           ` Vivek Goyal [this message]
2011-03-10 21:15             ` Chris Mason
2011-03-10 21:15               ` Chris Mason
2011-03-10 21:24               ` Andreas Dilger
2011-03-10 21:38                 ` Vivek Goyal
2011-03-10 21:43                   ` Chris Mason
2011-03-11  1:20                     ` KAMEZAWA Hiroyuki
2011-03-11  1:46                     ` Dave Chinner
2011-03-11  2:15                       ` Vivek Goyal
2011-03-11  2:52                         ` KAMEZAWA Hiroyuki
2011-03-11  3:15                           ` Vivek Goyal
2011-03-11  3:13                             ` KAMEZAWA Hiroyuki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110310194106.GH29464@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=chris.mason@oracle.com \
    --cc=ctalbott@google.com \
    --cc=guijianfeng@cn.fujitsu.com \
    --cc=jaxboe@fusionio.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m-ikeda@ds.jp.nec.com \
    --cc=mrubin@google.com \
    --cc=nauman@google.com \
    --cc=righi.andrea@gmail.com \
    --cc=ryov@valinux.co.jp \
    --cc=taka@valinux.co.jp \
    --cc=teravest@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.