[ltt-dev] [PATCH 3/9] rculfhash: make struct rcu_level allied

[Mathieu Desnoyers]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> reduce memory fragment
> add a slowpath overhead
> 
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

Hrm, I see what you want to do here: given we already have
synchronize_rcu calls in the fini code, you use them to guarantee
quiescence of the previous iteration instead of using call_rcu. I would
argue that at this point in the development process, I would rather
prefer to keep allocation/delayed free as decoupled as possible from the
algorithm. So I'll keep this optimisation in mind, but for now I think I
will keep the call_rcu scheme to deal with all memory free.  This makes
it more easy to modify the code when the allocation/delayed free
concerns are all encapsulated through use of call_rcu.

Moreover, this patch really is an optimisation, so it should be backed
by benchmarks that shows if there are measurable performance
improvements.

Thanks!

Mathieu

> ---
>  rculfhash.c |   22 ++++++++++------------
>  1 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/rculfhash.c b/rculfhash.c
> index e379c71..49f3637 100644
> --- a/rculfhash.c
> +++ b/rculfhash.c
> @@ -209,7 +209,6 @@ struct ht_items_count {
>  } __attribute__((aligned(CAA_CACHE_LINE_SIZE)));
>  
>  struct rcu_level {
> -	struct rcu_head head;
>  	struct _cds_lfht_node nodes[0];
>  };
>  
> @@ -712,14 +711,6 @@ unsigned long _uatomic_max(unsigned long *ptr, unsigned long v)
>  	return v;
>  }
>  
> -static
> -void cds_lfht_free_level(struct rcu_head *head)
> -{
> -	struct rcu_level *l =
> -		caa_container_of(head, struct rcu_level, head);
> -	poison_free(l);
> -}
> -
>  /*
>   * Remove all logically deleted nodes from a bucket up to a certain node key.
>   */
> @@ -1131,8 +1122,7 @@ void init_table(struct cds_lfht *ht,
>  		if (CMM_LOAD_SHARED(ht->t.resize_target) < (!i ? 1 : (1UL << i)))
>  			break;
>  
> -		ht->t.tbl[i] = calloc(1, sizeof(struct rcu_level)
> -				+ (len * sizeof(struct _cds_lfht_node)));
> +		ht->t.tbl[i] = calloc(1, len * sizeof(struct _cds_lfht_node));
>  		assert(ht->t.tbl[i]);
>  
>  		/*
> @@ -1218,6 +1208,7 @@ void fini_table(struct cds_lfht *ht,
>  		unsigned long first_order, unsigned long len_order)
>  {
>  	long i, end_order;
> +	void *free_by_rcu = NULL;
>  
>  	dbg_printf("fini table: first_order %lu end_order %lu\n",
>  		   first_order, first_order + len_order);
> @@ -1243,6 +1234,8 @@ void fini_table(struct cds_lfht *ht,
>  		 * return a logically removed node as insert position.
>  		 */
>  		ht->cds_lfht_synchronize_rcu();
> +		if (free_by_rcu)
> +			free(free_by_rcu);
>  
>  		/*
>  		 * Set "removed" flag in dummy nodes about to be removed.
> @@ -1252,12 +1245,17 @@ void fini_table(struct cds_lfht *ht,
>  		 */
>  		remove_table(ht, i, len);
>  
> -		ht->cds_lfht_call_rcu(&ht->t.tbl[i]->head, cds_lfht_free_level);
> +		free_by_rcu = ht->t.tbl[i];
>  
>  		dbg_printf("fini new size: %lu\n", 1UL << i);
>  		if (CMM_LOAD_SHARED(ht->in_progress_destroy))
>  			break;
>  	}
> +
> +	if (free_by_rcu) {
> +		ht->cds_lfht_synchronize_rcu();
> +		free(free_by_rcu);
> +	}
>  }
>  
>  struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com