From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Bob Liu <lliubbo@gmail.com>
Cc: james.harper@bendigoit.com.au, ian.campbell@citrix.com,
JBeulich@suse.com, xen-devel@lists.xenproject.org
Subject: Re: [PATCH v3 09/15] tmem: cleanup: drop tmem_lock_all
Date: Wed, 11 Dec 2013 10:45:46 +0000 [thread overview]
Message-ID: <52A8425A.4070609@citrix.com> (raw)
In-Reply-To: <1386751844-32387-10-git-send-email-bob.liu@oracle.com>
On 11/12/13 08:50, Bob Liu wrote:
> tmem_lock_all is used for debug only, remove it from upstream to make
> tmem source code more readable and easier to maintain.
> And no_evict is meaningless without tmem_lock_all, this patch removes it
> also.
>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
This should probably be tagged with Coverity ID 1055654 which is a
locking order reversal between tmem_spinlock and the heap_lock.
I certainly think this is an appropriate fix.
~Andrew
> ---
> xen/common/tmem.c | 281 ++++++++++++++++----------------------------
> xen/common/tmem_xen.c | 3 -
> xen/include/xen/tmem_xen.h | 8 --
> 3 files changed, 101 insertions(+), 191 deletions(-)
>
> diff --git a/xen/common/tmem.c b/xen/common/tmem.c
> index 205ee95..a09a506 100644
> --- a/xen/common/tmem.c
> +++ b/xen/common/tmem.c
> @@ -98,7 +98,6 @@ struct tmem_object_root {
> struct tmem_pool *pool;
> domid_t last_client;
> spinlock_t obj_spinlock;
> - bool_t no_evict; /* if globally locked, pseudo-locks against eviction */
> };
>
> struct tmem_object_node {
> @@ -167,22 +166,12 @@ static atomic_t client_weight_total = ATOMIC_INIT(0);
> static int tmem_initialized = 0;
>
> /************ CONCURRENCY ***********************************************/
> -DEFINE_SPINLOCK(tmem_spinlock); /* used iff tmem_lock_all */
> -DEFINE_RWLOCK(tmem_rwlock); /* used iff !tmem_lock_all */
> +DEFINE_RWLOCK(tmem_rwlock);
> static DEFINE_SPINLOCK(eph_lists_spinlock); /* protects global AND clients */
> static DEFINE_SPINLOCK(pers_lists_spinlock);
>
> -#define tmem_spin_lock(_l) do {if (!tmem_lock_all) spin_lock(_l);}while(0)
> -#define tmem_spin_unlock(_l) do {if (!tmem_lock_all) spin_unlock(_l);}while(0)
> -#define tmem_read_lock(_l) do {if (!tmem_lock_all) read_lock(_l);}while(0)
> -#define tmem_read_unlock(_l) do {if (!tmem_lock_all) read_unlock(_l);}while(0)
> -#define tmem_write_lock(_l) do {if (!tmem_lock_all) write_lock(_l);}while(0)
> -#define tmem_write_unlock(_l) do {if (!tmem_lock_all) write_unlock(_l);}while(0)
> -#define tmem_write_trylock(_l) ((tmem_lock_all)?1:write_trylock(_l))
> -#define tmem_spin_trylock(_l) (tmem_lock_all?1:spin_trylock(_l))
> -
> -#define ASSERT_SPINLOCK(_l) ASSERT(tmem_lock_all || spin_is_locked(_l))
> -#define ASSERT_WRITELOCK(_l) ASSERT(tmem_lock_all || rw_is_write_locked(_l))
> +#define ASSERT_SPINLOCK(_l) ASSERT(spin_is_locked(_l))
> +#define ASSERT_WRITELOCK(_l) ASSERT(rw_is_write_locked(_l))
>
> /* global counters (should use long_atomic_t access) */
> static long global_eph_count = 0; /* atomicity depends on eph_lists_spinlock */
> @@ -250,7 +239,7 @@ static int pcd_copy_to_client(xen_pfn_t cmfn, struct tmem_page_descriptor *pgp)
> int ret;
>
> ASSERT(tmem_dedup_enabled());
> - tmem_read_lock(&pcd_tree_rwlocks[firstbyte]);
> + read_lock(&pcd_tree_rwlocks[firstbyte]);
> pcd = pgp->pcd;
> if ( pgp->size < PAGE_SIZE && pgp->size != 0 &&
> pcd->size < PAGE_SIZE && pcd->size != 0 )
> @@ -260,7 +249,7 @@ static int pcd_copy_to_client(xen_pfn_t cmfn, struct tmem_page_descriptor *pgp)
> ret = tmem_copy_tze_to_client(cmfn, pcd->tze, pcd->size);
> else
> ret = tmem_copy_to_client(cmfn, pcd->pfp, tmem_cli_buf_null);
> - tmem_read_unlock(&pcd_tree_rwlocks[firstbyte]);
> + read_unlock(&pcd_tree_rwlocks[firstbyte]);
> return ret;
> }
>
> @@ -283,14 +272,14 @@ static void pcd_disassociate(struct tmem_page_descriptor *pgp, struct tmem_pool
> if ( have_pcd_rwlock )
> ASSERT_WRITELOCK(&pcd_tree_rwlocks[firstbyte]);
> else
> - tmem_write_lock(&pcd_tree_rwlocks[firstbyte]);
> + write_lock(&pcd_tree_rwlocks[firstbyte]);
> list_del_init(&pgp->pcd_siblings);
> pgp->pcd = NULL;
> pgp->firstbyte = NOT_SHAREABLE;
> pgp->size = -1;
> if ( --pcd->pgp_ref_count )
> {
> - tmem_write_unlock(&pcd_tree_rwlocks[firstbyte]);
> + write_unlock(&pcd_tree_rwlocks[firstbyte]);
> return;
> }
>
> @@ -317,7 +306,7 @@ static void pcd_disassociate(struct tmem_page_descriptor *pgp, struct tmem_pool
> /* real physical page */
> tmem_page_free(pool,pfp);
> }
> - tmem_write_unlock(&pcd_tree_rwlocks[firstbyte]);
> + write_unlock(&pcd_tree_rwlocks[firstbyte]);
> }
>
>
> @@ -349,7 +338,7 @@ static int pcd_associate(struct tmem_page_descriptor *pgp, char *cdata, pagesize
> ASSERT(pfp_size <= PAGE_SIZE);
> ASSERT(!(pfp_size & (sizeof(uint64_t)-1)));
> }
> - tmem_write_lock(&pcd_tree_rwlocks[firstbyte]);
> + write_lock(&pcd_tree_rwlocks[firstbyte]);
>
> /* look for page match */
> root = &pcd_tree_roots[firstbyte];
> @@ -443,7 +432,7 @@ match:
> pgp->pcd = pcd;
>
> unlock:
> - tmem_write_unlock(&pcd_tree_rwlocks[firstbyte]);
> + write_unlock(&pcd_tree_rwlocks[firstbyte]);
> return ret;
> }
>
> @@ -552,7 +541,7 @@ static void pgp_delist(struct tmem_page_descriptor *pgp, bool_t no_eph_lock)
> if ( !is_persistent(pgp->us.obj->pool) )
> {
> if ( !no_eph_lock )
> - tmem_spin_lock(&eph_lists_spinlock);
> + spin_lock(&eph_lists_spinlock);
> if ( !list_empty(&pgp->us.client_eph_pages) )
> client->eph_count--;
> ASSERT(client->eph_count >= 0);
> @@ -562,20 +551,20 @@ static void pgp_delist(struct tmem_page_descriptor *pgp, bool_t no_eph_lock)
> ASSERT(global_eph_count >= 0);
> list_del_init(&pgp->global_eph_pages);
> if ( !no_eph_lock )
> - tmem_spin_unlock(&eph_lists_spinlock);
> + spin_unlock(&eph_lists_spinlock);
> } else {
> if ( client->live_migrating )
> {
> - tmem_spin_lock(&pers_lists_spinlock);
> + spin_lock(&pers_lists_spinlock);
> list_add_tail(&pgp->client_inv_pages,
> &client->persistent_invalidated_list);
> if ( pgp != pgp->us.obj->pool->cur_pgp )
> list_del_init(&pgp->us.pool_pers_pages);
> - tmem_spin_unlock(&pers_lists_spinlock);
> + spin_unlock(&pers_lists_spinlock);
> } else {
> - tmem_spin_lock(&pers_lists_spinlock);
> + spin_lock(&pers_lists_spinlock);
> list_del_init(&pgp->us.pool_pers_pages);
> - tmem_spin_unlock(&pers_lists_spinlock);
> + spin_unlock(&pers_lists_spinlock);
> }
> }
> }
> @@ -709,7 +698,7 @@ static struct tmem_object_root * obj_find(struct tmem_pool *pool, struct oid *oi
> struct tmem_object_root *obj;
>
> restart_find:
> - tmem_read_lock(&pool->pool_rwlock);
> + read_lock(&pool->pool_rwlock);
> node = pool->obj_rb_root[oid_hash(oidp)].rb_node;
> while ( node )
> {
> @@ -717,17 +706,12 @@ restart_find:
> switch ( oid_compare(&obj->oid, oidp) )
> {
> case 0: /* equal */
> - if ( tmem_lock_all )
> - obj->no_evict = 1;
> - else
> + if ( !spin_trylock(&obj->obj_spinlock) )
> {
> - if ( !tmem_spin_trylock(&obj->obj_spinlock) )
> - {
> - tmem_read_unlock(&pool->pool_rwlock);
> - goto restart_find;
> - }
> - tmem_read_unlock(&pool->pool_rwlock);
> + read_unlock(&pool->pool_rwlock);
> + goto restart_find;
> }
> + read_unlock(&pool->pool_rwlock);
> return obj;
> case -1:
> node = node->rb_left;
> @@ -736,7 +720,7 @@ restart_find:
> node = node->rb_right;
> }
> }
> - tmem_read_unlock(&pool->pool_rwlock);
> + read_unlock(&pool->pool_rwlock);
> return NULL;
> }
>
> @@ -763,7 +747,7 @@ static void obj_free(struct tmem_object_root *obj, int no_rebalance)
> /* use no_rebalance only if all objects are being destroyed anyway */
> if ( !no_rebalance )
> rb_erase(&obj->rb_tree_node,&pool->obj_rb_root[oid_hash(&old_oid)]);
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> tmem_free(obj, pool);
> }
>
> @@ -813,9 +797,8 @@ static struct tmem_object_root * obj_new(struct tmem_pool *pool, struct oid *oid
> obj->oid = *oidp;
> obj->pgp_count = 0;
> obj->last_client = TMEM_CLI_ID_NULL;
> - tmem_spin_lock(&obj->obj_spinlock);
> + spin_lock(&obj->obj_spinlock);
> obj_rb_insert(&pool->obj_rb_root[oid_hash(oidp)], obj);
> - obj->no_evict = 1;
> ASSERT_SPINLOCK(&obj->obj_spinlock);
> return obj;
> }
> @@ -835,7 +818,7 @@ static void pool_destroy_objs(struct tmem_pool *pool, bool_t selective, domid_t
> struct tmem_object_root *obj;
> int i;
>
> - tmem_write_lock(&pool->pool_rwlock);
> + write_lock(&pool->pool_rwlock);
> pool->is_dying = 1;
> for (i = 0; i < OBJ_HASH_BUCKETS; i++)
> {
> @@ -843,19 +826,18 @@ static void pool_destroy_objs(struct tmem_pool *pool, bool_t selective, domid_t
> while ( node != NULL )
> {
> obj = container_of(node, struct tmem_object_root, rb_tree_node);
> - tmem_spin_lock(&obj->obj_spinlock);
> + spin_lock(&obj->obj_spinlock);
> node = rb_next(node);
> - ASSERT(obj->no_evict == 0);
> if ( !selective )
> /* FIXME: should be obj,1 but walking/erasing rbtree is racy */
> obj_destroy(obj,0);
> else if ( obj->last_client == cli_id )
> obj_destroy(obj,0);
> else
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> }
> }
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> }
>
>
> @@ -1114,9 +1096,7 @@ static bool_t tmem_try_to_evict_pgp(struct tmem_page_descriptor *pgp, bool_t *ho
>
> if ( pool->is_dying )
> return 0;
> - if ( tmem_lock_all && !obj->no_evict )
> - return 1;
> - if ( tmem_spin_trylock(&obj->obj_spinlock) )
> + if ( spin_trylock(&obj->obj_spinlock) )
> {
> if ( tmem_dedup_enabled() )
> {
> @@ -1124,7 +1104,7 @@ static bool_t tmem_try_to_evict_pgp(struct tmem_page_descriptor *pgp, bool_t *ho
> if ( firstbyte == NOT_SHAREABLE )
> goto obj_unlock;
> ASSERT(firstbyte < 256);
> - if ( !tmem_write_trylock(&pcd_tree_rwlocks[firstbyte]) )
> + if ( !write_trylock(&pcd_tree_rwlocks[firstbyte]) )
> goto obj_unlock;
> if ( pgp->pcd->pgp_ref_count > 1 && !pgp->eviction_attempted )
> {
> @@ -1138,15 +1118,15 @@ static bool_t tmem_try_to_evict_pgp(struct tmem_page_descriptor *pgp, bool_t *ho
> }
> if ( obj->pgp_count > 1 )
> return 1;
> - if ( tmem_write_trylock(&pool->pool_rwlock) )
> + if ( write_trylock(&pool->pool_rwlock) )
> {
> *hold_pool_rwlock = 1;
> return 1;
> }
> pcd_unlock:
> - tmem_write_unlock(&pcd_tree_rwlocks[firstbyte]);
> + write_unlock(&pcd_tree_rwlocks[firstbyte]);
> obj_unlock:
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> }
> return 0;
> }
> @@ -1160,7 +1140,7 @@ static int tmem_evict(void)
> int ret = 0;
> bool_t hold_pool_rwlock = 0;
>
> - tmem_spin_lock(&eph_lists_spinlock);
> + spin_lock(&eph_lists_spinlock);
> if ( (client != NULL) && client_over_quota(client) &&
> !list_empty(&client->ephemeral_page_list) )
> {
> @@ -1182,7 +1162,6 @@ found:
> ASSERT(pgp != NULL);
> obj = pgp->us.obj;
> ASSERT(obj != NULL);
> - ASSERT(obj->no_evict == 0);
> ASSERT(obj->pool != NULL);
> pool = obj->pool;
>
> @@ -1201,13 +1180,13 @@ found:
> obj_free(obj,0);
> }
> else
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> if ( hold_pool_rwlock )
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> ret = 1;
>
> out:
> - tmem_spin_unlock(&eph_lists_spinlock);
> + spin_unlock(&eph_lists_spinlock);
> return ret;
> }
>
> @@ -1348,8 +1327,7 @@ done:
> /* successfully replaced data, clean up and return success */
> if ( is_shared(pool) )
> obj->last_client = client->cli_id;
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> return 1;
>
> failed_dup:
> @@ -1362,12 +1340,11 @@ cleanup:
> pgp_delete(pgpfound,0);
> if ( obj->pgp_count == 0 )
> {
> - tmem_write_lock(&pool->pool_rwlock);
> + write_lock(&pool->pool_rwlock);
> obj_free(obj,0);
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> } else {
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> }
> return ret;
> }
> @@ -1403,14 +1380,14 @@ static int do_tmem_put(struct tmem_pool *pool,
> /* no puts allowed into a frozen pool (except dup puts) */
> if ( client->frozen )
> return ret;
> - tmem_write_lock(&pool->pool_rwlock);
> + write_lock(&pool->pool_rwlock);
> if ( (obj = obj_new(pool,oidp)) == NULL )
> {
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> return -ENOMEM;
> }
> newobj = 1;
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> }
>
> /* When arrive here, we have a spinlocked obj for use */
> @@ -1463,29 +1440,28 @@ copy_uncompressed:
> insert_page:
> if ( !is_persistent(pool) )
> {
> - tmem_spin_lock(&eph_lists_spinlock);
> + spin_lock(&eph_lists_spinlock);
> list_add_tail(&pgp->global_eph_pages,
> &global_ephemeral_page_list);
> ++global_eph_count;
> list_add_tail(&pgp->us.client_eph_pages,
> &client->ephemeral_page_list);
> ++client->eph_count;
> - tmem_spin_unlock(&eph_lists_spinlock);
> + spin_unlock(&eph_lists_spinlock);
> }
> else
> { /* is_persistent */
> - tmem_spin_lock(&pers_lists_spinlock);
> + spin_lock(&pers_lists_spinlock);
> list_add_tail(&pgp->us.pool_pers_pages,
> &pool->persistent_page_list);
> - tmem_spin_unlock(&pers_lists_spinlock);
> + spin_unlock(&pers_lists_spinlock);
> }
>
> if ( is_shared(pool) )
> obj->last_client = client->cli_id;
> - obj->no_evict = 0;
>
> /* free the obj spinlock */
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> return 1;
>
> del_pgp_from_obj:
> @@ -1497,14 +1473,13 @@ free_pgp:
> unlock_obj:
> if ( newobj )
> {
> - tmem_write_lock(&pool->pool_rwlock);
> + write_lock(&pool->pool_rwlock);
> obj_free(obj, 0);
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> }
> else
> {
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> }
> return ret;
> }
> @@ -1531,8 +1506,7 @@ static int do_tmem_get(struct tmem_pool *pool, struct oid *oidp, uint32_t index,
> pgp = pgp_delete_from_obj(obj, index);
> if ( pgp == NULL )
> {
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> return 0;
> }
> ASSERT(pgp->size != -1);
> @@ -1555,31 +1529,29 @@ static int do_tmem_get(struct tmem_pool *pool, struct oid *oidp, uint32_t index,
> pgp_delete(pgp,0);
> if ( obj->pgp_count == 0 )
> {
> - tmem_write_lock(&pool->pool_rwlock);
> + write_lock(&pool->pool_rwlock);
> obj_free(obj,0);
> obj = NULL;
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> }
> } else {
> - tmem_spin_lock(&eph_lists_spinlock);
> + spin_lock(&eph_lists_spinlock);
> list_del(&pgp->global_eph_pages);
> list_add_tail(&pgp->global_eph_pages,&global_ephemeral_page_list);
> list_del(&pgp->us.client_eph_pages);
> list_add_tail(&pgp->us.client_eph_pages,&client->ephemeral_page_list);
> - tmem_spin_unlock(&eph_lists_spinlock);
> + spin_unlock(&eph_lists_spinlock);
> obj->last_client = tmem_get_cli_id_from_current();
> }
> }
> if ( obj != NULL )
> {
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> }
> return 1;
>
> out:
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> return rc;
> }
>
> @@ -1594,19 +1566,17 @@ static int do_tmem_flush_page(struct tmem_pool *pool, struct oid *oidp, uint32_t
> pgp = pgp_delete_from_obj(obj, index);
> if ( pgp == NULL )
> {
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> goto out;
> }
> pgp_delete(pgp,0);
> if ( obj->pgp_count == 0 )
> {
> - tmem_write_lock(&pool->pool_rwlock);
> + write_lock(&pool->pool_rwlock);
> obj_free(obj,0);
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
> } else {
> - obj->no_evict = 0;
> - tmem_spin_unlock(&obj->obj_spinlock);
> + spin_unlock(&obj->obj_spinlock);
> }
>
> out:
> @@ -1623,9 +1593,9 @@ static int do_tmem_flush_object(struct tmem_pool *pool, struct oid *oidp)
> obj = obj_find(pool,oidp);
> if ( obj == NULL )
> goto out;
> - tmem_write_lock(&pool->pool_rwlock);
> + write_lock(&pool->pool_rwlock);
> obj_destroy(obj,0);
> - tmem_write_unlock(&pool->pool_rwlock);
> + write_unlock(&pool->pool_rwlock);
>
> out:
> if ( pool->client->frozen )
> @@ -2032,7 +2002,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
> if ( bufsize < pagesize + sizeof(struct tmem_handle) )
> return -ENOMEM;
>
> - tmem_spin_lock(&pers_lists_spinlock);
> + spin_lock(&pers_lists_spinlock);
> if ( list_empty(&pool->persistent_page_list) )
> {
> ret = -1;
> @@ -2064,7 +2034,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
> ret = do_tmem_get(pool, &oid, pgp->index, 0, buf);
>
> out:
> - tmem_spin_unlock(&pers_lists_spinlock);
> + spin_unlock(&pers_lists_spinlock);
> return ret;
> }
>
> @@ -2080,7 +2050,7 @@ static int tmemc_save_get_next_inv(int cli_id, tmem_cli_va_param_t buf,
> return 0;
> if ( bufsize < sizeof(struct tmem_handle) )
> return 0;
> - tmem_spin_lock(&pers_lists_spinlock);
> + spin_lock(&pers_lists_spinlock);
> if ( list_empty(&client->persistent_invalidated_list) )
> goto out;
> if ( client->cur_pgp == NULL )
> @@ -2106,7 +2076,7 @@ static int tmemc_save_get_next_inv(int cli_id, tmem_cli_va_param_t buf,
> tmem_copy_to_client_buf(buf, &h, 1);
> ret = 1;
> out:
> - tmem_spin_unlock(&pers_lists_spinlock);
> + spin_unlock(&pers_lists_spinlock);
> return ret;
> }
>
> @@ -2222,7 +2192,7 @@ long do_tmem_op(tmem_cli_op_t uops)
> struct tmem_pool *pool = NULL;
> struct oid *oidp;
> int rc = 0;
> - bool_t tmem_write_lock_set = 0, tmem_read_lock_set = 0;
> + bool_t write_lock_set = 0, read_lock_set = 0;
>
> if ( !tmem_initialized )
> return -ENODEV;
> @@ -2230,47 +2200,30 @@ long do_tmem_op(tmem_cli_op_t uops)
> if ( !tmem_current_permitted() )
> return -EPERM;
>
> - if ( tmem_lock_all )
> - {
> - if ( tmem_lock_all > 1 )
> - spin_lock_irq(&tmem_spinlock);
> - else
> - spin_lock(&tmem_spinlock);
> - }
> -
> if ( client != NULL && tmem_client_is_dying(client) )
> - {
> - rc = -ENODEV;
> - if ( tmem_lock_all )
> - goto out;
> - simple_error:
> - return rc;
> - }
> + return -ENODEV;
>
> if ( unlikely(tmem_get_tmemop_from_client(&op, uops) != 0) )
> {
> tmem_client_err("tmem: can't get tmem struct from %s\n", tmem_client_str);
> - rc = -EFAULT;
> - if ( !tmem_lock_all )
> - goto simple_error;
> - goto out;
> + return -EFAULT;
> }
>
> if ( op.cmd == TMEM_CONTROL )
> {
> - tmem_write_lock(&tmem_rwlock);
> - tmem_write_lock_set = 1;
> + write_lock(&tmem_rwlock);
> + write_lock_set = 1;
> rc = do_tmem_control(&op);
> goto out;
> } else if ( op.cmd == TMEM_AUTH ) {
> - tmem_write_lock(&tmem_rwlock);
> - tmem_write_lock_set = 1;
> + write_lock(&tmem_rwlock);
> + write_lock_set = 1;
> rc = tmemc_shared_pool_auth(op.u.creat.arg1,op.u.creat.uuid[0],
> op.u.creat.uuid[1],op.u.creat.flags);
> goto out;
> } else if ( op.cmd == TMEM_RESTORE_NEW ) {
> - tmem_write_lock(&tmem_rwlock);
> - tmem_write_lock_set = 1;
> + write_lock(&tmem_rwlock);
> + write_lock_set = 1;
> rc = do_tmem_new_pool(op.u.creat.arg1, op.pool_id, op.u.creat.flags,
> op.u.creat.uuid[0], op.u.creat.uuid[1]);
> goto out;
> @@ -2279,8 +2232,8 @@ long do_tmem_op(tmem_cli_op_t uops)
> /* create per-client tmem structure dynamically on first use by client */
> if ( client == NULL )
> {
> - tmem_write_lock(&tmem_rwlock);
> - tmem_write_lock_set = 1;
> + write_lock(&tmem_rwlock);
> + write_lock_set = 1;
> if ( (client = client_create(tmem_get_cli_id_from_current())) == NULL )
> {
> tmem_client_err("tmem: can't create tmem structure for %s\n",
> @@ -2292,18 +2245,18 @@ long do_tmem_op(tmem_cli_op_t uops)
>
> if ( op.cmd == TMEM_NEW_POOL || op.cmd == TMEM_DESTROY_POOL )
> {
> - if ( !tmem_write_lock_set )
> + if ( !write_lock_set )
> {
> - tmem_write_lock(&tmem_rwlock);
> - tmem_write_lock_set = 1;
> + write_lock(&tmem_rwlock);
> + write_lock_set = 1;
> }
> }
> else
> {
> - if ( !tmem_write_lock_set )
> + if ( !write_lock_set )
> {
> - tmem_read_lock(&tmem_rwlock);
> - tmem_read_lock_set = 1;
> + read_lock(&tmem_rwlock);
> + read_lock_set = 1;
> }
> if ( ((uint32_t)op.pool_id >= MAX_POOLS_PER_DOMAIN) ||
> ((pool = client->pools[op.pool_id]) == NULL) )
> @@ -2346,21 +2299,12 @@ long do_tmem_op(tmem_cli_op_t uops)
> }
>
> out:
> - if ( tmem_lock_all )
> - {
> - if ( tmem_lock_all > 1 )
> - spin_unlock_irq(&tmem_spinlock);
> - else
> - spin_unlock(&tmem_spinlock);
> - } else {
> - if ( tmem_write_lock_set )
> - write_unlock(&tmem_rwlock);
> - else if ( tmem_read_lock_set )
> - read_unlock(&tmem_rwlock);
> - else
> - ASSERT(0);
> - }
> -
> + if ( write_lock_set )
> + write_unlock(&tmem_rwlock);
> + else if ( read_lock_set )
> + read_unlock(&tmem_rwlock);
> + else
> + ASSERT(0);
> return rc;
> }
>
> @@ -2378,38 +2322,26 @@ void tmem_destroy(void *v)
> return;
> }
>
> - if ( tmem_lock_all )
> - spin_lock(&tmem_spinlock);
> - else
> - write_lock(&tmem_rwlock);
> + write_lock(&tmem_rwlock);
>
> printk("tmem: flushing tmem pools for %s=%d\n",
> tmem_cli_id_str, client->cli_id);
> client_flush(client, 1);
>
> - if ( tmem_lock_all )
> - spin_unlock(&tmem_spinlock);
> - else
> - write_unlock(&tmem_rwlock);
> + write_unlock(&tmem_rwlock);
> }
>
> /* freezing all pools guarantees that no additional memory will be consumed */
> void tmem_freeze_all(unsigned char key)
> {
> static int freeze = 0;
> -
> - if ( tmem_lock_all )
> - spin_lock(&tmem_spinlock);
> - else
> - write_lock(&tmem_rwlock);
> +
> + write_lock(&tmem_rwlock);
>
> freeze = !freeze;
> tmemc_freeze_pools(TMEM_CLI_ID_NULL,freeze);
>
> - if ( tmem_lock_all )
> - spin_unlock(&tmem_spinlock);
> - else
> - write_unlock(&tmem_rwlock);
> + write_unlock(&tmem_rwlock);
> }
>
> #define MAX_EVICTS 10 /* should be variable or set via TMEMC_ ?? */
> @@ -2431,12 +2363,7 @@ void *tmem_relinquish_pages(unsigned int order, unsigned int memflags)
> }
>
> if ( tmem_called_from_tmem(memflags) )
> - {
> - if ( tmem_lock_all )
> - spin_lock(&tmem_spinlock);
> - else
> - read_lock(&tmem_rwlock);
> - }
> + read_lock(&tmem_rwlock);
>
> while ( (pfp = tmem_alloc_page(NULL,1)) == NULL )
> {
> @@ -2451,12 +2378,7 @@ void *tmem_relinquish_pages(unsigned int order, unsigned int memflags)
> }
>
> if ( tmem_called_from_tmem(memflags) )
> - {
> - if ( tmem_lock_all )
> - spin_unlock(&tmem_spinlock);
> - else
> - read_unlock(&tmem_rwlock);
> - }
> + read_unlock(&tmem_rwlock);
>
> return pfp;
> }
> @@ -2482,9 +2404,8 @@ static int __init init_tmem(void)
>
> if ( tmem_init() )
> {
> - printk("tmem: initialized comp=%d dedup=%d tze=%d global-lock=%d\n",
> - tmem_compression_enabled(), tmem_dedup_enabled(), tmem_tze_enabled(),
> - tmem_lock_all);
> + printk("tmem: initialized comp=%d dedup=%d tze=%d\n",
> + tmem_compression_enabled(), tmem_dedup_enabled(), tmem_tze_enabled());
> if ( tmem_dedup_enabled()&&tmem_compression_enabled()&&tmem_tze_enabled() )
> {
> tmem_tze_disable();
> diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> index fbd1acc..bc8e249 100644
> --- a/xen/common/tmem_xen.c
> +++ b/xen/common/tmem_xen.c
> @@ -29,9 +29,6 @@ boolean_param("tmem_tze", opt_tmem_tze);
> bool_t __read_mostly opt_tmem_shared_auth = 0;
> boolean_param("tmem_shared_auth", opt_tmem_shared_auth);
>
> -int __read_mostly opt_tmem_lock = 0;
> -integer_param("tmem_lock", opt_tmem_lock);
> -
> atomic_t freeable_page_count = ATOMIC_INIT(0);
>
> /* these are a concurrency bottleneck, could be percpu and dynamically
> diff --git a/xen/include/xen/tmem_xen.h b/xen/include/xen/tmem_xen.h
> index 3777543..7612a3f 100644
> --- a/xen/include/xen/tmem_xen.h
> +++ b/xen/include/xen/tmem_xen.h
> @@ -34,11 +34,6 @@ extern spinlock_t tmem_page_list_lock;
> extern unsigned long tmem_page_list_pages;
> extern atomic_t freeable_page_count;
>
> -extern spinlock_t tmem_lock;
> -extern spinlock_t tmem_spinlock;
> -extern rwlock_t tmem_rwlock;
> -
> -extern void tmem_copy_page(char *to, char*from);
> extern int tmem_init(void);
> #define tmem_hash hash_long
>
> @@ -77,8 +72,6 @@ static inline bool_t tmem_enabled(void)
> return opt_tmem;
> }
>
> -extern int opt_tmem_lock;
> -
> /*
> * Memory free page list management
> */
> @@ -182,7 +175,6 @@ static inline unsigned long tmem_free_mb(void)
> return (tmem_page_list_pages + total_free_pages()) >> (20 - PAGE_SHIFT);
> }
>
> -#define tmem_lock_all opt_tmem_lock
> #define tmem_called_from_tmem(_memflags) (_memflags & MEMF_tmem)
>
> /* "Client" (==domain) abstraction */
next prev parent reply other threads:[~2013-12-11 10:46 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-11 8:50 [PATCH v3 00/15] tmem: continue to cleanup tmem Bob Liu
2013-12-11 8:50 ` [PATCH v3 01/15] tmem: cleanup: drop unused sub command Bob Liu
2013-12-11 8:50 ` [PATCH v3 02/15] tmem: cleanup: drop some debug code Bob Liu
2013-12-11 8:50 ` [PATCH v3 03/15] tmem: cleanup: drop useless function 'tmem_copy_page' Bob Liu
2013-12-11 8:50 ` [PATCH v3 04/15] tmem: cleanup: drop useless parameters from put/get page Bob Liu
2013-12-11 8:50 ` [PATCH v3 05/15] tmem: cleanup: reorg function do_tmem_put() Bob Liu
2013-12-11 8:50 ` [PATCH v3 06/15] tmem: drop unneeded is_ephemeral() and is_private() Bob Liu
2013-12-11 8:50 ` [PATCH v3 07/15] tmem: cleanup: rm useless EXPORT/FORWARD define Bob Liu
2013-12-11 8:50 ` [PATCH v3 08/15] tmem: cleanup: drop runtime statistics Bob Liu
2013-12-11 8:50 ` [PATCH v3 09/15] tmem: cleanup: drop tmem_lock_all Bob Liu
2013-12-11 10:45 ` Andrew Cooper [this message]
2013-12-11 13:11 ` Bob Liu
2013-12-11 22:01 ` Konrad Rzeszutek Wilk
2013-12-11 8:50 ` [PATCH v3 10/15] tmem: cleanup: refactor the alloc/free path Bob Liu
2013-12-11 8:50 ` [PATCH v3 11/15] tmem: cleanup: __tmem_alloc_page: drop unneed parameters Bob Liu
2013-12-11 8:50 ` [PATCH v3 12/15] tmem: cleanup: drop useless functions from head file Bob Liu
2013-12-11 8:50 ` [PATCH v3 13/15] tmem: refator function tmem_ensure_avail_pages() Bob Liu
2013-12-11 8:50 ` [PATCH v3 14/15] tmem: cleanup: rename tmem_relinquish_npages() Bob Liu
2013-12-11 8:50 ` [PATCH v3 15/15] tmem: cleanup: rm unused tmem_freeze_all() Bob Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52A8425A.4070609@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=ian.campbell@citrix.com \
--cc=james.harper@bendigoit.com.au \
--cc=lliubbo@gmail.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.