2012-02-28 14:56:33

by Fengguang Wu

[permalink] [raw]
Subject: [PATCH 3/9] memcg: add kernel calls for memcg dirty page stats

From: Greg Thelen <[email protected]>

Add calls into memcg dirty page accounting. Notify memcg when pages
transition between clean, file dirty, writeback, and unstable nfs. This
allows the memory controller to maintain an accurate view of the amount
of its memory that is dirty.

Signed-off-by: Greg Thelen <[email protected]>
Signed-off-by: Andrea Righi <[email protected]>
Acked-by: KAMEZAWA Hiroyuki <[email protected]>
Reviewed-by: Daisuke Nishimura <[email protected]>
Reviewed-by: Minchan Kim <[email protected]>
Signed-off-by: Fengguang Wu <[email protected]>
---
fs/nfs/write.c | 4 ++++
mm/filemap.c | 1 +
mm/page-writeback.c | 4 ++++
mm/truncate.c | 1 +
4 files changed, 10 insertions(+)

--- linux.orig/fs/nfs/write.c 2012-02-19 10:53:14.000000000 +0800
+++ linux/fs/nfs/write.c 2012-02-19 10:53:21.000000000 +0800
@@ -449,6 +449,7 @@ nfs_mark_request_commit(struct nfs_page
nfsi->ncommit++;
spin_unlock(&inode->i_lock);
pnfs_mark_request_commit(req, lseg);
+ mem_cgroup_inc_page_stat(req->wb_page, MEMCG_NR_FILE_UNSTABLE_NFS);
inc_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
inc_bdi_stat(req->wb_page->mapping->backing_dev_info, BDI_RECLAIMABLE);
__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
@@ -460,6 +461,7 @@ nfs_clear_request_commit(struct nfs_page
struct page *page = req->wb_page;

if (test_and_clear_bit(PG_CLEAN, &(req)->wb_flags)) {
+ mem_cgroup_dec_page_stat(page, MEMCG_NR_FILE_UNSTABLE_NFS);
dec_zone_page_state(page, NR_UNSTABLE_NFS);
dec_bdi_stat(page->mapping->backing_dev_info, BDI_RECLAIMABLE);
return 1;
@@ -1408,6 +1410,8 @@ void nfs_retry_commit(struct list_head *
req = nfs_list_entry(page_list->next);
nfs_list_remove_request(req);
nfs_mark_request_commit(req, lseg);
+ mem_cgroup_dec_page_stat(req->wb_page,
+ MEMCG_NR_FILE_UNSTABLE_NFS);
dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
dec_bdi_stat(req->wb_page->mapping->backing_dev_info,
BDI_RECLAIMABLE);
--- linux.orig/mm/filemap.c 2012-02-19 10:53:14.000000000 +0800
+++ linux/mm/filemap.c 2012-02-19 10:53:21.000000000 +0800
@@ -142,6 +142,7 @@ void __delete_from_page_cache(struct pag
* having removed the page entirely.
*/
if (PageDirty(page) && mapping_cap_account_dirty(mapping)) {
+ mem_cgroup_dec_page_stat(page, MEMCG_NR_FILE_DIRTY);
dec_zone_page_state(page, NR_FILE_DIRTY);
dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
}
--- linux.orig/mm/page-writeback.c 2012-02-19 10:53:14.000000000 +0800
+++ linux/mm/page-writeback.c 2012-02-19 10:53:21.000000000 +0800
@@ -1933,6 +1933,7 @@ int __set_page_dirty_no_writeback(struct
void account_page_dirtied(struct page *page, struct address_space *mapping)
{
if (mapping_cap_account_dirty(mapping)) {
+ mem_cgroup_inc_page_stat(page, MEMCG_NR_FILE_DIRTY);
__inc_zone_page_state(page, NR_FILE_DIRTY);
__inc_zone_page_state(page, NR_DIRTIED);
__inc_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
@@ -1951,6 +1952,7 @@ EXPORT_SYMBOL(account_page_dirtied);
*/
void account_page_writeback(struct page *page)
{
+ mem_cgroup_inc_page_stat(page, MEMCG_NR_FILE_WRITEBACK);
inc_zone_page_state(page, NR_WRITEBACK);
}
EXPORT_SYMBOL(account_page_writeback);
@@ -2152,6 +2154,7 @@ int clear_page_dirty_for_io(struct page
* for more comments.
*/
if (TestClearPageDirty(page)) {
+ mem_cgroup_dec_page_stat(page, MEMCG_NR_FILE_DIRTY);
dec_zone_page_state(page, NR_FILE_DIRTY);
dec_bdi_stat(mapping->backing_dev_info,
BDI_RECLAIMABLE);
@@ -2188,6 +2191,7 @@ int test_clear_page_writeback(struct pag
ret = TestClearPageWriteback(page);
}
if (ret) {
+ mem_cgroup_dec_page_stat(page, MEMCG_NR_FILE_WRITEBACK);
dec_zone_page_state(page, NR_WRITEBACK);
inc_zone_page_state(page, NR_WRITTEN);
}
--- linux.orig/mm/truncate.c 2012-02-19 10:53:14.000000000 +0800
+++ linux/mm/truncate.c 2012-02-19 10:53:21.000000000 +0800
@@ -76,6 +76,7 @@ void cancel_dirty_page(struct page *page
if (TestClearPageDirty(page)) {
struct address_space *mapping = page->mapping;
if (mapping && mapping_cap_account_dirty(mapping)) {
+ mem_cgroup_dec_page_stat(page, MEMCG_NR_FILE_DIRTY);
dec_zone_page_state(page, NR_FILE_DIRTY);
dec_bdi_stat(mapping->backing_dev_info,
BDI_RECLAIMABLE);


2012-02-29 01:12:32

by Kamezawa Hiroyuki

[permalink] [raw]
Subject: Re: [PATCH 3/9] memcg: add kernel calls for memcg dirty page stats

On Tue, 28 Feb 2012 22:00:25 +0800
Fengguang Wu <[email protected]> wrote:

> From: Greg Thelen <[email protected]>
>
> Add calls into memcg dirty page accounting. Notify memcg when pages
> transition between clean, file dirty, writeback, and unstable nfs. This
> allows the memory controller to maintain an accurate view of the amount
> of its memory that is dirty.
>
> Signed-off-by: Greg Thelen <[email protected]>
> Signed-off-by: Andrea Righi <[email protected]>
> Acked-by: KAMEZAWA Hiroyuki <[email protected]>
> Reviewed-by: Daisuke Nishimura <[email protected]>
> Reviewed-by: Minchan Kim <[email protected]>
> Signed-off-by: Fengguang Wu <[email protected]>
> ---
> fs/nfs/write.c | 4 ++++
> mm/filemap.c | 1 +
> mm/page-writeback.c | 4 ++++
> mm/truncate.c | 1 +
> 4 files changed, 10 insertions(+)
>
> --- linux.orig/fs/nfs/write.c 2012-02-19 10:53:14.000000000 +0800
> +++ linux/fs/nfs/write.c 2012-02-19 10:53:21.000000000 +0800
> @@ -449,6 +449,7 @@ nfs_mark_request_commit(struct nfs_page
> nfsi->ncommit++;
> spin_unlock(&inode->i_lock);
> pnfs_mark_request_commit(req, lseg);
> + mem_cgroup_inc_page_stat(req->wb_page, MEMCG_NR_FILE_UNSTABLE_NFS);

Hmm...Is the status UNSTABLE_NFS cannot be obtaiend by 'struct page' ?

One idea to avoid adding a new flag to pc->flags is..

Can't we do this by following if 'req' exists per page ?

memcg = mem_cgroup_from_page(page); # update memcg's refcnt+1
req->memcg = memcg; # record memcg to req.
mem_cgroup_inc_nfs_unstable(memcg) # a new call



> inc_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
> inc_bdi_stat(req->wb_page->mapping->backing_dev_info, BDI_RECLAIMABLE);
> __mark_inode_dirty(inode, I_DIRTY_DATASYNC);
> @@ -460,6 +461,7 @@ nfs_clear_request_commit(struct nfs_page
> struct page *page = req->wb_page;
>
> if (test_and_clear_bit(PG_CLEAN, &(req)->wb_flags)) {
> + mem_cgroup_dec_page_stat(page, MEMCG_NR_FILE_UNSTABLE_NFS);
> dec_zone_page_state(page, NR_UNSTABLE_NFS);
> dec_bdi_stat(page->mapping->backing_dev_info, BDI_RECLAIMABLE);
> return 1;
> @@ -1408,6 +1410,8 @@ void nfs_retry_commit(struct list_head *
> req = nfs_list_entry(page_list->next);
> nfs_list_remove_request(req);
> nfs_mark_request_commit(req, lseg);
> + mem_cgroup_dec_page_stat(req->wb_page,
> + MEMCG_NR_FILE_UNSTABLE_NFS);
> dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
> dec_bdi_stat(req->wb_page->mapping->backing_dev_info,
> BDI_RECLAIMABLE);
> --- linux.orig/mm/filemap.c 2012-02-19 10:53:14.000000000 +0800
> +++ linux/mm/filemap.c 2012-02-19 10:53:21.000000000 +0800
> @@ -142,6 +142,7 @@ void __delete_from_page_cache(struct pag
> * having removed the page entirely.
> */
> if (PageDirty(page) && mapping_cap_account_dirty(mapping)) {
> + mem_cgroup_dec_page_stat(page, MEMCG_NR_FILE_DIRTY);


I think we can make use of PageDirty() as explained.

Thanks,
-Kame