Re: [PATCH -mmotm 3/3] memcg: dirty pages instrumentation

[Kirill A. Shutemov]

On Tue, Mar 2, 2010 at 1:02 PM, Andrea Righi <arighi@xxxxxxxxxxx> wrote:
> On Tue, Mar 02, 2010 at 12:11:10PM +0200, Kirill A. Shutemov wrote:
>> On Mon, Mar 1, 2010 at 11:23 PM, Andrea Righi <arighi@xxxxxxxxxxx> wrote:
>> > Apply the cgroup dirty pages accounting and limiting infrastructure to
>> > the opportune kernel functions.
>> >
>> > Signed-off-by: Andrea Righi <arighi@xxxxxxxxxxx>
>> > ---
>> > Âfs/fuse/file.c   Â|  Â5 +++
>> > Âfs/nfs/write.c   Â|  Â4 ++
>> > Âfs/nilfs2/segment.c | Â 10 +++++-
>> > Âmm/filemap.c    Â|  Â1 +
>> > Âmm/page-writeback.c | Â 84 ++++++++++++++++++++++++++++++++------------------
>> > Âmm/rmap.c      |  Â4 +-
>> > Âmm/truncate.c    |  Â2 +
>> > Â7 files changed, 76 insertions(+), 34 deletions(-)
>> >
>> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> > index a9f5e13..dbbdd53 100644
>> > --- a/fs/fuse/file.c
>> > +++ b/fs/fuse/file.c
>> > @@ -11,6 +11,7 @@
>> > Â#include <linux/pagemap.h>
>> > Â#include <linux/slab.h>
>> > Â#include <linux/kernel.h>
>> > +#include <linux/memcontrol.h>
>> > Â#include <linux/sched.h>
>> > Â#include <linux/module.h>
>> >
>> > @@ -1129,6 +1130,8 @@ static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
>> >
>> > Â Â Â Âlist_del(&req->writepages_entry);
>> > Â Â Â Âdec_bdi_stat(bdi, BDI_WRITEBACK);
>> > + Â Â Â mem_cgroup_update_stat(req->pages[0],
>> > + Â Â Â Â Â Â Â Â Â Â Â MEM_CGROUP_STAT_WRITEBACK_TEMP, -1);
>> > Â Â Â Âdec_zone_page_state(req->pages[0], NR_WRITEBACK_TEMP);
>> > Â Â Â Âbdi_writeout_inc(bdi);
>> > Â Â Â Âwake_up(&fi->page_waitq);
>> > @@ -1240,6 +1243,8 @@ static int fuse_writepage_locked(struct page *page)
>> > Â Â Â Âreq->inode = inode;
>> >
>> > Â Â Â Âinc_bdi_stat(mapping->backing_dev_info, BDI_WRITEBACK);
>> > + Â Â Â mem_cgroup_update_stat(tmp_page,
>> > + Â Â Â Â Â Â Â Â Â Â Â MEM_CGROUP_STAT_WRITEBACK_TEMP, 1);
>> > Â Â Â Âinc_zone_page_state(tmp_page, NR_WRITEBACK_TEMP);
>> > Â Â Â Âend_page_writeback(page);
>> >
>> > diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>> > index b753242..7316f7a 100644
>> > --- a/fs/nfs/write.c
>> > +++ b/fs/nfs/write.c
>> > @@ -439,6 +439,7 @@ nfs_mark_request_commit(struct nfs_page *req)
>> > Â Â Â Â Â Â Â Â Â Â Â Âreq->wb_index,
>> > Â Â Â Â Â Â Â Â Â Â Â ÂNFS_PAGE_TAG_COMMIT);
>> > Â Â Â Âspin_unlock(&inode->i_lock);
>> > + Â Â Â mem_cgroup_update_stat(req->wb_page, MEM_CGROUP_STAT_UNSTABLE_NFS, 1);
>> > Â Â Â Âinc_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
>> > Â Â Â Âinc_bdi_stat(req->wb_page->mapping->backing_dev_info, BDI_UNSTABLE);
>> > Â Â Â Â__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
>> > @@ -450,6 +451,7 @@ nfs_clear_request_commit(struct nfs_page *req)
>> > Â Â Â Âstruct page *page = req->wb_page;
>> >
>> > Â Â Â Âif (test_and_clear_bit(PG_CLEAN, &(req)->wb_flags)) {
>> > + Â Â Â Â Â Â Â mem_cgroup_update_stat(page, MEM_CGROUP_STAT_UNSTABLE_NFS, -1);
>> > Â Â Â Â Â Â Â Âdec_zone_page_state(page, NR_UNSTABLE_NFS);
>> > Â Â Â Â Â Â Â Âdec_bdi_stat(page->mapping->backing_dev_info, BDI_UNSTABLE);
>> > Â Â Â Â Â Â Â Âreturn 1;
>> > @@ -1273,6 +1275,8 @@ nfs_commit_list(struct inode *inode, struct list_head *head, int how)
>> > Â Â Â Â Â Â Â Âreq = nfs_list_entry(head->next);
>> > Â Â Â Â Â Â Â Ânfs_list_remove_request(req);
>> > Â Â Â Â Â Â Â Ânfs_mark_request_commit(req);
>> > + Â Â Â Â Â Â Â mem_cgroup_update_stat(req->wb_page,
>> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â MEM_CGROUP_STAT_UNSTABLE_NFS, -1);
>> > Â Â Â Â Â Â Â Âdec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
>> > Â Â Â Â Â Â Â Âdec_bdi_stat(req->wb_page->mapping->backing_dev_info,
>> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂBDI_UNSTABLE);
>> > diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
>> > index ada2f1b..aef6d13 100644
>> > --- a/fs/nilfs2/segment.c
>> > +++ b/fs/nilfs2/segment.c
>> > @@ -1660,8 +1660,11 @@ nilfs_copy_replace_page_buffers(struct page *page, struct list_head *out)
>> > Â Â Â Â} while (bh = bh->b_this_page, bh2 = bh2->b_this_page, bh != head);
>> > Â Â Â Âkunmap_atomic(kaddr, KM_USER0);
>> >
>> > - Â Â Â if (!TestSetPageWriteback(clone_page))
>> > + Â Â Â if (!TestSetPageWriteback(clone_page)) {
>> > + Â Â Â Â Â Â Â mem_cgroup_update_stat(clone_page,
>>
>> s/clone_page/page/
>
> mmh... shouldn't we use the same page used by TestSetPageWriteback() and
> inc_zone_page_state()?

Sorry, I've commented wrong hunk. It's for the next one.

>>
>> And #include <linux/memcontrol.h> is missed.
>
> OK.
>
> I'll apply your fixes and post a new version.
>
> Thanks for reviewing,
> -Andrea
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/