WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
> 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.

I think that it is reasonable to add the hook right here rather than
to add many hooks to a variety of places.
 
> >     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. 

I think that it is not too bad to charge the owner of a page for
swap-out. From another perspective, it can be considered that swap-out
is caused by a process which uses a large amount of memory.

Thanks,
Ryo Tsuruta

> 
> 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

<Prev in Thread] Current Thread [Next in Thread>