Re: [PATCH] Doc/RCU: fix pseudocode in rcuref.txt

Paul E. McKenney
2008-09-10T15:40:32Z

On Wed, Sep 10, 2008 at 11:01:07AM +0800, Lai Jiangshan wrote:
>
> atomic_inc_not_zero(v) return 0 if *v = 0.
> use spin_lock instead of write_lock for update lock.

Good catch, Jiangshan, thank you!

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/Documentation/RCU/rcuref.txt b/Documentation/RCU/rcuref.txt
> index 451de2a..4202ad0 100644
> --- a/Documentation/RCU/rcuref.txt
> +++ b/Documentation/RCU/rcuref.txt
> @@ -29,9 +29,9 @@ release_referenced() delete()
> }
>
> If this list/array is made lock free using RCU as in changing the
> -write_lock() in add() and delete() to spin_lock and changing read_lock
> -in search_and_reference to rcu_read_lock(), the atomic_get in
> -search_and_reference could potentially hold reference to an element which
> +write_lock() in add() and delete() to spin_lock() and changing read_lock()
> +in search_and_reference() to rcu_read_lock(), the atomic_inc() in
> +search_and_reference() could potentially hold reference to an element which
> has already been deleted from the list/array. Use atomic_inc_not_zero()
> in this scenario as follows:
>
> @@ -40,20 +40,20 @@ add() search_and_reference()
> { {
> alloc_object rcu_read_lock();
> ... search_for_element
> - atomic_set(&el->rc, 1); if (atomic_inc_not_zero(&el->rc)) {
> - write_lock(&list_lock); rcu_read_unlock();
> + atomic_set(&el->rc, 1); if (!atomic_inc_not_zero(&el->rc)) {
> + spin_lock(&list_lock); rcu_read_unlock();
> return FAIL;
> add_element }
> ... ...
> - write_unlock(&list_lock); rcu_read_unlock();
> + spin_unlock(&list_lock); rcu_read_unlock();
> } }
> 3. 4.
> release_referenced() delete()
> { {
> - ... write_lock(&list_lock);
> + ... spin_lock(&list_lock);
> if (atomic_dec_and_test(&el->rc)) ...
> call_rcu(&el->head, el_free); delete_element
> - ... write_unlock(&list_lock);
> + ... spin_unlock(&list_lock);
> } ...
> if (atomic_dec_and_test(&el->rc))
> call_rcu(&el->head, el_free);
>
>