Re: [PATCH] docs: Move kref.txt to core-api/kref.rst

From: Thomas Hellstrom
Date: Fri May 10 2019 - 06:52:21 EST


On Fri, 2019-05-10 at 10:17 +1000, Tobin C. Harding wrote:
> kref.txt is already written using correct ReStructuredText
> format. This
> can be verified as follows
>
> make cleandocs
> make htmldocs 2> pre.stderr
> mv Documentation/kref.txt Documentation/core-api/kref.rst
> // Add 'kref' to core-api/index.rst
> make cleandocs
> make htmldocs 2> post.stderr
> diff pre.stderr post.stderr
>
> While doing the file move, fix the column width to be 72 characters
> wide
> as it is throughout the document. This is whitespace only. kref.txt
> is
> so cleanly written its a shame to have these few extra wide
> paragraphs.
>
> Signed-off-by: Tobin C. Harding <tobin@xxxxxxxxxx>
> ---
>
> I'm always hesitant to do docs patches that seem obvious - is there
> some reason that this was not done previously?

Speaking for the two kref.txt paragraphs, the width being too large is
simply an oversight from my side. I wasn't aware of the restriction.


Reviewed-by: Thomas Hellstrom <thellstrom@xxxxxxxxxx>

>
> I did this one in preparation for converting kobject.txt, my intent
> is
> to put kboject.rst in core-api/ also?
>
> I can split the whitespace change and the file rename into separate
> patches if you'd prefer.
>
> thanks,
> Tobin.
>
> Documentation/core-api/index.rst | 1 +
> Documentation/{kref.txt => core-api/kref.rst} | 24 +++++++++------
> ----
> Documentation/kobject.txt | 4 ++++
> 3 files changed, 17 insertions(+), 12 deletions(-)
> rename Documentation/{kref.txt => core-api/kref.rst} (93%)
>
> diff --git a/Documentation/core-api/index.rst b/Documentation/core-
> api/index.rst
> index ee1bb8983a88..1c95f0cdd239 100644
> --- a/Documentation/core-api/index.rst
> +++ b/Documentation/core-api/index.rst
> @@ -34,6 +34,7 @@ Core utilities
> timekeeping
> boot-time-mm
> memory-hotplug
> + kref
>
>
> Interfaces for kernel debugging
> diff --git a/Documentation/kref.txt b/Documentation/core-api/kref.rst
> similarity index 93%
> rename from Documentation/kref.txt
> rename to Documentation/core-api/kref.rst
> index 3af384156d7e..a2174dd09eb2 100644
> --- a/Documentation/kref.txt
> +++ b/Documentation/core-api/kref.rst
> @@ -230,8 +230,8 @@ of the free operations that could take a long
> time or might claim the
> same lock. Note that doing everything in the release routine is
> still
> preferred as it is a little neater.
>
> -The above example could also be optimized using
> kref_get_unless_zero() in
> -the following way::
> +The above example could also be optimized using
> kref_get_unless_zero()
> +in the following way::
>
> static struct my_data *get_entry()
> {
> @@ -261,11 +261,11 @@ the following way::
> kref_put(&entry->refcount, release_entry);
> }
>
> -Which is useful to remove the mutex lock around kref_put() in
> put_entry(), but
> -it's important that kref_get_unless_zero is enclosed in the same
> critical
> -section that finds the entry in the lookup table,
> -otherwise kref_get_unless_zero may reference already freed memory.
> -Note that it is illegal to use kref_get_unless_zero without checking
> its
> +Which is useful to remove the mutex lock around kref_put() in
> +put_entry(), but it's important that kref_get_unless_zero is
> enclosed in
> +the same critical section that finds the entry in the lookup table,
> +otherwise kref_get_unless_zero may reference already freed
> memory. Note
> +that it is illegal to use kref_get_unless_zero without checking its
> return value. If you are sure (by already having a valid pointer)
> that
> kref_get_unless_zero() will return true, then use kref_get()
> instead.
>
> @@ -312,8 +312,8 @@ locking for lookups in the above example::
> kref_put(&entry->refcount, release_entry_rcu);
> }
>
> -But note that the struct kref member needs to remain in valid memory
> for a
> -rcu grace period after release_entry_rcu was called. That can be
> accomplished
> -by using kfree_rcu(entry, rhead) as done above, or by calling
> synchronize_rcu()
> -before using kfree, but note that synchronize_rcu() may sleep for a
> -substantial amount of time.
> +But note that the struct kref member needs to remain in valid memory
> for
> +a rcu grace period after release_entry_rcu was called. That can be
> +accomplished by using kfree_rcu(entry, rhead) as done above, or by
> +calling synchronize_rcu() before using kfree, but note that
> +synchronize_rcu() may sleep for a substantial amount of time.
> diff --git a/Documentation/kobject.txt b/Documentation/kobject.txt
> index ff4c25098119..140030b4603b 100644
> --- a/Documentation/kobject.txt
> +++ b/Documentation/kobject.txt
> @@ -159,6 +159,10 @@ kernel at the same time, called surprisingly
> enough kobject_init_and_add()::
> int kobject_init_and_add(struct kobject *kobj, struct kobj_type
> *ktype,
> struct kobject *parent, const char
> *fmt, ...);
>
> +An error return from kobject_init_and_add() must be followed by a
> call to
> +kobject_put() since the 'init' part of this function is always
> called and the
> +error return is due to the 'add' part.
> +
> The arguments are the same as the individual kobject_init() and
> kobject_add() functions described above.
>