* Lai Jiangshan ([email protected]) wrote: > On 11/22/2011 06:06 PM, Mathieu Desnoyers wrote: > > * Lai Jiangshan ([email protected]) 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 <[email protected]> > >> --- > >> 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 > [email protected] > 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 _______________________________________________ ltt-dev mailing list [email protected] http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
