Hi Lai,
* Lai Jiangshan ([email protected]) wrote:
> Signed-off-by: Lai Jiangshan <[email protected]>
> ---
> rculfhash.c | 41 ++++++++++++++++-------------------------
> 1 files changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/rculfhash.c b/rculfhash.c
> index 88079f9..0163a1b 100644
> --- a/rculfhash.c
> +++ b/rculfhash.c
> @@ -316,13 +316,6 @@ struct partition_resize_work {
> unsigned long start, unsigned long len);
> };
>
> -static
> -void _cds_lfht_add(struct cds_lfht *ht,
> - unsigned long size,
> - struct cds_lfht_node *node,
> - struct cds_lfht_iter *unique_ret,
> - int bucket);
> -
> /*
> * Algorithm to reverse bits in a word by lookup table, extended to
> * 64-bit words.
> @@ -728,6 +721,14 @@ struct cds_lfht_node *flag_bucket(struct cds_lfht_node
> *node)
> }
>
> static
> +struct cds_lfht_node *flag_keep_bucket(struct cds_lfht_node *node,
> + unsigned long bucket)
> +{
> + return (struct cds_lfht_node *) (((unsigned long) node) |
> + (bucket & BUCKET_FLAG));
> +}
Hrm. The name is "flag_keep_bucket", but we don't do a clear_flag on
"node" within this helper. So we end up having a behavior that differs
from the helper semantic. I understand that the code below that uses
flag_keep_bucket performs the clear_flag when needed, but I think it
would be good to rename flag_keep_bucket so anyone reviewing the code
would not be mislead by the name: I initially thought that
flag_keep_bucket kept only the "BUCKET" flag and cleared all other flags
from node, which is not the case.
In fact, what it does is to keep the node flag as-is, and OR the BUCKET
flag to node if it is set in the "bucket" argument.
So maybe "bucket_flag_or" would be more appropriate ?
> +
> +static
> struct cds_lfht_node *get_end(void)
> {
> return (struct cds_lfht_node *) END_VALUE;
> @@ -815,10 +816,7 @@ void _cds_lfht_gc_bucket(struct cds_lfht_node *bucket,
> struct cds_lfht_node *nod
> iter = next;
> }
> assert(!is_removed(iter));
> - if (is_bucket(iter))
> - new_next = flag_bucket(clear_flag(next));
> - else
> - new_next = clear_flag(next);
> + new_next = flag_keep_bucket(clear_flag(next), (unsigned
> long)iter);
Also please use "(unsigned long) iter" (with a space) to keep the coding
style of the file.
Thanks,
Mathieu
> (void) uatomic_cmpxchg(&iter_prev->next, iter, new_next);
> }
> return;
> @@ -884,13 +882,15 @@ int _cds_lfht_replace(struct cds_lfht *ht, unsigned
> long size,
> /*
> * A non-NULL unique_ret pointer uses the "add unique" (or uniquify) add
> * mode. A NULL unique_ret allows creation of duplicate keys.
> + *
> + * @bucket_flag should be 0 or BUCKET_FLAG
> */
> static
> void _cds_lfht_add(struct cds_lfht *ht,
> unsigned long size,
> struct cds_lfht_node *node,
> struct cds_lfht_iter *unique_ret,
> - int bucket_flag)
> + unsigned long bucket_flag)
> {
> struct cds_lfht_node *iter_prev, *iter, *next, *new_node, *new_next,
> *return_node;
> @@ -960,14 +960,8 @@ void _cds_lfht_add(struct cds_lfht *ht,
> assert(!is_removed(iter_prev));
> assert(!is_removed(iter));
> assert(iter_prev != node);
> - if (!bucket_flag)
> - node->next = clear_flag(iter);
> - else
> - node->next = flag_bucket(clear_flag(iter));
> - if (is_bucket(iter))
> - new_node = flag_bucket(node);
> - else
> - new_node = node;
> + node->next = flag_keep_bucket(clear_flag(iter), bucket_flag);
> + new_node = flag_keep_bucket(node, (unsigned long)iter);
> if (uatomic_cmpxchg(&iter_prev->next, iter,
> new_node) != iter) {
> continue; /* retry */
> @@ -978,10 +972,7 @@ void _cds_lfht_add(struct cds_lfht *ht,
>
> gc_node:
> assert(!is_removed(iter));
> - if (is_bucket(iter))
> - new_next = flag_bucket(clear_flag(next));
> - else
> - new_next = clear_flag(next);
> + new_next = flag_keep_bucket(clear_flag(next), (unsigned
> long)iter);
> (void) uatomic_cmpxchg(&iter_prev->next, iter, new_next);
> /* retry */
> }
> @@ -1113,7 +1104,7 @@ void init_table_populate_partition(struct cds_lfht *ht,
> unsigned long i,
> new_node->reverse_hash =
> bit_reverse_ulong((1UL << (i - 1)) + j);
> _cds_lfht_add(ht, 1UL << (i - 1),
> - new_node, NULL, 1);
> + new_node, NULL, BUCKET_FLAG);
> }
> ht->cds_lfht_rcu_read_unlock();
> }
> --
> 1.7.4.4
>
--
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
