* Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote:
> On Thu, Feb 14, 2013 at 11:19:33AM -0500, Mathieu Desnoyers wrote:
> > I've had a report of someone running into issues with the RCU lock-free
> > hash table by embedding the struct cds_lfht_node into a packed structure
> > by mistake, thus not respecting alignment requirements stated in
> > urcu/rculfhash.h. Assertions on "replace" and "add" operations should
> > catch this, but I notice that we should add assertions on the
> > REMOVAL_OWNER_FLAG to cover all possible misalignments.
> >
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoy...@efficios.com>
>
> Makes sense to me!
Thanks for the feedback! Now merged into urcu master branch.
Mathieu
>
> Thanx, Paul
>
> > ---
> > diff --git a/rculfhash.c b/rculfhash.c
> > index 8c3d621..e0be7df 100644
> > --- a/rculfhash.c
> > +++ b/rculfhash.c
> > @@ -833,13 +833,16 @@ void _cds_lfht_gc_bucket(struct cds_lfht_node
> > *bucket, struct cds_lfht_node *nod
> >
> > assert(!is_bucket(bucket));
> > assert(!is_removed(bucket));
> > + assert(!is_removal_owner(bucket));
> > assert(!is_bucket(node));
> > assert(!is_removed(node));
> > + assert(!is_removal_owner(node));
> > for (;;) {
> > iter_prev = bucket;
> > /* We can always skip the bucket node initially */
> > iter = rcu_dereference(iter_prev->next);
> > assert(!is_removed(iter));
> > + assert(!is_removal_owner(iter));
> > assert(iter_prev->reverse_hash <= node->reverse_hash);
> > /*
> > * We should never be called with bucket (start of chain)
> > @@ -860,6 +863,7 @@ void _cds_lfht_gc_bucket(struct cds_lfht_node *bucket,
> > struct cds_lfht_node *nod
> > iter = next;
> > }
> > assert(!is_removed(iter));
> > + assert(!is_removal_owner(iter));
> > if (is_bucket(iter))
> > new_next = flag_bucket(clear_flag(next));
> > else
> > @@ -880,8 +884,10 @@ int _cds_lfht_replace(struct cds_lfht *ht, unsigned
> > long size,
> > return -ENOENT;
> >
> > assert(!is_removed(old_node));
> > + assert(!is_removal_owner(old_node));
> > assert(!is_bucket(old_node));
> > assert(!is_removed(new_node));
> > + assert(!is_removal_owner(new_node));
> > assert(!is_bucket(new_node));
> > assert(new_node != old_node);
> > for (;;) {
> > @@ -956,6 +962,7 @@ void _cds_lfht_add(struct cds_lfht *ht,
> >
> > assert(!is_bucket(node));
> > assert(!is_removed(node));
> > + assert(!is_removal_owner(node));
> > bucket = lookup_bucket(ht, size, hash);
> > for (;;) {
> > uint32_t chain_len = 0;
> > @@ -1016,7 +1023,9 @@ void _cds_lfht_add(struct cds_lfht *ht,
> > insert:
> > assert(node != clear_flag(iter));
> > assert(!is_removed(iter_prev));
> > + assert(!is_removal_owner(iter_prev));
> > assert(!is_removed(iter));
> > + assert(!is_removal_owner(iter));
> > assert(iter_prev != node);
> > if (!bucket_flag)
> > node->next = clear_flag(iter);
> > @@ -1036,6 +1045,7 @@ void _cds_lfht_add(struct cds_lfht *ht,
> >
> > gc_node:
> > assert(!is_removed(iter));
> > + assert(!is_removal_owner(iter));
> > if (is_bucket(iter))
> > new_next = flag_bucket(clear_flag(next));
> > else
> > @@ -1700,6 +1710,7 @@ int cds_lfht_delete_bucket(struct cds_lfht *ht)
> > if (!is_bucket(node))
> > return -EPERM;
> > assert(!is_removed(node));
> > + assert(!is_removal_owner(node));
> > } while (!is_end(node));
> > /*
> > * size accessed without rcu_dereference because hash table is
> >
> > --
> > Mathieu Desnoyers
> > EfficiOS Inc.
> > http://www.efficios.com
> >
> > _______________________________________________
> > rp mailing list
> > r...@svcs.cs.pdx.edu
> > http://svcs.cs.pdx.edu/mailman/listinfo/rp
> >
>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
