From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756798Ab1FCXqW (ORCPT ); Fri, 3 Jun 2011 19:46:22 -0400 Received: from smtp-out.google.com ([216.239.44.51]:12696 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753350Ab1FCXqV convert rfc822-to-8bit (ORCPT ); Fri, 3 Jun 2011 19:46:21 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=google.com; s=beta; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=befworQ1npVeIB/YvQWH8wKihFQ7acD1AKX2pCZuUS987uQB0PBckxqjLdgqFb0vML tk1xFgaFL9qQtWhKKNeQ== MIME-Version: 1.0 In-Reply-To: <20110603230939.GA2073@thinkpad> References: <1307117538-14317-1-git-send-email-gthelen@google.com> <1307117538-14317-4-git-send-email-gthelen@google.com> <20110603230939.GA2073@thinkpad> From: Greg Thelen Date: Fri, 3 Jun 2011 16:45:58 -0700 Message-ID: Subject: Re: [PATCH v8 03/12] memcg: add mem_cgroup_mark_inode_dirty() To: Andrea Righi Cc: Andrew Morton , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , containers@lists.osdl.org, linux-fsdevel@vger.kernel.org, Balbir Singh , KAMEZAWA Hiroyuki , Daisuke Nishimura , Minchan Kim , Johannes Weiner , Ciju Rajan K , David Rientjes , Wu Fengguang , Vivek Goyal , Dave Chinner Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT X-System-Of-Record: true Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 3, 2011 at 4:09 PM, Andrea Righi wrote: > On Fri, Jun 03, 2011 at 09:12:09AM -0700, Greg Thelen wrote: > ... >> diff --git a/fs/inode.c b/fs/inode.c >> index ce61a1b..9ecb0bb 100644 >> --- a/fs/inode.c >> +++ b/fs/inode.c >> @@ -228,6 +228,9 @@ int inode_init_always(struct super_block *sb, struct inode *inode) >>       mapping->assoc_mapping = NULL; >>       mapping->backing_dev_info = &default_backing_dev_info; >>       mapping->writeback_index = 0; >> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR >> +     mapping->i_memcg = 0; >> +#endif > > It won't change too much, since it's always 0, but shouldn't we use > I_MEMCG_SHARED by default? I agree, I_MEMCG_SHARED should be used instead of 0. Will include in -v9, if there is a need for -v9. >>       /* >>        * If the block_device provides a backing_dev_info for client >> diff --git a/include/linux/fs.h b/include/linux/fs.h >> index 29c02f6..deabca3 100644 >> --- a/include/linux/fs.h >> +++ b/include/linux/fs.h >> @@ -645,6 +645,9 @@ struct address_space { >>       spinlock_t              private_lock;   /* for use by the address_space */ >>       struct list_head        private_list;   /* ditto */ >>       struct address_space    *assoc_mapping; /* ditto */ >> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR >> +     unsigned short          i_memcg;        /* css_id of memcg dirtier */ >> +#endif >>  } __attribute__((aligned(sizeof(long)))); >>       /* >>        * On most architectures that alignment is already the case; but >> @@ -652,6 +655,12 @@ struct address_space { >>        * of struct page's "mapping" pointer be used for PAGE_MAPPING_ANON. >>        */ >> >> +/* >> + * When an address_space is shared by multiple memcg dirtieres, then i_memcg is >> + * set to this special, wildcard, css_id value (zero). >> + */ >> +#define I_MEMCG_SHARED 0 >> + >>  struct block_device { >>       dev_t                   bd_dev;  /* not a kdev_t - it's a search key */ >>       int                     bd_openers; >> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h >> index 77e47f5..14b6d67 100644 >> --- a/include/linux/memcontrol.h >> +++ b/include/linux/memcontrol.h >> @@ -103,6 +103,8 @@ mem_cgroup_prepare_migration(struct page *page, >>  extern void mem_cgroup_end_migration(struct mem_cgroup *mem, >>       struct page *oldpage, struct page *newpage, bool migration_ok); >> >> +void mem_cgroup_mark_inode_dirty(struct inode *inode); >> + >>  /* >>   * For memory reclaim. >>   */ >> @@ -273,6 +275,10 @@ static inline void mem_cgroup_end_migration(struct mem_cgroup *mem, >>  { >>  } >> >> +static inline void mem_cgroup_mark_inode_dirty(struct inode *inode) >> +{ >> +} >> + >>  static inline int mem_cgroup_get_reclaim_priority(struct mem_cgroup *mem) >>  { >>       return 0; >> diff --git a/include/trace/events/memcontrol.h b/include/trace/events/memcontrol.h >> new file mode 100644 >> index 0000000..781ef9fc >> --- /dev/null >> +++ b/include/trace/events/memcontrol.h >> @@ -0,0 +1,32 @@ >> +#undef TRACE_SYSTEM >> +#define TRACE_SYSTEM memcontrol >> + >> +#if !defined(_TRACE_MEMCONTROL_H) || defined(TRACE_HEADER_MULTI_READ) >> +#define _TRACE_MEMCONTROL_H >> + >> +#include >> +#include >> + >> +TRACE_EVENT(mem_cgroup_mark_inode_dirty, >> +     TP_PROTO(struct inode *inode), >> + >> +     TP_ARGS(inode), >> + >> +     TP_STRUCT__entry( >> +             __field(unsigned long, ino) >> +             __field(unsigned short, css_id) >> +             ), >> + >> +     TP_fast_assign( >> +             __entry->ino = inode->i_ino; >> +             __entry->css_id = >> +                     inode->i_mapping ? inode->i_mapping->i_memcg : 0; >> +             ), >> + >> +     TP_printk("ino=%ld css_id=%d", __entry->ino, __entry->css_id) >> +) >> + >> +#endif /* _TRACE_MEMCONTROL_H */ >> + >> +/* This part must be outside protection */ >> +#include >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index bf642b5..e83ef74 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -54,6 +54,9 @@ >> >>  #include >> >> +#define CREATE_TRACE_POINTS >> +#include >> + >>  struct cgroup_subsys mem_cgroup_subsys __read_mostly; >>  #define MEM_CGROUP_RECLAIM_RETRIES   5 >>  struct mem_cgroup *root_mem_cgroup __read_mostly; >> @@ -1122,6 +1125,27 @@ static int calc_inactive_ratio(struct mem_cgroup *memcg, unsigned long *present_ >>       return inactive_ratio; >>  } >> >> +/* >> + * Mark the current task's memcg as the memcg associated with inode.  Note: the >> + * recorded cgroup css_id is not guaranteed to remain correct.  The current task >> + * may be moved to another cgroup.  The memcg may also be deleted before the >> + * caller has time to use the i_memcg. >> + */ >> +void mem_cgroup_mark_inode_dirty(struct inode *inode) >> +{ >> +     struct mem_cgroup *mem; >> +     unsigned short id; >> + >> +     rcu_read_lock(); >> +     mem = mem_cgroup_from_task(current); >> +     id = mem ? css_id(&mem->css) : 0; > > Ditto. I agree, I_MEMCG_SHARED should be used instead of 0. >> +     rcu_read_unlock(); >> + >> +     inode->i_mapping->i_memcg = id; >> + >> +     trace_mem_cgroup_mark_inode_dirty(inode); >> +} >> + >>  int mem_cgroup_inactive_anon_is_low(struct mem_cgroup *memcg) >>  { >>       unsigned long active; >> -- >> 1.7.3.1 > > Thanks, > -Andrea >