linux-kernel - Re: [PATCH 3/6] memcg: make throttle_vm

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: <20101112081754.GE9131@cmpxchg.org>
Date:	Fri, 12 Nov 2010 09:17:55 +0100
From:	Johannes Weiner <hannes@...xchg.org>
To:	Greg Thelen <gthelen@...gle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Daisuke Nishimura <nishimura@....nes.nec.co.jp>,
	Wu Fengguang <fengguang.wu@...el.com>,
	Minchan Kim <minchan.kim@...il.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] memcg: make throttle_vm_writeout() memcg aware

On Tue, Nov 09, 2010 at 01:24:28AM -0800, Greg Thelen wrote:
> If called with a mem_cgroup, then throttle_vm_writeout() should
> query the given cgroup for its dirty memory usage limits.
> 
> dirty_writeback_pages() is no longer used, so delete it.
> 
> Signed-off-by: Greg Thelen <gthelen@...gle.com>
> ---
>  include/linux/writeback.h |    2 +-
>  mm/page-writeback.c       |   31 ++++++++++++++++---------------
>  mm/vmscan.c               |    2 +-
>  3 files changed, 18 insertions(+), 17 deletions(-)

> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index d717fa9..bf85062 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -131,18 +131,6 @@ EXPORT_SYMBOL(laptop_mode);
>  static struct prop_descriptor vm_completions;
>  static struct prop_descriptor vm_dirties;
>  
> -static unsigned long dirty_writeback_pages(void)
> -{
> -	unsigned long ret;
> -
> -	ret = mem_cgroup_page_stat(NULL, MEMCG_NR_DIRTY_WRITEBACK_PAGES);
> -	if ((long)ret < 0)
> -		ret = global_page_state(NR_UNSTABLE_NFS) +
> -			global_page_state(NR_WRITEBACK);

There are two bugfixes in this patch.  One is getting rid of this
fallback to global numbers that are compared to memcg limits.  The
other one is that reclaim will now throttle writeout based on the
cgroup it runs on behalf of, instead of that of the current task.

Both are undocumented and should arguably not even be in the same
patch...?

> @@ -703,12 +691,25 @@ void balance_dirty_pages_ratelimited_nr(struct address_space *mapping,
>  }
>  EXPORT_SYMBOL(balance_dirty_pages_ratelimited_nr);
>  
> -void throttle_vm_writeout(gfp_t gfp_mask)
> +/*
> + * Throttle the current task if it is near dirty memory usage limits.
> + * If @mem_cgroup is NULL or the root_cgroup, then use global dirty memory
> + * information; otherwise use the per-memcg dirty limits.
> + */
> +void throttle_vm_writeout(gfp_t gfp_mask, struct mem_cgroup *mem_cgroup)
>  {
>  	struct dirty_info dirty_info;
> +	unsigned long nr_writeback;
>  
>          for ( ; ; ) {
> -		global_dirty_info(&dirty_info);
> +		if (!mem_cgroup || !memcg_dirty_info(mem_cgroup, &dirty_info)) {
> +			global_dirty_info(&dirty_info);
> +			nr_writeback = global_page_state(NR_UNSTABLE_NFS) +
> +				global_page_state(NR_WRITEBACK);
> +		} else {
> +			nr_writeback = mem_cgroup_page_stat(
> +				mem_cgroup, MEMCG_NR_DIRTY_WRITEBACK_PAGES);
> +		}

Odd branch ordering, but I may be OCDing again.

	if (mem_cgroup && memcg_dirty_info())
		do_mem_cgroup_stuff()
	else
		do_global_stuff()

would be more natural, IMO.
--
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/