[lttng-dev] [rp] [PATCH urcu] rculfhash: add assertions on node alignment

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck at 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.desnoyers at 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
> > rp at svcs.cs.pdx.edu
> > http://svcs.cs.pdx.edu/mailman/listinfo/rp
> > 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com