zcache: fix refcount leak
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v2 1/9 zcache: fix refcount leak
In zcache_get_pool_by_id, the refcount of zcache_host is not increased, but it is always decreased in zcache_put_pool
--- drivers/staging/zcache/zcache-main.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/staging/zcache/zcache-main.c +2 -1
@@ -946,8 +946,9 @@ static struct tmem_pool *zcache_get_pool_by_id(uint16_t cli_id, uint16_t poolid) cli = &zcache_clients[cli_id];if (cli ==NULL ) goto out; -atomic_inc (&cli->refcount); } + +atomic_inc (&cli->refcount); pool =idr_find (&cli->tmem_pools, poolid);if (pool !=NULL )atomic_inc (&pool->refcount);
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 2 lines · Xiao Guangrong
In
zcache_get_pool_by_id, the refcount ofzcache_hostis not increased, but it is always decreased inzcache_put_pool
All of the patches (1-9) look good to me, so please also affix 'Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>'.
You also might want to send this patch series with Greg KH being on the To line- not just as CC -as he is the one committing the patches in the git tree.
Quoted 31 lines
Acked-by Seth Jennings <[email protected]>Signed-off-by Xiao Guangrong <[email protected]>--- drivers/staging/zcache/zcache-main.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)drivers/staging/zcache/zcache-main.c +2
-1@@ -946,8 +946,9 @@ static struct tmem_pool *zcache_get_pool_by_id(uint16_t cli_id, uint16_t poolid) cli = &zcache_clients[cli_id];if (cli ==NULL ) goto out; -atomic_inc (&cli->refcount); } + +atomic_inc (&cli->refcount); pool =idr_find (&cli->tmem_pools, poolid);if (pool !=NULL )atomic_inc (&pool->refcount);Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 7 lines · Konrad Rzeszutek Wilk
Quoted 2 lines · Xiao Guangrong
In
zcache_get_pool_by_id, the refcount ofzcache_hostis not increased, but it is always decreased inzcache_put_poolAll of the patches (1-9) look good to me, so please also affix 'Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>'.
Thank you, Konrad!
Greg, need i repost this patchset with Konrad's Reviewed-by?
Quoted 3 lines
You also might want to send this patch series with Greg KH being on the To line- not just as CC -as he is the one committing the patches in the git tree.
Yes, i did it in the [PATCH 0/9], but i do not know why it was missed in mm list, the later patches replied this patch, so my mail is in the To line-. :(
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Greg Kroah-Hartman
maintainerQuoted 5 lines · Xiao Guangrong
Quoted 4 lines · Konrad Rzeszutek Wilk
Quoted 2 lines · Xiao Guangrong
In
zcache_get_pool_by_id, the refcount ofzcache_hostis not increased, but it is always decreased inzcache_put_pool
Quoted 7 lines
Quoted 3 lines
All of the patches (1-9) look good to me, so please also affix 'Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>'.
Thank you, Konrad!
Greg, need i repost this patchset with Konrad's Reviewed-by?
No, I can add it when I apply them.
greg k-h
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 10 lines · Greg Kroah-Hartman
Quoted 9 lines · Xiao Guangrong
Quoted 7 lines · Konrad Rzeszutek Wilk
Quoted 2 lines · Xiao Guangrong
In
zcache_get_pool_by_id, the refcount ofzcache_hostis not increased, but it is always decreased inzcache_put_poolAll of the patches (1-9) look good to me, so please also affix 'Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>'.
Quoted 6 lines
Quoted 3 lines
Thank you, Konrad!
Greg, need i repost this patchset with Konrad's Reviewed-by?
No, I can add it when I apply them.
Greg, sorry to trouble you but this patches stayed in the list for nearly two weeks. If it is ok, could you please apply them? :)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Greg Kroah-Hartman
maintainerQuoted 13 lines · Xiao Guangrong
Quoted 12 lines · Greg Kroah-Hartman
Quoted 11 lines · Xiao Guangrong
Quoted 7 lines · Konrad Rzeszutek Wilk
Quoted 2 lines · Xiao Guangrong
In
zcache_get_pool_by_id, the refcount ofzcache_hostis not increased, but it is always decreased inzcache_put_poolAll of the patches (1-9) look good to me, so please also affix 'Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>'.
Thank you, Konrad!
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Greg, need i repost this patchset with Konrad's Reviewed-by?
No, I can add it when I apply them.
Greg, sorry to trouble you but this patches stayed in the list for nearly two weeks. If it is ok, could you please apply them? :)
I have now returned from my vacation that I was on for parts of the last two weeks and am digging out through my patch queue. I'll get to these soon, thanks for your patience.
greg k-h
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 2/9 zcache: fix a compile warning
In reply to a message not in the archiveFix:
drivers/staging/zcache/zcache-main.c: In function a??zcache_comp_opa??:drivers/staging/zcache/zcache-main.c:112:2: warning: a??reta?? may be used uninitial
--- drivers/staging/zcache/zcache-main.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/staging/zcache/zcache-main.c +2 -0
@@ -109,6 +109,8 @@ static inline int zcache_comp_op(enum comp_op op,case ZCACHE_COMPOP_DECOMPRESS : ret =crypto_comp_decompress (tfm, src, slen, dst, dlen);break ; +default : + ret = -EINVAL ; }put_cpu ();return ret;
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 3/9 zcache: remove unnecessary config option dependence
In reply to a message not in the archivezcache is enabled only if one of CONFIG_CLEANCACHE and CONFIG_FRONTSWAP is enabled, see the Kconfig:
depends on (CLEANCACHE || FRONTSWAP) && CRYPTO=y && X86
So, we can remove the check in the source code
--- drivers/staging/zcache/zcache-main.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-)
drivers/staging/zcache/zcache-main.c +2 -5
@@ -36,9 +36,6 @@#include "../zsmalloc/zsmalloc.h" -#if (!defined(CONFIG_CLEANCACHE ) && !defined(CONFIG_FRONTSWAP )) -#error "zcache is useless without CONFIG_CLEANCACHE or CONFIG_FRONTSWAP" -#endif #ifdef CONFIG_CLEANCACHE #include <linux/cleancache.h> #endif
@@ -2028,7 +2025,7 @@ static int __init zcache_init(void) goto out; }#endif /* CONFIG_SYSFS */ -#if defined(CONFIG_CLEANCACHE ) || defined(CONFIG_FRONTSWAP ) +if (zcache_enabled) {unsigned int cpu;
@@ -2059,7 +2056,7 @@ static int __init zcache_init(void)pr_err ("zcache: can't create client\n" ); goto out; } -#endif +#ifdef CONFIG_CLEANCACHE if (zcache_enabled && use_cleancache) {struct cleancache_ops old_ops;
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 4/9 zcache: mark zbud_init/zcache_comp_init as __init
In reply to a message not in the archiveThese functions are called only when system is initializing, so mark __init for them to free memory
--- drivers/staging/zcache/zcache-main.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/staging/zcache/zcache-main.c +2 -2
@@ -600,7 +600,7 @@ out:return ; } -static void zbud_init (void ) +static void __init zbud_init (void ) {int i;
@@ -1985,7 +1985,7 @@ static int __init enable_zcache_compressor(char *s)__setup ("zcache=" , enable_zcache_compressor); -static int zcache_comp_init (void ) +static int __init zcache_comp_init (void ) {int ret =0 ;
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 5/9 zcache: cleanup zbud_init
In reply to a message not in the archiveNeed not set global parameters to 0
--- drivers/staging/zcache/zcache-main.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/staging/zcache/zcache-main.c +2 -4
@@ -605,11 +605,9 @@ static void __init zbud_init(void)int i;INIT_LIST_HEAD (&zbud_buddied_list); - zcache_zbud_buddied_count =0 ; -for (i =0 ; i <NCHUNKS ; i++) { + +for (i =0 ; i <NCHUNKS ; i++)INIT_LIST_HEAD (&zbud_unbuddied[i].list); - zbud_unbuddied[i].count =0 ; - } }#ifdef CONFIG_SYSFS
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 6/9 zcache: optimize zcache_do_preload
In reply to a message not in the archivezcache_do_preload is called in zcache_put_page where IRQ is disabled, so, need not care preempt
--- drivers/staging/zcache/zcache-main.c | 21 +++++++++------------ 1 files changed, 9 insertions(+), 12 deletions(-)
drivers/staging/zcache/zcache-main.c +9 -12
@@ -1031,45 +1031,43 @@ static int zcache_do_preload(struct tmem_pool *pool) goto out;if (unlikely (zcache_obj_cache ==NULL )) goto out; -preempt_disable (); + +/* IRQ has already been disabled. */ kp = &__get_cpu_var (zcache_preloads);while (kp->nr <ARRAY_SIZE (kp->objnodes)) { -preempt_enable_no_resched (); objnode =kmem_cache_alloc (zcache_objnode_cache,ZCACHE_GFP_MASK );if (unlikely (objnode ==NULL )) { zcache_failed_alloc++; goto out; } -preempt_disable (); - kp = &__get_cpu_var (zcache_preloads); -if (kp->nr <ARRAY_SIZE (kp->objnodes)) - kp->objnodes[kp->nr++] = objnode; -else -kmem_cache_free (zcache_objnode_cache, objnode); + + kp->objnodes[kp->nr++] = objnode; } -preempt_enable_no_resched (); + obj =kmem_cache_alloc (zcache_obj_cache,ZCACHE_GFP_MASK );if (unlikely (obj ==NULL )) { zcache_failed_alloc++; goto out; } + page = (void *)__get_free_page (ZCACHE_GFP_MASK );if (unlikely (page ==NULL )) { zcache_failed_get_free_pages++;kmem_cache_free (zcache_obj_cache, obj); goto out; } -preempt_disable (); - kp = &__get_cpu_var (zcache_preloads); +if (kp->obj ==NULL ) kp->obj = obj;else kmem_cache_free (zcache_obj_cache, obj); +if (kp->page ==NULL ) kp->page = page;else free_page ((unsigned long )page); + ret =0 ; out:return ret;
@@ -1580,7 +1578,6 @@ static int zcache_put_page(int cli_id, int pool_id, struct tmem_oid *oidp, zcache_failed_pers_puts++; }zcache_put_pool (pool); -preempt_enable_no_resched (); }else { zcache_put_to_flush++;if (atomic_read (&pool->obj_count) >0 )
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 7/9 zcache: cleanup zcache_do_preload and zcache_put_page
In reply to a message not in the archiveCleanup the code for zcache_do_preload and zcache_put_page
--- drivers/staging/zcache/zcache-main.c | 37 ++++++++++++++------------------- 1 files changed, 16 insertions(+), 21 deletions(-)
drivers/staging/zcache/zcache-main.c +16 -21
@@ -1045,29 +1045,24 @@ static int zcache_do_preload(struct tmem_pool *pool) kp->objnodes[kp->nr++] = objnode; } - obj =kmem_cache_alloc (zcache_obj_cache,ZCACHE_GFP_MASK ); -if (unlikely (obj ==NULL )) { - zcache_failed_alloc++; - goto out; +if (!kp->obj) { + obj =kmem_cache_alloc (zcache_obj_cache,ZCACHE_GFP_MASK ); +if (unlikely (obj ==NULL )) { + zcache_failed_alloc++; + goto out; + } + kp->obj = obj; } - page = (void *)__get_free_page (ZCACHE_GFP_MASK ); -if (unlikely (page ==NULL )) { - zcache_failed_get_free_pages++; -kmem_cache_free (zcache_obj_cache, obj); - goto out; +if (!kp->page) { + page = (void *)__get_free_page (ZCACHE_GFP_MASK ); +if (unlikely (page ==NULL )) { + zcache_failed_get_free_pages++; + goto out; + } + kp->page = page; } -if (kp->obj ==NULL ) - kp->obj = obj; -else -kmem_cache_free (zcache_obj_cache, obj); - -if (kp->page ==NULL ) - kp->page = page; -else -free_page ((unsigned long )page); - ret =0 ; out:return ret;
@@ -1577,14 +1572,14 @@ static int zcache_put_page(int cli_id, int pool_id, struct tmem_oid *oidp,else zcache_failed_pers_puts++; } -zcache_put_pool (pool); }else { zcache_put_to_flush++;if (atomic_read (&pool->obj_count) >0 )/* the put fails whether the flush succeeds or not */ (void )tmem_flush_page (pool, oidp, index); -zcache_put_pool (pool); } + +zcache_put_pool (pool); out:return ret; }
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 8/9 zcache: introduce get_zcache_client
In reply to a message not in the archiveIntroduce get_zcache_client to remove the common code
--- drivers/staging/zcache/zcache-main.c | 46 +++++++++++++++++----------------- 1 files changed, 23 insertions(+), 23 deletions(-)
drivers/staging/zcache/zcache-main.c +23 -23
@@ -74,6 +74,17 @@ static inline uint16_t get_client_id_from_client(struct zcache_client *cli)return cli - &zcache_clients[0 ]; } +static struct zcache_client *get_zcache_client (uint16_t cli_id) +{ +if (cli_id ==LOCAL_CLIENT ) +return &zcache_host; + +if ((unsigned int )cli_id <MAX_CLIENTS ) +return &zcache_clients[cli_id]; + +return NULL ; +} +static inline bool is_local_client (struct zcache_client *cli) {return cli == &zcache_host;
@@ -935,15 +946,9 @@ static struct tmem_pool *zcache_get_pool_by_id(uint16_t cli_id, uint16_t poolid)struct tmem_pool *pool =NULL ;struct zcache_client *cli =NULL ; -if (cli_id ==LOCAL_CLIENT ) - cli = &zcache_host; -else { -if (cli_id >=MAX_CLIENTS ) - goto out; - cli = &zcache_clients[cli_id]; -if (cli ==NULL ) - goto out; - } + cli =get_zcache_client (cli_id); +if (!cli) + goto out;atomic_inc (&cli->refcount); pool =idr_find (&cli->tmem_pools, poolid);
@@ -966,13 +971,11 @@ static void zcache_put_pool(struct tmem_pool *pool)int zcache_new_client (uint16_t cli_id) { -struct zcache_client *cli =NULL ; +struct zcache_client *cli;int ret =-1 ; -if (cli_id ==LOCAL_CLIENT ) - cli = &zcache_host; -else if ((unsigned int )cli_id <MAX_CLIENTS ) - cli = &zcache_clients[cli_id]; + cli =get_zcache_client (cli_id); +if (cli ==NULL ) goto out;if (cli->allocated)
@@ -1649,17 +1652,16 @@ static int zcache_flush_object(int cli_id, int pool_id,static int zcache_destroy_pool (int cli_id,int pool_id) {struct tmem_pool *pool =NULL ; -struct zcache_client *cli =NULL ; +struct zcache_client *cli;int ret =-1 ;if (pool_id <0 ) goto out; -if (cli_id ==LOCAL_CLIENT ) - cli = &zcache_host; -else if ((unsigned int )cli_id <MAX_CLIENTS ) - cli = &zcache_clients[cli_id]; + + cli =get_zcache_client (cli_id);if (cli ==NULL ) goto out; +atomic_inc (&cli->refcount); pool =idr_find (&cli->tmem_pools, pool_id);if (pool ==NULL )
@@ -1686,12 +1688,10 @@ static int zcache_new_pool(uint16_t cli_id, uint32_t flags)struct zcache_client *cli =NULL ;int r; -if (cli_id ==LOCAL_CLIENT ) - cli = &zcache_host; -else if ((unsigned int )cli_id <MAX_CLIENTS ) - cli = &zcache_clients[cli_id]; + cli =get_zcache_client (cli_id);if (cli ==NULL ) goto out; +atomic_inc (&cli->refcount); pool =kmalloc (sizeof (struct tmem_pool ),GFP_ATOMIC );if (pool ==NULL ) {
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 9/9 zcache: cleanup the code between tmem_obj_init and tmem_obj_find
In reply to a message not in the archivetmem_obj_find and insertion tmem-obj have the some logic, we can integrate the code
--- drivers/staging/zcache/tmem.c | 63 +++++++++++++++++++++------------------- 1 files changed, 33 insertions(+), 30 deletions(-)
drivers/staging/zcache/tmem.c +33 -30
@@ -72,33 +72,49 @@ void tmem_register_pamops(struct tmem_pamops *m)* the hashbucket lock must be held. */ -/* searches for object==oid in pool, returns locked object if found */ -static struct tmem_obj *tmem_obj_find (struct tmem_hashbucket *hb, -struct tmem_oid *oidp) +static struct tmem_obj +*__tmem_obj_find (struct tmem_hashbucket *hb,struct tmem_oid *oidp, +struct rb_node **parent,struct rb_node ***link) { -struct rb_node *rbnode; -struct tmem_obj *obj; - - rbnode = hb->obj_rb_root.rb_node; -while (rbnode) { -BUG_ON (RB_EMPTY_NODE (rbnode)); - obj = rb_entry(rbnode, struct tmem_obj, rb_tree_node); +struct rb_node *_parent =NULL , **rbnode; +struct tmem_obj *obj =NULL ; + + rbnode = &hb->obj_rb_root.rb_node; +while (*rbnode) { +BUG_ON (RB_EMPTY_NODE (*rbnode)); + _parent = *rbnode; + obj = rb_entry(*rbnode, struct tmem_obj, + rb_tree_node);switch (tmem_oid_compare (oidp, &obj->oid)) {case 0 :/* equal */ goto out;case -1 : - rbnode = rbnode->rb_left; + rbnode = &(*rbnode)->rb_left;break ;case 1 : - rbnode = rbnode->rb_right; + rbnode = &(*rbnode)->rb_right;break ; } } + +if (parent) + *parent = _parent; +if (link) + *link = rbnode; + obj =NULL ; out:return obj; } + +/* searches for object==oid in pool, returns locked object if found */ +static struct tmem_obj *tmem_obj_find (struct tmem_hashbucket *hb, +struct tmem_oid *oidp) +{ +return __tmem_obj_find (hb, oidp,NULL ,NULL ); +} +static void tmem_pampd_destroy_all_in_obj (struct tmem_obj *);/* free an object that has no more pampds in it */
@@ -131,8 +147,7 @@ static void tmem_obj_init(struct tmem_obj *obj, struct tmem_hashbucket *hb, struct tmem_oid *oidp) {struct rb_root *root = &hb->obj_rb_root; -struct rb_node **new = &(root->rb_node), *parent =NULL ; -struct tmem_obj *this; +struct rb_node **new =NULL , *parent =NULL ;BUG_ON (pool ==NULL );atomic_inc (&pool->obj_count);
@@ -144,22 +159,10 @@ static void tmem_obj_init(struct tmem_obj *obj, struct tmem_hashbucket *hb, obj->pampd_count =0 ; (*tmem_pamops.new_obj)(obj);SET_SENTINEL (obj,OBJ ); -while (*new) { -BUG_ON (RB_EMPTY_NODE (*new)); - this = rb_entry(*new, struct tmem_obj, rb_tree_node); - parent = *new; -switch (tmem_oid_compare (oidp, &this->oid)) { -case 0 : -BUG ();/* already present; should never happen! */ -break ; -case -1 : - new = &(*new)->rb_left; -break ; -case 1 : - new = &(*new)->rb_right; -break ; - } - } + +if (__tmem_obj_find (hb, oidp, &parent, &new)) +BUG (); +rb_link_node (&obj->rb_tree_node, parent, new);rb_insert_color (&obj->rb_tree_node, root); }
Signature · 7 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>