* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID
2009-02-05 9:59 [RFC][PATCH] Reduce size of swap_cgroup by CSS ID KAMEZAWA Hiroyuki
@ 2009-02-05 13:17 ` Paul E. McKenney
2009-02-05 13:27 ` KAMEZAWA Hiroyuki
2009-02-06 2:07 ` Li Zefan
2009-02-09 5:55 ` [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2 KAMEZAWA Hiroyuki
2 siblings, 1 reply; 10+ messages in thread
From: Paul E. McKenney @ 2009-02-05 13:17 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, balbir, nishimura, lizf, linux-kernel
On Thu, Feb 05, 2009 at 06:59:59PM +0900, KAMEZAWA Hiroyuki wrote:
> !!EXPERIMENTAL!!
>
> against mmotm.
>
> This patch tires to use CSS ID for records in swap_cgroup instead of pointer.
> By this, on 64bit machine, size of swap_cgroup goes down to 2 bytes from 8bytes.
>
> This means, when 2GB of swap is equipped, (assume the page size is 4096bytes)
> From size of swap_cgroup = 2G/4k * 8 = 4Mbytes.
> To size of swap_cgroup = 2G/4k * 2 = 1Mbytes.
> Reduction is large. Of course, there are trade-offs. This CSS ID will add
> overhead to swap-in/swap-out/swap-free.
>
> But in general,
> - swap is a resource which the user tend to avoid use.
> - If swap is never used, swap_cgroup area is not used.
> - Reading traditional manuals, size of swap should be proportional to
> size of memory. Memory size of machine is increasing now. So, reducing
> size of swap_cgroup makes sense.
> Note:
> ID->CSS lookup routine has no locks, it's under RCU-Read-Side.
One question about css_tryget() below.
Thanx, Paul
> This is still under test. Any comments are welcome.
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> include/linux/page_cgroup.h | 9 +++----
> mm/memcontrol.c | 55 +++++++++++++++++++++++++++++++++++---------
> mm/page_cgroup.c | 22 ++++++++---------
> 3 files changed, 59 insertions(+), 27 deletions(-)
>
> Index: mmotm-2.6.29-Feb03/include/linux/page_cgroup.h
> ===================================================================
> --- mmotm-2.6.29-Feb03.orig/include/linux/page_cgroup.h
> +++ mmotm-2.6.29-Feb03/include/linux/page_cgroup.h
> @@ -91,22 +91,21 @@ static inline void page_cgroup_init(void
>
> #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> #include <linux/swap.h>
> -extern struct mem_cgroup *
> -swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem);
> -extern struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent);
> +extern unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id);
> +extern unsigned short lookup_swap_cgroup(swp_entry_t ent);
> extern int swap_cgroup_swapon(int type, unsigned long max_pages);
> extern void swap_cgroup_swapoff(int type);
> #else
> #include <linux/swap.h>
>
> static inline
> -struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
> +unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
> {
> return NULL;
> }
>
> static inline
> -struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent)
> +unsigned short lookup_swap_cgroup(swp_entry_t ent)
> {
> return NULL;
> }
> Index: mmotm-2.6.29-Feb03/mm/memcontrol.c
> ===================================================================
> --- mmotm-2.6.29-Feb03.orig/mm/memcontrol.c
> +++ mmotm-2.6.29-Feb03/mm/memcontrol.c
> @@ -1001,20 +1001,38 @@ nomem:
> return -ENOMEM;
> }
>
> +/*
> + * A helper function to get mem_cgroup from ID. must be called under
> + * rcu_read_lock(). Because css_tryget() is called under this, css_put
> + * should be called later.
> + */
> +static struct mem_cgroup *mem_cgroup_lookup_get(unsigned short id)
> +{
> + struct cgroup_subsys_state *css;
> +
> + /* ID 0 is unused ID */
> + if (!id)
> + return NULL;
> + css = css_lookup(&mem_cgroup_subsys, id);
> + if (css && css_tryget(css))
> + return container_of(css, struct mem_cgroup, css);
So css_tryget(), if successful, prevents the structure referenced by
css from being freed, correct? (If not, the range of the RCU read-side
critical sections surrounding calls to mem_cgroup_lookup_get() must be
extended.)
> + return NULL;
> +}
> +
> static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page)
> {
> - struct mem_cgroup *mem;
> + unsigned short id;
> + struct mem_cgroup *mem = NULL;
> swp_entry_t ent;
>
> if (!PageSwapCache(page))
> return NULL;
>
> ent.val = page_private(page);
> - mem = lookup_swap_cgroup(ent);
> - if (!mem)
> - return NULL;
> - if (!css_tryget(&mem->css))
> - return NULL;
> + id = lookup_swap_cgroup(ent);
> + rcu_read_lock();
> + mem = mem_cgroup_lookup_get(id);
> + rcu_read_unlock();
> return mem;
> }
>
> @@ -1275,11 +1293,16 @@ int mem_cgroup_cache_charge(struct page
>
> if (do_swap_account && !ret && PageSwapCache(page)) {
> swp_entry_t ent = {.val = page_private(page)};
> + unsigned short id;
> /* avoid double counting */
> - mem = swap_cgroup_record(ent, NULL);
> + id = swap_cgroup_record(ent, 0);
> + rcu_read_lock();
> + mem = mem_cgroup_lookup_get(id);
> + rcu_read_unlock();
> if (mem) {
> res_counter_uncharge(&mem->memsw, PAGE_SIZE);
> mem_cgroup_put(mem);
> + css_put(&mem->css);
> }
> }
> return ret;
> @@ -1345,13 +1368,18 @@ void mem_cgroup_commit_charge_swapin(str
> */
> if (do_swap_account && PageSwapCache(page)) {
> swp_entry_t ent = {.val = page_private(page)};
> + unsigned short id;
> struct mem_cgroup *memcg;
> - memcg = swap_cgroup_record(ent, NULL);
> +
> + id = swap_cgroup_record(ent, 0);
> + rcu_read_lock();
> + memcg = mem_cgroup_lookup_get(id);
> + rcu_read_unlock();
> if (memcg) {
> res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
> mem_cgroup_put(memcg);
> + css_put(&memcg->css);
> }
> -
> }
> /* add this page(page_cgroup) to the LRU we want. */
>
> @@ -1472,7 +1500,7 @@ void mem_cgroup_uncharge_swapcache(struc
> MEM_CGROUP_CHARGE_TYPE_SWAPOUT);
> /* record memcg information */
> if (do_swap_account && memcg) {
> - swap_cgroup_record(ent, memcg);
> + swap_cgroup_record(ent, css_id(&memcg->css));
> mem_cgroup_get(memcg);
> }
> if (memcg)
> @@ -1487,14 +1515,19 @@ void mem_cgroup_uncharge_swapcache(struc
> void mem_cgroup_uncharge_swap(swp_entry_t ent)
> {
> struct mem_cgroup *memcg;
> + unsigned short id;
>
> if (!do_swap_account)
> return;
>
> - memcg = swap_cgroup_record(ent, NULL);
> + id = swap_cgroup_record(ent, 0);
> + rcu_read_lock();
> + memcg = mem_cgroup_lookup_get(id);
> + rcu_read_unlock();
> if (memcg) {
> res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
> mem_cgroup_put(memcg);
> + css_put(&memcg->css);
> }
> }
> #endif
> Index: mmotm-2.6.29-Feb03/mm/page_cgroup.c
> ===================================================================
> --- mmotm-2.6.29-Feb03.orig/mm/page_cgroup.c
> +++ mmotm-2.6.29-Feb03/mm/page_cgroup.c
> @@ -290,7 +290,7 @@ struct swap_cgroup_ctrl swap_cgroup_ctrl
> * cgroup rather than pointer.
> */
> struct swap_cgroup {
> - struct mem_cgroup *val;
> + unsigned short id;
> };
> #define SC_PER_PAGE (PAGE_SIZE/sizeof(struct swap_cgroup))
> #define SC_POS_MASK (SC_PER_PAGE - 1)
> @@ -345,7 +345,7 @@ not_enough_page:
> * Returns old value at success, NULL at failure.
> * (Of course, old value can be NULL.)
> */
> -struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
> +unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
> {
> int type = swp_type(ent);
> unsigned long offset = swp_offset(ent);
> @@ -354,18 +354,18 @@ struct mem_cgroup *swap_cgroup_record(sw
> struct swap_cgroup_ctrl *ctrl;
> struct page *mappage;
> struct swap_cgroup *sc;
> - struct mem_cgroup *old;
> + unsigned short old;
>
> if (!do_swap_account)
> - return NULL;
> + return 0;
>
> ctrl = &swap_cgroup_ctrl[type];
>
> mappage = ctrl->map[idx];
> sc = page_address(mappage);
> sc += pos;
> - old = sc->val;
> - sc->val = mem;
> + old = sc->id;
> + sc->id = id;
>
> return old;
> }
> @@ -374,9 +374,9 @@ struct mem_cgroup *swap_cgroup_record(sw
> * lookup_swap_cgroup - lookup mem_cgroup tied to swap entry
> * @ent: swap entry to be looked up.
> *
> - * Returns pointer to mem_cgroup at success. NULL at failure.
> + * Returns CSS ID of mem_cgroup at success. NULL at failure.
> */
> -struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent)
> +unsigned short lookup_swap_cgroup(swp_entry_t ent)
> {
> int type = swp_type(ent);
> unsigned long offset = swp_offset(ent);
> @@ -385,16 +385,16 @@ struct mem_cgroup *lookup_swap_cgroup(sw
> struct swap_cgroup_ctrl *ctrl;
> struct page *mappage;
> struct swap_cgroup *sc;
> - struct mem_cgroup *ret;
> + unsigned short ret;
>
> if (!do_swap_account)
> - return NULL;
> + return 0; /* 0 is invalid ID */
>
> ctrl = &swap_cgroup_ctrl[type];
> mappage = ctrl->map[idx];
> sc = page_address(mappage);
> sc += pos;
> - ret = sc->val;
> + ret = sc->id;
> return ret;
> }
>
>
> --
> 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/
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID
2009-02-05 13:17 ` Paul E. McKenney
@ 2009-02-05 13:27 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 10+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-02-05 13:27 UTC (permalink / raw)
To: paulmck
Cc: KAMEZAWA Hiroyuki, linux-mm, balbir, nishimura, lizf, linux-kernel
Paul E. McKenney wrote:
> On Thu, Feb 05, 2009 at 06:59:59PM +0900, KAMEZAWA Hiroyuki wrote:
>> +static struct mem_cgroup *mem_cgroup_lookup_get(unsigned short id)
>> +{
>> + struct cgroup_subsys_state *css;
>> +
>> + /* ID 0 is unused ID */
>> + if (!id)
>> + return NULL;
>> + css = css_lookup(&mem_cgroup_subsys, id);
>> + if (css && css_tryget(css))
>> + return container_of(css, struct mem_cgroup, css);
>
> So css_tryget(), if successful, prevents the structure referenced by
> css from being freed, correct? (If not, the range of the RCU read-side
> critical sections surrounding calls to mem_cgroup_lookup_get() must be
> extended.)
>
One reference to css by css_tryget() prevents rmdir(). So, css will
not be freed until css_put() is called.
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID
2009-02-05 9:59 [RFC][PATCH] Reduce size of swap_cgroup by CSS ID KAMEZAWA Hiroyuki
2009-02-05 13:17 ` Paul E. McKenney
@ 2009-02-06 2:07 ` Li Zefan
2009-02-06 2:18 ` KAMEZAWA Hiroyuki
2009-02-09 5:55 ` [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2 KAMEZAWA Hiroyuki
2 siblings, 1 reply; 10+ messages in thread
From: Li Zefan @ 2009-02-06 2:07 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, balbir, nishimura, linux-kernel
> +/*
> + * A helper function to get mem_cgroup from ID. must be called under
> + * rcu_read_lock(). Because css_tryget() is called under this, css_put
> + * should be called later.
> + */
> +static struct mem_cgroup *mem_cgroup_lookup_get(unsigned short id)
> +{
> + struct cgroup_subsys_state *css;
> +
> + /* ID 0 is unused ID */
> + if (!id)
> + return NULL;
> + css = css_lookup(&mem_cgroup_subsys, id);
> + if (css && css_tryget(css))
> + return container_of(css, struct mem_cgroup, css);
> + return NULL;
> +}
the returned mem_cgroup needn't be protected by rcu_read_lock(), so I
think this is better:
rcu_read_lock();
css = css_lookup(&mem_cgroup_subsys, id);
rcu_read_unlock();
and no lock is needed when calling mem_cgroup_lookup_get().
> * Returns old value at success, NULL at failure.
> * (Of course, old value can be NULL.)
> */
> -struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
> +unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
kernel-doc needs to be updated
> * lookup_swap_cgroup - lookup mem_cgroup tied to swap entry
> * @ent: swap entry to be looked up.
> *
> - * Returns pointer to mem_cgroup at success. NULL at failure.
> + * Returns CSS ID of mem_cgroup at success. NULL at failure.
s/NULL/0/
> */
> -struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent)
> +unsigned short lookup_swap_cgroup(swp_entry_t ent)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID
2009-02-06 2:07 ` Li Zefan
@ 2009-02-06 2:18 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 10+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-02-06 2:18 UTC (permalink / raw)
To: Li Zefan; +Cc: linux-mm, balbir, nishimura, linux-kernel
On Fri, 06 Feb 2009 10:07:40 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:
> > +/*
> > + * A helper function to get mem_cgroup from ID. must be called under
> > + * rcu_read_lock(). Because css_tryget() is called under this, css_put
> > + * should be called later.
> > + */
> > +static struct mem_cgroup *mem_cgroup_lookup_get(unsigned short id)
> > +{
> > + struct cgroup_subsys_state *css;
> > +
> > + /* ID 0 is unused ID */
> > + if (!id)
> > + return NULL;
> > + css = css_lookup(&mem_cgroup_subsys, id);
> > + if (css && css_tryget(css))
> > + return container_of(css, struct mem_cgroup, css);
> > + return NULL;
> > +}
>
> the returned mem_cgroup needn't be protected by rcu_read_lock(), so I
> think this is better:
> rcu_read_lock();
> css = css_lookup(&mem_cgroup_subsys, id);
> rcu_read_unlock();
> and no lock is needed when calling mem_cgroup_lookup_get().
>
Hmm, maybe you're right.
> > * Returns old value at success, NULL at failure.
> > * (Of course, old value can be NULL.)
> > */
> > -struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
> > +unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
>
> kernel-doc needs to be updated
>
ok.
> > * lookup_swap_cgroup - lookup mem_cgroup tied to swap entry
> > * @ent: swap entry to be looked up.
> > *
> > - * Returns pointer to mem_cgroup at success. NULL at failure.
> > + * Returns CSS ID of mem_cgroup at success. NULL at failure.
>
> s/NULL/0/
>
yes.
Okay, thanks.
BTW, this patch is totally buggy ;( Sorry. Below is my memo.
==
- mem_cgroup can by destroyed() while there are swap_cgroup point to mem_cgroup.
If there are reference from swap_cgroup, kfree() is delayed.
- But this patch just uses css_tryget(). css_tryget() returnes false if
mem_cgroup is destroyed.
- So, refcnt for destroyed mem_cgroup will not be decreased and memory for
mem_cgroup will be never freed.
- Even if we use ID instead of pointer, the situation does not change.
we have to prevent ID to be reused....
- One choice for removing all swap_account at destroy() is forget all swap
accounts. But this may need "scan" all swap_cgroup in the system at rmdir().
This will be unacceptably slow on large swap systems.
==
I'll post v2 but it will be still tricky.
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2
2009-02-05 9:59 [RFC][PATCH] Reduce size of swap_cgroup by CSS ID KAMEZAWA Hiroyuki
2009-02-05 13:17 ` Paul E. McKenney
2009-02-06 2:07 ` Li Zefan
@ 2009-02-09 5:55 ` KAMEZAWA Hiroyuki
2009-02-10 8:07 ` Balbir Singh
2009-02-23 5:58 ` Daisuke Nishimura
2 siblings, 2 replies; 10+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-02-09 5:55 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, balbir, nishimura, lizf, linux-kernel
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Still !!EXPERIMENTAL!!, sorry.
This patch tires to use CSS ID for records in swap_cgroup.
By this, on 64bit machine, size of swap_cgroup goes down to 2 bytes from 8bytes.
This means, when 2GB of swap is equipped, (assume the page size is 4096bytes)
From size of swap_cgroup = 2G/4k * 8 = 4Mbytes.
To size of swap_cgroup = 2G/4k * 2 = 1Mbytes.
Reduction is large. Of course, there are trade-offs. This CSS ID will add
overhead to swap-in/swap-out/swap-free.
But in general,
- swap is a resource which the user tend to avoid use.
- If swap is never used, swap_cgroup area is not used.
- Reading traditional manuals, size of swap should be proportional to
size of memory. Memory size of machine is increasing now.
I think reducing size of swap_cgroup makes sense.
Note:
- ID->CSS lookup routine has no locks, it's under RCU-Read-Side.
- memcg can be obsolete at rmdir() but not freed while refcnt from
swap_cgroup is available.
This is still under test. Any comments are welcome.
Changelog: v1 -> v2
- removed css_tryget().
- fixed text in comments.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
include/linux/page_cgroup.h | 9 ++----
mm/memcontrol.c | 64 +++++++++++++++++++++++++++++++++++++-------
mm/page_cgroup.c | 26 ++++++++---------
3 files changed, 71 insertions(+), 28 deletions(-)
Index: mmotm-2.6.29-Feb05/include/linux/page_cgroup.h
===================================================================
--- mmotm-2.6.29-Feb05.orig/include/linux/page_cgroup.h
+++ mmotm-2.6.29-Feb05/include/linux/page_cgroup.h
@@ -91,22 +91,21 @@ static inline void page_cgroup_init(void
#ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
#include <linux/swap.h>
-extern struct mem_cgroup *
-swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem);
-extern struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent);
+extern unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id);
+extern unsigned short lookup_swap_cgroup(swp_entry_t ent);
extern int swap_cgroup_swapon(int type, unsigned long max_pages);
extern void swap_cgroup_swapoff(int type);
#else
#include <linux/swap.h>
static inline
-struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
+unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
{
return NULL;
}
static inline
-struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent)
+unsigned short lookup_swap_cgroup(swp_entry_t ent)
{
return NULL;
}
Index: mmotm-2.6.29-Feb05/mm/memcontrol.c
===================================================================
--- mmotm-2.6.29-Feb05.orig/mm/memcontrol.c
+++ mmotm-2.6.29-Feb05/mm/memcontrol.c
@@ -1001,20 +1001,41 @@ nomem:
return -ENOMEM;
}
+/*
+ * A helper function to get mem_cgroup from ID. must be called under
+ * rcu_read_lock(). The caller must check css_is_removed() or some if
+ * it's concern. (dropping refcnt from swap can be called against removed
+ * memcg.)
+ */
+static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
+{
+ struct cgroup_subsys_state *css;
+
+ /* ID 0 is unused ID */
+ if (!id)
+ return NULL;
+ css = css_lookup(&mem_cgroup_subsys, id);
+ if (!css)
+ return NULL;
+ return container_of(css, struct mem_cgroup, css);
+}
+
static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page)
{
- struct mem_cgroup *mem;
+ unsigned short id;
+ struct mem_cgroup *mem = NULL;
swp_entry_t ent;
if (!PageSwapCache(page))
return NULL;
ent.val = page_private(page);
- mem = lookup_swap_cgroup(ent);
- if (!mem)
- return NULL;
+ id = lookup_swap_cgroup(ent);
+ rcu_read_lock();
+ mem = mem_cgroup_lookup(id);
if (!css_tryget(&mem->css))
- return NULL;
+ mem = NULL;
+ rcu_read_unlock();
return mem;
}
@@ -1275,12 +1296,20 @@ int mem_cgroup_cache_charge(struct page
if (do_swap_account && !ret && PageSwapCache(page)) {
swp_entry_t ent = {.val = page_private(page)};
+ unsigned short id;
/* avoid double counting */
- mem = swap_cgroup_record(ent, NULL);
+ id = swap_cgroup_record(ent, 0);
+ rcu_read_lock();
+ mem = mem_cgroup_lookup(id);
if (mem) {
+ /*
+ * Recorded ID can be obsolete. We avoid calling
+ * css_tryget()
+ */
res_counter_uncharge(&mem->memsw, PAGE_SIZE);
mem_cgroup_put(mem);
}
+ rcu_read_unlock();
}
return ret;
}
@@ -1345,13 +1374,21 @@ void mem_cgroup_commit_charge_swapin(str
*/
if (do_swap_account && PageSwapCache(page)) {
swp_entry_t ent = {.val = page_private(page)};
+ unsigned short id;
struct mem_cgroup *memcg;
- memcg = swap_cgroup_record(ent, NULL);
+
+ id = swap_cgroup_record(ent, 0);
+ rcu_read_lock();
+ memcg = mem_cgroup_lookup(id);
if (memcg) {
+ /*
+ * This recorded memcg can be obsolete one. So, avoid
+ * calling css_tryget
+ */
res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
mem_cgroup_put(memcg);
}
-
+ rcu_read_unlock();
}
/* add this page(page_cgroup) to the LRU we want. */
@@ -1472,7 +1509,7 @@ void mem_cgroup_uncharge_swapcache(struc
MEM_CGROUP_CHARGE_TYPE_SWAPOUT);
/* record memcg information */
if (do_swap_account && memcg) {
- swap_cgroup_record(ent, memcg);
+ swap_cgroup_record(ent, css_id(&memcg->css));
mem_cgroup_get(memcg);
}
if (memcg)
@@ -1487,15 +1524,22 @@ void mem_cgroup_uncharge_swapcache(struc
void mem_cgroup_uncharge_swap(swp_entry_t ent)
{
struct mem_cgroup *memcg;
+ unsigned short id;
if (!do_swap_account)
return;
- memcg = swap_cgroup_record(ent, NULL);
+ id = swap_cgroup_record(ent, 0);
+ rcu_read_lock();
+ memcg = mem_cgroup_lookup(id);
if (memcg) {
+ /*
+ * This memcg can be obsolete one. We avoid calling css_tryget
+ */
res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
mem_cgroup_put(memcg);
}
+ rcu_read_unlock();
}
#endif
Index: mmotm-2.6.29-Feb05/mm/page_cgroup.c
===================================================================
--- mmotm-2.6.29-Feb05.orig/mm/page_cgroup.c
+++ mmotm-2.6.29-Feb05/mm/page_cgroup.c
@@ -290,7 +290,7 @@ struct swap_cgroup_ctrl swap_cgroup_ctrl
* cgroup rather than pointer.
*/
struct swap_cgroup {
- struct mem_cgroup *val;
+ unsigned short id;
};
#define SC_PER_PAGE (PAGE_SIZE/sizeof(struct swap_cgroup))
#define SC_POS_MASK (SC_PER_PAGE - 1)
@@ -342,10 +342,10 @@ not_enough_page:
* @ent: swap entry to be recorded into
* @mem: mem_cgroup to be recorded
*
- * Returns old value at success, NULL at failure.
- * (Of course, old value can be NULL.)
+ * Returns old value at success, 0 at failure.
+ * (Of course, old value can be 0.)
*/
-struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
+unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
{
int type = swp_type(ent);
unsigned long offset = swp_offset(ent);
@@ -354,18 +354,18 @@ struct mem_cgroup *swap_cgroup_record(sw
struct swap_cgroup_ctrl *ctrl;
struct page *mappage;
struct swap_cgroup *sc;
- struct mem_cgroup *old;
+ unsigned short old;
if (!do_swap_account)
- return NULL;
+ return 0;
ctrl = &swap_cgroup_ctrl[type];
mappage = ctrl->map[idx];
sc = page_address(mappage);
sc += pos;
- old = sc->val;
- sc->val = mem;
+ old = sc->id;
+ sc->id = id;
return old;
}
@@ -374,9 +374,9 @@ struct mem_cgroup *swap_cgroup_record(sw
* lookup_swap_cgroup - lookup mem_cgroup tied to swap entry
* @ent: swap entry to be looked up.
*
- * Returns pointer to mem_cgroup at success. NULL at failure.
+ * Returns CSS ID of mem_cgroup at success. 0 at failure. (0 is invalid ID)
*/
-struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent)
+unsigned short lookup_swap_cgroup(swp_entry_t ent)
{
int type = swp_type(ent);
unsigned long offset = swp_offset(ent);
@@ -385,16 +385,16 @@ struct mem_cgroup *lookup_swap_cgroup(sw
struct swap_cgroup_ctrl *ctrl;
struct page *mappage;
struct swap_cgroup *sc;
- struct mem_cgroup *ret;
+ unsigned short ret;
if (!do_swap_account)
- return NULL;
+ return 0;
ctrl = &swap_cgroup_ctrl[type];
mappage = ctrl->map[idx];
sc = page_address(mappage);
sc += pos;
- ret = sc->val;
+ ret = sc->id;
return ret;
}
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2
2009-02-09 5:55 ` [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2 KAMEZAWA Hiroyuki
@ 2009-02-10 8:07 ` Balbir Singh
2009-02-10 8:50 ` KAMEZAWA Hiroyuki
2009-02-23 5:58 ` Daisuke Nishimura
1 sibling, 1 reply; 10+ messages in thread
From: Balbir Singh @ 2009-02-10 8:07 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, nishimura, lizf, linux-kernel
* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-09 14:55:57]:
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> Still !!EXPERIMENTAL!!, sorry.
>
> This patch tires to use CSS ID for records in swap_cgroup.
> By this, on 64bit machine, size of swap_cgroup goes down to 2 bytes from 8bytes.
>
> This means, when 2GB of swap is equipped, (assume the page size is 4096bytes)
> From size of swap_cgroup = 2G/4k * 8 = 4Mbytes.
> To size of swap_cgroup = 2G/4k * 2 = 1Mbytes.
> Reduction is large. Of course, there are trade-offs. This CSS ID will add
> overhead to swap-in/swap-out/swap-free.
>
> But in general,
> - swap is a resource which the user tend to avoid use.
> - If swap is never used, swap_cgroup area is not used.
> - Reading traditional manuals, size of swap should be proportional to
> size of memory. Memory size of machine is increasing now.
>
> I think reducing size of swap_cgroup makes sense.
>
> Note:
> - ID->CSS lookup routine has no locks, it's under RCU-Read-Side.
> - memcg can be obsolete at rmdir() but not freed while refcnt from
> swap_cgroup is available.
>
> This is still under test. Any comments are welcome.
Yes, this does save memory. I wonder if we should move away from
mem_cgroup even in page_cgroup. I guess it depends on the cost of
mem_cgroup lookup from CSS ID.
>
> Changelog: v1 -> v2
> - removed css_tryget().
> - fixed text in comments.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> include/linux/page_cgroup.h | 9 ++----
> mm/memcontrol.c | 64 +++++++++++++++++++++++++++++++++++++-------
> mm/page_cgroup.c | 26 ++++++++---------
> 3 files changed, 71 insertions(+), 28 deletions(-)
>
> Index: mmotm-2.6.29-Feb05/include/linux/page_cgroup.h
> ===================================================================
> --- mmotm-2.6.29-Feb05.orig/include/linux/page_cgroup.h
> +++ mmotm-2.6.29-Feb05/include/linux/page_cgroup.h
> @@ -91,22 +91,21 @@ static inline void page_cgroup_init(void
>
> #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> #include <linux/swap.h>
> -extern struct mem_cgroup *
> -swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem);
> -extern struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent);
> +extern unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id);
> +extern unsigned short lookup_swap_cgroup(swp_entry_t ent);
> extern int swap_cgroup_swapon(int type, unsigned long max_pages);
> extern void swap_cgroup_swapoff(int type);
> #else
> #include <linux/swap.h>
>
> static inline
> -struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
> +unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
> {
> return NULL;
> }
>
> static inline
> -struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent)
> +unsigned short lookup_swap_cgroup(swp_entry_t ent)
> {
> return NULL;
> }
> Index: mmotm-2.6.29-Feb05/mm/memcontrol.c
> ===================================================================
> --- mmotm-2.6.29-Feb05.orig/mm/memcontrol.c
> +++ mmotm-2.6.29-Feb05/mm/memcontrol.c
> @@ -1001,20 +1001,41 @@ nomem:
> return -ENOMEM;
> }
>
> +/*
> + * A helper function to get mem_cgroup from ID. must be called under
> + * rcu_read_lock(). The caller must check css_is_removed() or some if
> + * it's concern. (dropping refcnt from swap can be called against removed
> + * memcg.)
> + */
> +static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
> +{
> + struct cgroup_subsys_state *css;
> +
> + /* ID 0 is unused ID */
> + if (!id)
> + return NULL;
> + css = css_lookup(&mem_cgroup_subsys, id);
> + if (!css)
> + return NULL;
> + return container_of(css, struct mem_cgroup, css);
> +}
> +
> static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page)
> {
> - struct mem_cgroup *mem;
> + unsigned short id;
> + struct mem_cgroup *mem = NULL;
> swp_entry_t ent;
>
> if (!PageSwapCache(page))
> return NULL;
>
> ent.val = page_private(page);
> - mem = lookup_swap_cgroup(ent);
> - if (!mem)
> - return NULL;
> + id = lookup_swap_cgroup(ent);
> + rcu_read_lock();
> + mem = mem_cgroup_lookup(id);
> if (!css_tryget(&mem->css))
> - return NULL;
> + mem = NULL;
This part is a bit confusing. If the page got swapped out and the CSS
it belonged to got swapped out, we set mem to NULL. Is this so that it
can be charged to root cgroup? If so, could you please add a comment
indicating the same.
> + rcu_read_unlock();
> return mem;
> }
>
> @@ -1275,12 +1296,20 @@ int mem_cgroup_cache_charge(struct page
>
> if (do_swap_account && !ret && PageSwapCache(page)) {
> swp_entry_t ent = {.val = page_private(page)};
> + unsigned short id;
> /* avoid double counting */
> - mem = swap_cgroup_record(ent, NULL);
> + id = swap_cgroup_record(ent, 0);
> + rcu_read_lock();
> + mem = mem_cgroup_lookup(id);
> if (mem) {
> + /*
> + * Recorded ID can be obsolete. We avoid calling
> + * css_tryget()
> + */
> res_counter_uncharge(&mem->memsw, PAGE_SIZE);
> mem_cgroup_put(mem);
> }
If !mem, do we leak charge? BTW, We no longer hold css references if
the page is swapped out?
> + rcu_read_unlock();
> }
> return ret;
> }
> @@ -1345,13 +1374,21 @@ void mem_cgroup_commit_charge_swapin(str
> */
> if (do_swap_account && PageSwapCache(page)) {
> swp_entry_t ent = {.val = page_private(page)};
> + unsigned short id;
> struct mem_cgroup *memcg;
> - memcg = swap_cgroup_record(ent, NULL);
> +
> + id = swap_cgroup_record(ent, 0);
> + rcu_read_lock();
> + memcg = mem_cgroup_lookup(id);
> if (memcg) {
> + /*
> + * This recorded memcg can be obsolete one. So, avoid
> + * calling css_tryget
> + */
> res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
> mem_cgroup_put(memcg);
> }
> -
Same question as above
> + rcu_read_unlock();
> }
> /* add this page(page_cgroup) to the LRU we want. */
>
> @@ -1472,7 +1509,7 @@ void mem_cgroup_uncharge_swapcache(struc
> MEM_CGROUP_CHARGE_TYPE_SWAPOUT);
> /* record memcg information */
> if (do_swap_account && memcg) {
> - swap_cgroup_record(ent, memcg);
> + swap_cgroup_record(ent, css_id(&memcg->css));
> mem_cgroup_get(memcg);
> }
> if (memcg)
> @@ -1487,15 +1524,22 @@ void mem_cgroup_uncharge_swapcache(struc
> void mem_cgroup_uncharge_swap(swp_entry_t ent)
> {
> struct mem_cgroup *memcg;
> + unsigned short id;
>
> if (!do_swap_account)
> return;
>
> - memcg = swap_cgroup_record(ent, NULL);
> + id = swap_cgroup_record(ent, 0);
> + rcu_read_lock();
> + memcg = mem_cgroup_lookup(id);
> if (memcg) {
> + /*
> + * This memcg can be obsolete one. We avoid calling css_tryget
> + */
> res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
> mem_cgroup_put(memcg);
> }
> + rcu_read_unlock();
> }
> #endif
>
> Index: mmotm-2.6.29-Feb05/mm/page_cgroup.c
> ===================================================================
> --- mmotm-2.6.29-Feb05.orig/mm/page_cgroup.c
> +++ mmotm-2.6.29-Feb05/mm/page_cgroup.c
> @@ -290,7 +290,7 @@ struct swap_cgroup_ctrl swap_cgroup_ctrl
> * cgroup rather than pointer.
> */
> struct swap_cgroup {
> - struct mem_cgroup *val;
> + unsigned short id;
> };
> #define SC_PER_PAGE (PAGE_SIZE/sizeof(struct swap_cgroup))
> #define SC_POS_MASK (SC_PER_PAGE - 1)
> @@ -342,10 +342,10 @@ not_enough_page:
> * @ent: swap entry to be recorded into
> * @mem: mem_cgroup to be recorded
> *
> - * Returns old value at success, NULL at failure.
> - * (Of course, old value can be NULL.)
> + * Returns old value at success, 0 at failure.
> + * (Of course, old value can be 0.)
> */
> -struct mem_cgroup *swap_cgroup_record(swp_entry_t ent, struct mem_cgroup *mem)
> +unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
> {
> int type = swp_type(ent);
> unsigned long offset = swp_offset(ent);
> @@ -354,18 +354,18 @@ struct mem_cgroup *swap_cgroup_record(sw
> struct swap_cgroup_ctrl *ctrl;
> struct page *mappage;
> struct swap_cgroup *sc;
> - struct mem_cgroup *old;
> + unsigned short old;
>
> if (!do_swap_account)
> - return NULL;
> + return 0;
>
> ctrl = &swap_cgroup_ctrl[type];
>
> mappage = ctrl->map[idx];
> sc = page_address(mappage);
> sc += pos;
> - old = sc->val;
> - sc->val = mem;
> + old = sc->id;
> + sc->id = id;
>
> return old;
> }
> @@ -374,9 +374,9 @@ struct mem_cgroup *swap_cgroup_record(sw
> * lookup_swap_cgroup - lookup mem_cgroup tied to swap entry
> * @ent: swap entry to be looked up.
> *
> - * Returns pointer to mem_cgroup at success. NULL at failure.
> + * Returns CSS ID of mem_cgroup at success. 0 at failure. (0 is invalid ID)
> */
> -struct mem_cgroup *lookup_swap_cgroup(swp_entry_t ent)
> +unsigned short lookup_swap_cgroup(swp_entry_t ent)
> {
> int type = swp_type(ent);
> unsigned long offset = swp_offset(ent);
> @@ -385,16 +385,16 @@ struct mem_cgroup *lookup_swap_cgroup(sw
> struct swap_cgroup_ctrl *ctrl;
> struct page *mappage;
> struct swap_cgroup *sc;
> - struct mem_cgroup *ret;
> + unsigned short ret;
>
> if (!do_swap_account)
> - return NULL;
> + return 0;
>
> ctrl = &swap_cgroup_ctrl[type];
> mappage = ctrl->map[idx];
> sc = page_address(mappage);
> sc += pos;
> - ret = sc->val;
> + ret = sc->id;
> return ret;
> }
>
>
--
Balbir
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2
2009-02-10 8:07 ` Balbir Singh
@ 2009-02-10 8:50 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 10+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-02-10 8:50 UTC (permalink / raw)
To: balbir; +Cc: linux-mm, nishimura, lizf, linux-kernel
On Tue, 10 Feb 2009 13:37:00 +0530
Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-09 14:55:57]:
>
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page)
> > {
> > - struct mem_cgroup *mem;
> > + unsigned short id;
> > + struct mem_cgroup *mem = NULL;
> > swp_entry_t ent;
> >
> > if (!PageSwapCache(page))
> > return NULL;
> >
> > ent.val = page_private(page);
> > - mem = lookup_swap_cgroup(ent);
> > - if (!mem)
> > - return NULL;
> > + id = lookup_swap_cgroup(ent);
> > + rcu_read_lock();
> > + mem = mem_cgroup_lookup(id);
> > if (!css_tryget(&mem->css))
> > - return NULL;
> > + mem = NULL;
>
> This part is a bit confusing. If the page got swapped out and the CSS
> it belonged to got swapped out, we set mem to NULL. Is this so that it
> can be charged to root cgroup?
IIUC, this charge will go to "current" process's cgroup.
> If so, could you please add a comment indicating the same.
>
Ah yes, I'll add some comments.
> > + rcu_read_unlock();
> > return mem;
> > }
> >
> > @@ -1275,12 +1296,20 @@ int mem_cgroup_cache_charge(struct page
> >
> > if (do_swap_account && !ret && PageSwapCache(page)) {
> > swp_entry_t ent = {.val = page_private(page)};
> > + unsigned short id;
> > /* avoid double counting */
> > - mem = swap_cgroup_record(ent, NULL);
> > + id = swap_cgroup_record(ent, 0);
> > + rcu_read_lock();
> > + mem = mem_cgroup_lookup(id);
> > if (mem) {
> > + /*
> > + * Recorded ID can be obsolete. We avoid calling
> > + * css_tryget()
> > + */
> > res_counter_uncharge(&mem->memsw, PAGE_SIZE);
> > mem_cgroup_put(mem);
> > }
>
> If !mem, do we leak charge?
No, it just means some other thread removed the ID before us.
> BTW, We no longer hold css references if the page is swapped out?
>
The situation is a bit complicated.
When cgroup is obsolete but its mem_cgroup is alive (because of reference
from swap), css_tryget() always fails. swap_cgroup_record() is atomic
compare-and-exchange, so I think we can trust mem_cgroup_put/get refcnt
management and doesn't need to rely on css's refcnt at swap management.
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2
2009-02-09 5:55 ` [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2 KAMEZAWA Hiroyuki
2009-02-10 8:07 ` Balbir Singh
@ 2009-02-23 5:58 ` Daisuke Nishimura
2009-02-24 0:10 ` KAMEZAWA Hiroyuki
1 sibling, 1 reply; 10+ messages in thread
From: Daisuke Nishimura @ 2009-02-23 5:58 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, balbir, lizf, linux-kernel, nishimura
I'm sorry for my late reply.
It looks good basically, but I have 1 comment.
> static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page)
> {
> - struct mem_cgroup *mem;
> + unsigned short id;
> + struct mem_cgroup *mem = NULL;
> swp_entry_t ent;
>
> if (!PageSwapCache(page))
> return NULL;
>
> ent.val = page_private(page);
> - mem = lookup_swap_cgroup(ent);
> - if (!mem)
> - return NULL;
> + id = lookup_swap_cgroup(ent);
> + rcu_read_lock();
> + mem = mem_cgroup_lookup(id);
> if (!css_tryget(&mem->css))
We should check whether "mem" is NULL or not before css_tryget, because
"mem" can be NULL(or "id" can be 0) if the page is on swapcache,
that is, remove_from_swap_cache has not been called yet.
Actually, I got NULL pointer dereference bug here.
> - return NULL;
> + mem = NULL;
> + rcu_read_unlock();
> return mem;
> }
>
Thanks,
Daisuke Nishimura.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2
2009-02-23 5:58 ` Daisuke Nishimura
@ 2009-02-24 0:10 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 10+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-02-24 0:10 UTC (permalink / raw)
To: Daisuke Nishimura; +Cc: linux-mm, balbir, lizf, linux-kernel
On Mon, 23 Feb 2009 14:58:28 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> I'm sorry for my late reply.
>
> It looks good basically, but I have 1 comment.
>
> > static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page)
> > {
> > - struct mem_cgroup *mem;
> > + unsigned short id;
> > + struct mem_cgroup *mem = NULL;
> > swp_entry_t ent;
> >
> > if (!PageSwapCache(page))
> > return NULL;
> >
> > ent.val = page_private(page);
> > - mem = lookup_swap_cgroup(ent);
> > - if (!mem)
> > - return NULL;
> > + id = lookup_swap_cgroup(ent);
> > + rcu_read_lock();
> > + mem = mem_cgroup_lookup(id);
> > if (!css_tryget(&mem->css))
> We should check whether "mem" is NULL or not before css_tryget, because
> "mem" can be NULL(or "id" can be 0) if the page is on swapcache,
> that is, remove_from_swap_cache has not been called yet.
>
> Actually, I got NULL pointer dereference bug here.
>
Okay, will fix.
Thanks,
-kame
> > - return NULL;
> > + mem = NULL;
> > + rcu_read_unlock();
> > return mem;
> > }
> >
>
>
> Thanks,
> Daisuke Nishimura.
> --
> 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/
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread