[Xen-devel] Re: [PATCH 7/9] blkio-cgroup-v9: Page tracking hooks

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>]

On Tue, 21 Jul 2009 23:23:16 +0900 (JST)
Ryo Tsuruta <ryov@xxxxxxxxxxxxx> wrote:

> This patch contains several hooks that let the blkio-cgroup framework to know
> which blkio-cgroup is the owner of a page before starting I/O against the 
> page.

> @@ -464,6 +465,7 @@ int add_to_page_cache_locked(struct page
>                                       gfp_mask & GFP_RECLAIM_MASK);
>       if (error)
>               goto out;
> +     blkio_cgroup_set_owner(page, current->mm);
>  

This part is doubtful...Is this necessary ? 
I recommend you that the caller should attach owner by itself.


>       error = radix_tree_preload(gfp_mask & ~__GFP_HIGHMEM);
>       if (error == 0) {
> Index: linux-2.6.31-rc3/mm/memory.c
> ===================================================================
> --- linux-2.6.31-rc3.orig/mm/memory.c
> +++ linux-2.6.31-rc3/mm/memory.c
> @@ -51,6 +51,7 @@
>  #include <linux/init.h>
>  #include <linux/writeback.h>
>  #include <linux/memcontrol.h>
> +#include <linux/biotrack.h>
>  #include <linux/mmu_notifier.h>
>  #include <linux/kallsyms.h>
>  #include <linux/swapops.h>
> @@ -2115,6 +2116,7 @@ gotten:
>                */
>               ptep_clear_flush_notify(vma, address, page_table);
>               page_add_new_anon_rmap(new_page, vma, address);
> +             blkio_cgroup_set_owner(new_page, mm);

plz do this in swap-out code.

>               set_pte_at(mm, address, page_table, entry);
>               update_mmu_cache(vma, address, entry);
>               if (old_page) {
> @@ -2580,6 +2582,7 @@ static int do_swap_page(struct mm_struct
>       flush_icache_page(vma, page);
>       set_pte_at(mm, address, page_table, pte);
>       page_add_anon_rmap(page, vma, address);
> +     blkio_cgroup_reset_owner(page, mm);

and this.


>       /* It's better to call commit-charge after rmap is established */
>       mem_cgroup_commit_charge_swapin(page, ptr);
>  
> @@ -2644,6 +2647,7 @@ static int do_anonymous_page(struct mm_s
>               goto release;
>       inc_mm_counter(mm, anon_rss);
>       page_add_new_anon_rmap(page, vma, address);
> +     blkio_cgroup_set_owner(page, mm);
>       set_pte_at(mm, address, page_table, entry);
>  
and this.

>       /* No need to invalidate - it was non-present before */
> @@ -2791,6 +2795,7 @@ static int __do_fault(struct mm_struct *
>               if (anon) {
>                       inc_mm_counter(mm, anon_rss);
>                       page_add_new_anon_rmap(page, vma, address);
> +                     blkio_cgroup_set_owner(page, mm);
and this.

IMHO, later io for swap-out is caused by the caller of swapout, not page's
owner. plz charge to them or,
  - add special BLOCK CGROUP ID for the kernel's swap out. 

Bye,
-Kame

>               } else {
>                       inc_mm_counter(mm, file_rss);
>                       page_add_file_rmap(page);
> Index: linux-2.6.31-rc3/mm/page-writeback.c
> ===================================================================
> --- linux-2.6.31-rc3.orig/mm/page-writeback.c
> +++ linux-2.6.31-rc3/mm/page-writeback.c
> @@ -23,6 +23,7 @@
>  #include <linux/init.h>
>  #include <linux/backing-dev.h>
>  #include <linux/task_io_accounting_ops.h>
> +#include <linux/biotrack.h>
>  #include <linux/blkdev.h>
>  #include <linux/mpage.h>
>  #include <linux/rmap.h>
> @@ -1247,6 +1248,7 @@ int __set_page_dirty_nobuffers(struct pa
>                       BUG_ON(mapping2 != mapping);
>                       WARN_ON_ONCE(!PagePrivate(page) && !PageUptodate(page));
>                       account_page_dirtied(page, mapping);
> +                     blkio_cgroup_reset_owner_pagedirty(page, current->mm);
>                       radix_tree_tag_set(&mapping->page_tree,
>                               page_index(page), PAGECACHE_TAG_DIRTY);
>               }
> Index: linux-2.6.31-rc3/mm/swap_state.c
> ===================================================================
> --- linux-2.6.31-rc3.orig/mm/swap_state.c
> +++ linux-2.6.31-rc3/mm/swap_state.c
> @@ -18,6 +18,7 @@
>  #include <linux/pagevec.h>
>  #include <linux/migrate.h>
>  #include <linux/page_cgroup.h>
> +#include <linux/biotrack.h>
>  
>  #include <asm/pgtable.h>
>  
> @@ -307,6 +308,7 @@ struct page *read_swap_cache_async(swp_e
>                */
>               __set_page_locked(new_page);
>               SetPageSwapBacked(new_page);
> +             blkio_cgroup_set_owner(new_page, current->mm);
>               err = add_to_swap_cache(new_page, entry, gfp_mask & GFP_KERNEL);
>               if (likely(!err)) {
>                       /*
> --
> 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/
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel