Re: [PATCH 3/7] page_cgroup: provide a generic page tracking infrastructure

[Andrea Righi <righi.andrea@gmail.com>]

On Fri, Apr 24, 2009 at 10:11:09AM +0800, Gui Jianfeng wrote:
> Andrea Righi wrote:
> > Dirty pages in the page cache can be processed asynchronously by kernel
> > threads (pdflush) using a writeback policy. For this reason the real
> > writes to the underlying block devices occur in a different IO context
> > respect to the task that originally generated the dirty pages involved
> > in the IO operation. This makes the tracking and throttling of writeback
> > IO more complicate respect to the synchronous IO.
> > 
> > The page_cgroup infrastructure, currently available only for the memory
> > cgroup controller, can be used to store the owner of each page and
> > opportunely track the writeback IO. This information is encoded in
> > page_cgroup->flags.
> 
>   You encode id in page_cgroup->flags, if a cgroup get removed, IMHO, you
>   should remove the corresponding id in flags.

OK, the same same ID could be reused by another cgroup. I think this
should happen very rarely because IDs are recovered slowly anyway.

What about simply executing a sys_sync() when a io-throttle cgroup is
removed? If we're going to remove a cgroup no additional dirty page will
be generated by this cgroup, because it must be empty. And the sync
would allow that old dirty pages will be flushed back to disk (for those
pages the cgroup ID will be simply ignored).

>   One more thing, if a task is moving from a cgroup to another, the id in
>   flags also need to be changed.

I do not agree here. Even if a task is moving from a cgroup to another
the cgroup that generated the dirty page is always the old one. Remember
that we want to save cgroup's identity in this case, and not the task.

Thanks,
-Andrea