Message-ID: <a3db1384695bbaa051d93c18ac30175fb95165e3.camel@vmware.com>
Date:   Fri, 10 May 2019 10:50:57 +0000
From:   Thomas Hellstrom <thellstrom@...are.com>
To:     "corbet@....net" <corbet@....net>,
        "tobin@...nel.org" <tobin@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "minyard@....org" <minyard@....org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH] docs: Move kref.txt to core-api/kref.rst

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@...nel.org>
> ---
> 
> 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@...are.com>

> 
> 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.
>  

Powered by blists - more mailing lists