Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755747Ab0GIXqM (ORCPT ); Fri, 9 Jul 2010 19:46:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:24859 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750987Ab0GIXqJ (ORCPT ); Fri, 9 Jul 2010 19:46:09 -0400 Message-ID: <4C37B410.1070306@ds.jp.nec.com> Date: Fri, 09 Jul 2010 19:43:12 -0400 From: Munehiro Ikeda User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc11 Thunderbird/3.0.4 MIME-Version: 1.0 To: Andrea Righi CC: linux-kernel@vger.kernel.org, Vivek Goyal , Ryo Tsuruta , taka@valinux.co.jp, kamezawa.hiroyu@jp.fujitsu.com, Gui Jianfeng , akpm@linux-foundation.org, balbir@linux.vnet.ibm.com, Muuhh Ikeda Subject: Re: [RFC][PATCH 03/11] blkiocg async: Hooks for iotrack References: <4C369009.80503@ds.jp.nec.com> <4C36948C.9000902@ds.jp.nec.com> <20100709092450.GA1814@linux.develer.com> In-Reply-To: <20100709092450.GA1814@linux.develer.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5248 Lines: 158 Andrea Righi wrote, on 07/09/2010 05:24 AM: > On Thu, Jul 08, 2010 at 11:16:28PM -0400, Munehiro Ikeda wrote: >> Embedding hooks for iotrack to record process info, namely >> cgroup ID. >> This patch is based on a patch posted from Ryo Tsuruta on Oct 2, >> 2009 titled "Page tracking hooks". >> >> Signed-off-by: Hirokazu Takahashi >> Signed-off-by: Ryo Tsuruta >> Signed-off-by: Munehiro "Muuhh" Ikeda >> --- >> fs/buffer.c | 2 ++ >> fs/direct-io.c | 2 ++ >> mm/bounce.c | 2 ++ >> mm/filemap.c | 2 ++ >> mm/memory.c | 5 +++++ >> mm/page-writeback.c | 2 ++ >> mm/swap_state.c | 2 ++ >> 7 files changed, 17 insertions(+), 0 deletions(-) >> (snip) >> diff --git a/mm/memory.c b/mm/memory.c >> index 119b7cc..3eb2d0d 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -52,6 +52,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -2283,6 +2284,7 @@ gotten: >> */ >> ptep_clear_flush(vma, address, page_table); >> page_add_new_anon_rmap(new_page, vma, address); >> + blk_iotrack_set_owner(new_page, mm); >> /* >> * We call the notify macro here because, when using secondary >> * mmu page tables (such as kvm shadow page tables), we want the >> @@ -2718,6 +2720,7 @@ static int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma, >> flush_icache_page(vma, page); >> set_pte_at(mm, address, page_table, pte); >> page_add_anon_rmap(page, vma, address); >> + blk_iotrack_reset_owner(page, mm); >> /* It's better to call commit-charge after rmap is established */ >> mem_cgroup_commit_charge_swapin(page, ptr); >> >> @@ -2795,6 +2798,7 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma, >> >> inc_mm_counter_fast(mm, MM_ANONPAGES); >> page_add_new_anon_rmap(page, vma, address); >> + blk_iotrack_set_owner(page, mm); > > We're tracking anonymous memory here. Should we charge the cost of the > swap IO to the root cgroup or the actual owner of the page? there was a > previous discussion about this topic, but can't remember the details, > sorry. > > IMHO we could remove some overhead simply ignoring the tracking of > anonymous pages (swap IO) and just consider direct and writeback IO of > file pages. Well, this needs a decision. Actually who should be charged for swap IO has multiple options. (1) root cgroup (2) page owner (3) memory hogger who triggered swap The choice of this patch is (2). However, I agree that there is no concrete rationale to select (2) but (3), and (1) is the simplest and the best for performance. Alright, I will remove anonymous pages from tracking target. Thanks, Muuhh >> setpte: >> set_pte_at(mm, address, page_table, entry); >> >> @@ -2949,6 +2953,7 @@ static int __do_fault(struct mm_struct *mm, struct vm_area_struct *vma, >> if (anon) { >> inc_mm_counter_fast(mm, MM_ANONPAGES); >> page_add_new_anon_rmap(page, vma, address); >> + blk_iotrack_set_owner(page, mm); > > Ditto. > >> } else { >> inc_mm_counter_fast(mm, MM_FILEPAGES); >> page_add_file_rmap(page); >> diff --git a/mm/page-writeback.c b/mm/page-writeback.c >> index 54f28bd..f3e6b2c 100644 >> --- a/mm/page-writeback.c >> +++ b/mm/page-writeback.c >> @@ -23,6 +23,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1128,6 +1129,7 @@ int __set_page_dirty_nobuffers(struct page *page) >> BUG_ON(mapping2 != mapping); >> WARN_ON_ONCE(!PagePrivate(page)&& !PageUptodate(page)); >> account_page_dirtied(page, mapping); >> + blk_iotrack_reset_owner_pagedirty(page, current->mm); >> radix_tree_tag_set(&mapping->page_tree, >> page_index(page), PAGECACHE_TAG_DIRTY); >> } >> diff --git a/mm/swap_state.c b/mm/swap_state.c >> index e10f583..ab26978 100644 >> --- a/mm/swap_state.c >> +++ b/mm/swap_state.c >> @@ -19,6 +19,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> @@ -324,6 +325,7 @@ struct page *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, >> /* May fail (-ENOMEM) if radix-tree node allocation failed. */ >> __set_page_locked(new_page); >> SetPageSwapBacked(new_page); >> + blk_iotrack_set_owner(new_page, current->mm); > > Ditto. > >> err = __add_to_swap_cache(new_page, entry); >> if (likely(!err)) { >> radix_tree_preload_end(); >> -- >> 1.6.2.5 > > Thanks, > -Andrea > -- IKEDA, Munehiro NEC Corporation of America m-ikeda@ds.jp.nec.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/