Mailing List Archive

# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1276261236 -3600
# Node ID 94793113faef36e58e4960cba1c9f9f8bb6f60d8
# Parent 2d74cb5f973fe3eb26f3623ba461b8c9b3a40893
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@oracle.com>
xen-unstable changeset: 21595:39657b168068
xen-unstable date: Thu Jun 10 22:12:36 2010 +0100
---
xen/common/tmem.c | 50 ++++++++-------------------------------------
xen/include/xen/tmem_xen.h | 19 +++++++++--------
2 files changed, 20 insertions(+), 49 deletions(-)

diff -r 2d74cb5f973f -r 94793113faef xen/common/tmem.c
--- a/xen/common/tmem.c Thu Jun 10 10:19:26 2010 +0100
+++ b/xen/common/tmem.c Fri Jun 11 14:00: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 2d74cb5f973f -r 94793113faef xen/include/xen/tmem_xen.h
--- a/xen/include/xen/tmem_xen.h Thu Jun 10 10:19:26 2010 +0100
+++ b/xen/include/xen/tmem_xen.h Fri Jun 11 14:00: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@lists.xensource.com
http://lists.xensource.com/xen-changelog