[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-unstable] Tmem: fix domain refcount leak by returning to simpler model
# HG changeset patch # User Keir Fraser <keir.fraser@xxxxxxxxxx> # Date 1276204356 -3600 # Node ID 39657b16806826f88947b9882884374f3ed15c30 # Parent 6d35ded36a79198efe8dcf0d7ea53dcf2324e365 Tmem: fix domain refcount leak by returning to simpler model which claims a ref once when the tmem client is first associated with the domain, and puts it once when the tmem client is destroyed. Signed-off-by: Dan Magenheimer <dan.magenheimer@xxxxxxxxxx> --- xen/common/tmem.c | 50 ++++++++------------------------------------- xen/include/xen/tmem_xen.h | 19 +++++++++-------- 2 files changed, 20 insertions(+), 49 deletions(-) diff -r 6d35ded36a79 -r 39657b168068 xen/common/tmem.c --- a/xen/common/tmem.c Thu Jun 10 22:11:26 2010 +0100 +++ b/xen/common/tmem.c Thu Jun 10 22:12:36 2010 +0100 @@ -1916,8 +1916,6 @@ static NOINLINE int do_tmem_new_pool(cli client->pools[d_poolid] = global_shared_pools[s_poolid]; shared_pool_join(global_shared_pools[s_poolid], client); pool_free(pool); - if ( this_cli_id != CLI_ID_NULL ) - tmh_client_put(client->tmh); return d_poolid; } } @@ -1938,8 +1936,6 @@ static NOINLINE int do_tmem_new_pool(cli } } client->pools[d_poolid] = pool; - if ( this_cli_id != CLI_ID_NULL ) - tmh_client_put(client->tmh); list_add_tail(&pool->pool_list, &global_pool_list); pool->pool_id = d_poolid; pool->persistent = persistent; @@ -1949,8 +1945,6 @@ static NOINLINE int do_tmem_new_pool(cli fail: pool_free(pool); - if ( this_cli_id != CLI_ID_NULL ) - tmh_client_put(client->tmh); return -EPERM; } @@ -1976,7 +1970,6 @@ static int tmemc_freeze_pools(cli_id_t c if ( (client = tmh_client_from_cli_id(cli_id)) == NULL) return -1; client_freeze(client,freeze); - tmh_client_put(client->tmh); printk("tmem: all pools %s for %s=%d\n",s,cli_id_str,cli_id); } return 0; @@ -2185,10 +2178,8 @@ static int tmemc_list(cli_id_t cli_id, t } else if ( (client = tmh_client_from_cli_id(cli_id)) == NULL) return -1; - else { + else off = tmemc_list_client(client, buf, 0, len, use_long); - tmh_client_put(client->tmh); - } return 0; } @@ -2243,10 +2234,7 @@ static int tmemc_set_var(cli_id_t cli_id else if ( (client = tmh_client_from_cli_id(cli_id)) == NULL) return -1; else - { tmemc_set_var_one(client, subop, arg1); - tmh_client_put(client->tmh); - } return 0; } @@ -2272,7 +2260,6 @@ static NOINLINE int tmemc_shared_pool_au if ( auth == 0 ) client->shared_auth_uuid[i][0] = client->shared_auth_uuid[i][1] = -1L; - tmh_client_put(client->tmh); return 1; } if ( (auth == 1) && (client->shared_auth_uuid[i][0] == -1L) && @@ -2280,15 +2267,11 @@ static NOINLINE int tmemc_shared_pool_au free = i; } if ( auth == 0 ) - { - tmh_client_put(client->tmh); return 0; - } if ( auth == 1 && free == -1 ) return -ENOMEM; client->shared_auth_uuid[free][0] = uuid_lo; client->shared_auth_uuid[free][1] = uuid_hi; - tmh_client_put(client->tmh); return 1; } @@ -2370,8 +2353,6 @@ static NOINLINE int tmemc_save_subop(int client->frozen = client->was_frozen; rc = 0; } - if ( client ) - tmh_client_put(client->tmh); return rc; } @@ -2387,15 +2368,9 @@ static NOINLINE int tmemc_save_get_next_ unsigned int pagesize = 1 << (pool->pageshift+12); if ( pool == NULL || is_ephemeral(pool) ) - { - tmh_client_put(client->tmh); return -1; - } if ( bufsize < pagesize + sizeof(struct tmem_handle) ) - { - tmh_client_put(client->tmh); return -ENOMEM; - } tmem_spin_lock(&pers_lists_spinlock); if ( list_empty(&pool->persistent_page_list) ) @@ -2427,7 +2402,6 @@ static NOINLINE int tmemc_save_get_next_ out: tmem_spin_unlock(&pers_lists_spinlock); - tmh_client_put(client->tmh); return ret; } @@ -2442,10 +2416,7 @@ static NOINLINE int tmemc_save_get_next_ if ( client == NULL ) return 0; if ( bufsize < sizeof(struct tmem_handle) ) - { - tmh_client_put(client->tmh); return 0; - } tmem_spin_lock(&pers_lists_spinlock); if ( list_empty(&client->persistent_invalidated_list) ) goto out; @@ -2472,7 +2443,6 @@ static NOINLINE int tmemc_save_get_next_ ret = 1; out: tmem_spin_unlock(&pers_lists_spinlock); - tmh_client_put(client->tmh); return ret; } @@ -2482,11 +2452,10 @@ static int tmemc_restore_put_page(int cl client_t *client = tmh_client_from_cli_id(cli_id); pool_t *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN) ? NULL : client->pools[pool_id]; - int rc = pool ? do_tmem_put(pool,oid,index,0,0,0,bufsize,buf.p) : -1; - - if ( client ) - tmh_client_put(client->tmh); - return rc; + + if ( pool == NULL ) + return -1; + return do_tmem_put(pool,oid,index,0,0,0,bufsize,buf.p); } static int tmemc_restore_flush_page(int cli_id, int pool_id, uint64_t oid, @@ -2495,11 +2464,10 @@ static int tmemc_restore_flush_page(int client_t *client = tmh_client_from_cli_id(cli_id); pool_t *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN) ? NULL : client->pools[pool_id]; - int rc = pool ? do_tmem_flush_page(pool, oid, index) : -1; - - if ( client ) - tmh_client_put(client->tmh); - return rc; + + if ( pool == NULL ) + return -1; + return do_tmem_flush_page(pool,oid,index); } static NOINLINE int do_tmem_control(struct tmem_op *op) diff -r 6d35ded36a79 -r 39657b168068 xen/include/xen/tmem_xen.h --- a/xen/include/xen/tmem_xen.h Thu Jun 10 22:11:26 2010 +0100 +++ b/xen/include/xen/tmem_xen.h Thu Jun 10 22:12:36 2010 +0100 @@ -302,18 +302,18 @@ extern tmh_client_t *tmh_client_init(cli extern tmh_client_t *tmh_client_init(cli_id_t); extern void tmh_client_destroy(tmh_client_t *); -/* this appears to be unreliable when a domain is being shut down */ +/* we don't need to take a reference to the domain here as we hold + * one for the entire life of the client, so use rcu_lock_domain_by_id + * variant instead of get_domain_by_id() */ static inline struct client *tmh_client_from_cli_id(cli_id_t cli_id) { - struct domain *d = get_domain_by_id(cli_id); /* incs d->refcnt! */ + struct client *c; + struct domain *d = rcu_lock_domain_by_id(cli_id); if (d == NULL) return NULL; - return (struct client *)(d->tmem); -} - -static inline void tmh_client_put(tmh_client_t *tmh) -{ - put_domain(tmh->domain); + c = (struct client *)(d->tmem); + rcu_unlock_domain(d); + return c; } static inline struct client *tmh_client_from_current(void) @@ -336,6 +336,9 @@ static inline void tmh_set_client_from_i static inline void tmh_set_client_from_id(struct client *client, tmh_client_t *tmh, cli_id_t cli_id) { + /* here we DO want to take/hold a reference to the domain as + * this routine should be called exactly once when the client is created; + * the matching put_domain is in tmh_client_destroy */ struct domain *d = get_domain_by_id(cli_id); d->tmem = client; tmh->domain = d; _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-changelog
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |