linux-kernel - Re: [PATCH 7/9] blkio-cgroup-v9: Page tracking hooks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20090722111714.8af6c5d8.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Wed, 22 Jul 2009 11:17:14 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	Ryo Tsuruta <ryov@...inux.co.jp>
Cc:	linux-kernel@...r.kernel.org, dm-devel@...hat.com,
	containers@...ts.linux-foundation.org,
	virtualization@...ts.linux-foundation.org,
	xen-devel@...ts.xensource.com, agk@...hat.com
Subject: Re: [PATCH 7/9] blkio-cgroup-v9: Page tracking hooks

On Tue, 21 Jul 2009 23:23:16 +0900 (JST)
Ryo Tsuruta <ryov@...inux.co.jp> 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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/