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

[Lai Jiangshan]

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.

Thanks,
Lai