[ltt-dev] [PATCH 03/10 round10] ht->t.size is no required larger than ht->min_table_size now

[compudj@krystal.dyndns.org (Mathieu Desnoyers)]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> On 11/22/2011 06:06 PM, Mathieu Desnoyers wrote:
> > * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> >> Thanks to the wrappers of bucket table alloc/free.
> > 
> > Sorry, I don't understand this change, and I don't see how the patch
> > below matches the patch title. This patch does not touch ht->t.size at
> > all, and seems to use MIN_TABLE_SIZE to replace ht->min_table_size, but
> > does not explain why this is possible.
> > 
> 
> Original code always ensure ht->t.size >= ht->min_table_size.
> It is not good, ht->min_table_size should be used for allocation,
> not for the bucket size in used.
> 
> Thanks,
> Lai
> 
> Add:
> Original code don't do special alloc/free when ht->t.size < ht->min_table_size
> in init_table()/fini_table(), so we have to force ht->t.size >= ht->min_table_size.
> 
> New code use the wrappers, they handle the special cases.

OK. So I think this clearer description of what the patch does is
required for the next round.

Thanks!

Mathieu

> 
> 
> 
> > More below,
> > 
> >>
> >> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> >> ---
> >>  rculfhash.c            |   19 ++++++++++---------
> >>  tests/test_urcu_hash.c |    2 +-
> >>  2 files changed, 11 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/rculfhash.c b/rculfhash.c
> >> index c7f993f..a72e1a5 100644
> >> --- a/rculfhash.c
> >> +++ b/rculfhash.c
> >> @@ -186,7 +186,8 @@
> >>  /*
> >>   * Define the minimum table size.
> >>   */
> >> -#define MIN_TABLE_SIZE			1
> >> +#define MIN_TABLE_ORDER			0
> >> +#define MIN_TABLE_SIZE			(1UL << MIN_TABLE_ORDER)
> >>  
> >>  #if (CAA_BITS_PER_LONG == 32)
> >>  #define MAX_TABLE_ORDER			32
> >> @@ -1141,7 +1142,7 @@ void init_table_populate_partition(struct cds_lfht *ht, unsigned long i,
> >>  {
> >>  	unsigned long j, size = 1UL << (i - 1);
> >>  
> >> -	assert(i > ht->min_alloc_order);
> >> +	assert(i > MIN_TABLE_ORDER);
> > 
> > Here, all this does is change the min_alloc_order
> > 
> >>  	ht->cds_lfht_rcu_read_lock();
> >>  	for (j = start + size; j < size + start + len; j++) {
> >>  		struct cds_lfht_node *new_node = bucket_at(ht, j);
> >> @@ -1177,7 +1178,7 @@ void init_table(struct cds_lfht *ht,
> >>  
> >>  	dbg_printf("init table: first_order %lu last_order %lu\n",
> >>  		   first_order, last_order);
> >> -	assert(first_order > ht->min_alloc_order);
> >> +	assert(first_order > MIN_TABLE_ORDER);
> >>  	for (i = first_order; i <= last_order; i++) {
> >>  		unsigned long len;
> >>  
> >> @@ -1239,7 +1240,7 @@ void remove_table_partition(struct cds_lfht *ht, unsigned long i,
> >>  {
> >>  	unsigned long j, size = 1UL << (i - 1);
> >>  
> >> -	assert(i > ht->min_alloc_order);
> >> +	assert(i > MIN_TABLE_ORDER);
> >>  	ht->cds_lfht_rcu_read_lock();
> >>  	for (j = size + start; j < size + start + len; j++) {
> >>  		struct cds_lfht_node *fini_node = bucket_at(ht, j);
> >> @@ -1276,7 +1277,7 @@ void fini_table(struct cds_lfht *ht,
> >>  
> >>  	dbg_printf("fini table: first_order %lu last_order %lu\n",
> >>  		   first_order, last_order);
> >> -	assert(first_order > ht->min_alloc_order);
> >> +	assert(first_order > MIN_TABLE_ORDER);
> >>  	for (i = last_order; i >= first_order; i--) {
> >>  		unsigned long len;
> >>  
> >> @@ -1376,7 +1377,7 @@ struct cds_lfht *_cds_lfht_new(unsigned long init_size,
> >>  	if (!init_size || (init_size & (init_size - 1)))
> >>  		return NULL;
> >>  	min_alloc_size = max(min_alloc_size, MIN_TABLE_SIZE);
> >> -	init_size = max(init_size, min_alloc_size);
> >> +	init_size = max(init_size, MIN_TABLE_SIZE);
> >>  	ht = calloc(1, sizeof(struct cds_lfht));
> >>  	assert(ht);
> >>  	ht->flags = flags;
> >> @@ -1706,7 +1707,7 @@ void _do_cds_lfht_shrink(struct cds_lfht *ht,
> >>  {
> >>  	unsigned long old_order, new_order;
> >>  
> >> -	new_size = max(new_size, ht->min_alloc_size);
> >> +	new_size = max(new_size, MIN_TABLE_SIZE);
> >>  	old_order = get_count_order_ulong(old_size);
> >>  	new_order = get_count_order_ulong(new_size);
> >>  	dbg_printf("resize from %lu (order %lu) to %lu (order %lu) buckets\n",
> >> @@ -1754,7 +1755,7 @@ static
> >>  void resize_target_update_count(struct cds_lfht *ht,
> >>  				unsigned long count)
> >>  {
> >> -	count = max(count, ht->min_alloc_size);
> >> +	count = max(count, MIN_TABLE_SIZE);
> >>  	uatomic_set(&ht->t.resize_target, count);
> >>  }
> >>  
> >> @@ -1829,7 +1830,7 @@ void cds_lfht_resize_lazy_count(struct cds_lfht *ht, unsigned long size,
> >>  {
> >>  	if (!(ht->flags & CDS_LFHT_AUTO_RESIZE))
> >>  		return;
> >> -	count = max(count, ht->min_alloc_size);
> >> +	count = max(count, MIN_TABLE_SIZE);
> >>  	if (count == size)
> >>  		return;		/* Already the right size, no resize needed */
> >>  	if (count > size) {	/* lazy grow */
> >> diff --git a/tests/test_urcu_hash.c b/tests/test_urcu_hash.c
> >> index 509767c..b9e3e81 100644
> >> --- a/tests/test_urcu_hash.c
> >> +++ b/tests/test_urcu_hash.c
> >> @@ -896,7 +896,7 @@ int main(int argc, char **argv)
> >>  		return -1;
> >>  	}
> >>  
> >> -	if (min_hash_alloc_size && min_hash_alloc_size * (min_hash_alloc_size - 1)) {
> >> +	if (min_hash_alloc_size && min_hash_alloc_size & (min_hash_alloc_size - 1)) {
> > 
> > This seems to fix a bug in the test program, but it's not documented.
> > Maybe this should go in as a separate patch ?
> > 
> > Thanks,
> > 
> > Mathieu
> > 
> >>  		printf("Error: Min hash alloc size %lu is not a power of 2.\n",
> >>  			min_hash_alloc_size);
> >>  		return -1;
> >> -- 
> >> 1.7.4.4
> >>
> > 
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

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