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 Content-Length: 5660 Lines: 170 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 > -- 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/