[ltt-dev] [PATCH 02/10 round10] proper wrapper for bucket table alloc and free

[mathieu.desnoyers@efficios.com (Mathieu Desnoyers)]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> On 11/22/2011 06:01 PM, Mathieu Desnoyers wrote:
> > * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> >> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> >> ---
> >>  rculfhash.c |   78 +++++++++++++++++++++++++++++++++++-----------------------
> >>  1 files changed, 47 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/rculfhash.c b/rculfhash.c
> >> index b0c7de5..c7f993f 100644
> >> --- a/rculfhash.c
> >> +++ b/rculfhash.c
> >> @@ -755,6 +755,36 @@ unsigned long _uatomic_xchg_monotonic_increase(unsigned long *ptr,
> >>  	return old2;
> >>  }
> >>  
> >> +static
> >> +void cds_lfht_alloc_bucket_table(struct cds_lfht *ht, unsigned long order)
> >> +{
> >> +	if (order == 0) {
> >> +		ht->t.tbl[0] = calloc(ht->min_alloc_size,
> >> +			sizeof(struct cds_lfht_node));
> >> +		assert(ht->t.tbl[0]);
> >> +	} else if (order > ht->min_alloc_order) {
> >> +		ht->t.tbl[order] = calloc(1UL << (order -1),
> >> +			sizeof(struct cds_lfht_node));
> >> +		assert(ht->t.tbl[order]);
> >> +	}
> >> +	/* Nothing to do for 0 < order && order <= ht->min_alloc_order */
> >> +}
> >> +
> >> +/*
> >> + * cds_lfht_free_bucket_table() should be called with decreasing order.
> >> + * When cds_lfht_free_bucket_table(0) is called, it means the whole
> >> + * lfht is destroyed.
> >> + */
> >> +static
> >> +void cds_lfht_free_bucket_table(struct cds_lfht *ht, unsigned long order)
> >> +{
> >> +	if (order == 0)
> >> +		poison_free(ht->t.tbl[0]);
> >> +	else if (order > ht->min_alloc_order)
> >> +		poison_free(ht->t.tbl[order]);
> >> +	/* Nothing to do for 0 < order && order <= ht->min_alloc_order */
> >> +}
> >> +
> >>  static inline
> >>  struct cds_lfht_node *bucket_at(struct cds_lfht *ht, unsigned long index)
> >>  {
> >> @@ -1158,8 +1188,7 @@ void init_table(struct cds_lfht *ht,
> >>  		if (CMM_LOAD_SHARED(ht->t.resize_target) < (1UL << i))
> >>  			break;
> >>  
> >> -		ht->t.tbl[i] = calloc(1, len * sizeof(struct cds_lfht_node));
> >> -		assert(ht->t.tbl[i]);
> >> +		cds_lfht_alloc_bucket_table(ht, i);
> >>  
> >>  		/*
> >>  		 * Set all bucket nodes reverse hash values for a level and
> >> @@ -1243,7 +1272,7 @@ void fini_table(struct cds_lfht *ht,
> >>  		unsigned long first_order, unsigned long last_order)
> >>  {
> >>  	long i;
> >> -	void *free_by_rcu = NULL;
> >> +	unsigned long free_by_rcu_order = 0;
> >>  
> >>  	dbg_printf("fini table: first_order %lu last_order %lu\n",
> >>  		   first_order, last_order);
> >> @@ -1268,8 +1297,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);
> >> +		if (free_by_rcu_order)
> >> +			cds_lfht_free_bucket_table(ht, free_by_rcu_order);
> >>  
> >>  		/*
> >>  		 * Set "removed" flag in bucket nodes about to be removed.
> >> @@ -1279,16 +1308,16 @@ void fini_table(struct cds_lfht *ht,
> >>  		 */
> >>  		remove_table(ht, i, len);
> >>  
> >> -		free_by_rcu = ht->t.tbl[i];
> >> +		free_by_rcu_order = i;
> > 
> > Hrm, so for i = 0 (order 0), we never call free() ?
> > 
> 
> You can find the "assert" before the loop, the i is not possible 0.
> when ds_lfht_free_bucket_table(0), it means the whole hash table is
> being destroyed, it is not happened now.

I see, sounds good!

Thanks,

Mathieu

> 
> Thanks,
> Lai

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