Re: [PATCH] doc/rcuref: Document real world examples in kernel

From: Jann Horn
Date: Fri Mar 29 2019 - 00:06:51 EST


On Fri, Mar 29, 2019 at 3:40 AM Joel Fernandes (Google)
<joel@xxxxxxxxxxxxxxxxx> wrote:
> Document similar real world examples in the kernel corresponding to the
> second and third code snippets. Also correct an issue in
> release_referenced() in the code snippet example.
>
> Cc: oleg@xxxxxxxxxx
> Cc: jannh@xxxxxxxxxx
> Signed-off-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>
>
> ---
> Documentation/RCU/rcuref.txt | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/RCU/rcuref.txt b/Documentation/RCU/rcuref.txt
> index 613033ff2b9b..e5f4a49f886a 100644
> --- a/Documentation/RCU/rcuref.txt
> +++ b/Documentation/RCU/rcuref.txt
> @@ -28,7 +28,8 @@ add() search_and_reference()
> release_referenced() delete()
> { {
> ... write_lock(&list_lock);
> - atomic_dec(&el->rc, relfunc) ...
> + if(atomic_dec_and_test(&el->rc)) ...
> + kfree(el);
> ... remove_element
> } write_unlock(&list_lock);
> ...
> @@ -114,6 +115,11 @@ element can therefore safely be freed. This in turn guarantees that if
> any reader finds the element, that reader may safely acquire a reference
> without checking the value of the reference counter.
>
> +The other advantage of the last pattern is, if there are several calls to
> +search_and_reference() in parallel to the delete(), then all of those will
> +succeed in obtaining a reference to the object if the object could be found in
> +the list before it was deleted in delete().

Isn't this the same as what the previous paragraph said? "if
any reader finds the element, that reader may safely acquire a reference
without checking the value of the reference counter".

> In cases where delete() can sleep, synchronize_rcu() can be called from
> delete(), so that el_free() can be subsumed into delete as follows:
>
> @@ -130,3 +136,7 @@ delete()
> kfree(el);
> ...
> }
> +
> +As additional examples in the kernel, This last pattern is also followed by

nit: s/, This/, this/