All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Hellstrom <thellstrom@vmware.com>
To: "corbet@lwn.net" <corbet@lwn.net>, "tobin@kernel.org" <tobin@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"minyard@acm.org" <minyard@acm.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: Re: [PATCH] docs: Move kref.txt to core-api/kref.rst
Date: Fri, 10 May 2019 10:50:57 +0000	[thread overview]
Message-ID: <a3db1384695bbaa051d93c18ac30175fb95165e3.camel@vmware.com> (raw)
In-Reply-To: <20190510001747.8767-1-tobin@kernel.org>

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@kernel.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@vmware.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.
>  

  parent reply	other threads:[~2019-05-10 10:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-10  0:17 [PATCH] docs: Move kref.txt to core-api/kref.rst Tobin C. Harding
2019-05-10  0:41 ` Tobin C. Harding
2019-05-10 14:17   ` Jonathan Corbet
2019-05-10 10:50 ` Thomas Hellstrom [this message]
2019-05-10 20:45   ` Tobin C. Harding
2019-05-15 16:56     ` [TRIVIA] " Jonathan Corbet
2019-05-16 23:19       ` Tobin C. Harding
2019-05-20 18:23         ` Jonathan Corbet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a3db1384695bbaa051d93c18ac30175fb95165e3.camel@vmware.com \
    --to=thellstrom@vmware.com \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minyard@acm.org \
    --cc=tobin@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.