From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751713Ab1FEELm (ORCPT ); Sun, 5 Jun 2011 00:11:42 -0400 Received: from mail-pz0-f46.google.com ([209.85.210.46]:47051 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750721Ab1FEELl (ORCPT ); Sun, 5 Jun 2011 00:11:41 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=axqo1aaK2tbBkIbBN2HimiS8tNBbZAm/opKxBkc0r8qFKLmRxl+7seGgH46GGAtNjx zm/I+U0TkNLqokaR9JL7CT8s3C3UJYifjvOomgWrENmRtMR98GUs2f+iCOTCkjHIA1DA sghRTLa7IuNr0sr4HnUWl81oIQh5uHK070fpM= Date: Sun, 5 Jun 2011 13:11:30 +0900 From: Minchan Kim To: Greg Thelen Cc: Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, containers@lists.osdl.org, linux-fsdevel@vger.kernel.org, Andrea Righi , Balbir Singh , KAMEZAWA Hiroyuki , Daisuke Nishimura , Johannes Weiner , Ciju Rajan K , David Rientjes , Wu Fengguang , Vivek Goyal , Dave Chinner Subject: Re: [PATCH v8 11/12] writeback: make background writeback cgroup aware Message-ID: <20110605041130.GC5914@barrios-laptop> References: <1307117538-14317-1-git-send-email-gthelen@google.com> <1307117538-14317-12-git-send-email-gthelen@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1307117538-14317-12-git-send-email-gthelen@google.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 03, 2011 at 09:12:17AM -0700, Greg Thelen wrote: > When the system is under background dirty memory threshold but a cgroup > is over its background dirty memory threshold, then only writeback > inodes associated with the over-limit cgroup(s). > > In addition to checking if the system dirty memory usage is over the > system background threshold, over_bground_thresh() also checks if any > cgroups are over their respective background dirty memory thresholds. > The writeback_control.for_cgroup field is set to distinguish between a > system and memcg overage. > > If performing cgroup writeback, move_expired_inodes() skips inodes that > do not contribute dirty pages to the cgroup being written back. > > After writing some pages, wb_writeback() will call > mem_cgroup_writeback_done() to update the set of over-bg-limits memcg. > > Signed-off-by: Greg Thelen Reviewed-by: Minchan Kim There are some nitpicks at below. > --- > Changelog since v7: > - over_bground_thresh() now sets shared_inodes=1. In -v7 per memcg > background writeback did not, so it did not write pages of shared > inodes in background writeback. In the (potentially common) case > where the system dirty memory usage is below the system background > dirty threshold but at least one cgroup is over its background dirty > limit, then per memcg background writeback is queued for any > over-background-threshold cgroups. Background writeback should be > allowed to writeback shared inodes. The hope is that writing such > inodes has good chance of cleaning the inodes so they can transition > from shared to non-shared. Such a transition is good because then the > inode will remain unshared until it is written by multiple cgroup. > Non-shared inodes offer better isolation. Above comment should be in description. > > fs/fs-writeback.c | 32 ++++++++++++++++++++++++-------- > 1 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c > index 0174fcf..c0bfe62 100644 > --- a/fs/fs-writeback.c > +++ b/fs/fs-writeback.c > @@ -256,14 +256,17 @@ static void move_expired_inodes(struct list_head *delaying_queue, > LIST_HEAD(tmp); > struct list_head *pos, *node; > struct super_block *sb = NULL; > - struct inode *inode; > + struct inode *inode, *tmp_inode; > int do_sb_sort = 0; > > - while (!list_empty(delaying_queue)) { > - inode = wb_inode(delaying_queue->prev); > + list_for_each_entry_safe_reverse(inode, tmp_inode, delaying_queue, > + i_wb_list) { > if (wbc->older_than_this && > inode_dirtied_after(inode, *wbc->older_than_this)) > break; > + if (wbc->for_cgroup && > + !should_writeback_mem_cgroup_inode(inode, wbc)) > + continue; > if (sb && sb != inode->i_sb) > do_sb_sort = 1; > sb = inode->i_sb; > @@ -614,14 +617,22 @@ void writeback_inodes_wb(struct bdi_writeback *wb, > */ > #define MAX_WRITEBACK_PAGES 1024 > > -static inline bool over_bground_thresh(void) > +static inline bool over_bground_thresh(struct bdi_writeback *wb, At present, wb isn't used. Do you remain it intentionally for using in future? > + struct writeback_control *wbc) > { > unsigned long background_thresh, dirty_thresh; > > global_dirty_limits(&background_thresh, &dirty_thresh); > > - return (global_page_state(NR_FILE_DIRTY) + > - global_page_state(NR_UNSTABLE_NFS) > background_thresh); > + if (global_page_state(NR_FILE_DIRTY) + > + global_page_state(NR_UNSTABLE_NFS) > background_thresh) { > + wbc->for_cgroup = 0; > + return true; > + } > + > + wbc->for_cgroup = 1; > + wbc->shared_inodes = 1; > + return mem_cgroups_over_bground_dirty_thresh(); > } > > /* > @@ -700,7 +711,7 @@ static long wb_writeback(struct bdi_writeback *wb, > * For background writeout, stop when we are below the > * background dirty threshold > */ > - if (work->for_background && !over_bground_thresh()) > + if (work->for_background && !over_bground_thresh(wb, &wbc)) > break; > > if (work->for_kupdate || work->for_background) { > @@ -729,6 +740,9 @@ retry: > work->nr_pages -= write_chunk - wbc.nr_to_write; > wrote += write_chunk - wbc.nr_to_write; > > + if (write_chunk - wbc.nr_to_write > 0) > + mem_cgroup_writeback_done(); > + > /* > * Did we write something? Try for more > * > @@ -809,7 +823,9 @@ static unsigned long get_nr_dirty_pages(void) > > static long wb_check_background_flush(struct bdi_writeback *wb) > { > - if (over_bground_thresh()) { > + struct writeback_control wbc; > + > + if (over_bground_thresh(wb, &wbc)) { > > struct wb_writeback_work work = { > .nr_pages = LONG_MAX, > -- > 1.7.3.1 > -- Kind regards Minchan Kim