[PATCH] idr: Document calling context for IDA APIs mustn't use locks

[PATCH] idr: Document calling context for IDA APIs mustn't use locks

[Stephen Boyd]

The documentation for these functions indicates that callers don't need
to hold a lock while calling them, but that documentation is only in one
place under "IDA Usage". Let's state the same information on each IDA
function so that it's clear what the calling context requires.
Furthermore, let's document ida_simple_get() with the same information
so that callers know how this API works.

Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Tri Vo <trong@android.com>
Cc: linux-doc@vger.kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

See Greg's comment[1] for the reason why this patch is created.

[1] https://lkml.kernel.org/r/20190730064657.GA1213@kroah.com

 include/linux/idr.h | 23 ++++++++++++++++++++---
 lib/idr.c           |  9 ++++++---
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/include/linux/idr.h b/include/linux/idr.h
index 4ec8986e5dfb..b591ecbba3f4 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -263,7 +263,8 @@ void ida_destroy(struct ida *ida);
  *
  * Allocate an ID between 0 and %INT_MAX, inclusive.
  *
- * Context: Any context.
+ * Context: Any context. It is safe to call this function without
+ * synchronisation in your code.
  * Return: The allocated ID, or %-ENOMEM if memory could not be allocated,
  * or %-ENOSPC if there are no free IDs.
  */
@@ -280,7 +281,8 @@ static inline int ida_alloc(struct ida *ida, gfp_t gfp)
  *
  * Allocate an ID between @min and %INT_MAX, inclusive.
  *
- * Context: Any context.
+ * Context: Any context. It is safe to call this function without
+ * synchronisation in your code.
  * Return: The allocated ID, or %-ENOMEM if memory could not be allocated,
  * or %-ENOSPC if there are no free IDs.
  */
@@ -297,7 +299,8 @@ static inline int ida_alloc_min(struct ida *ida, unsigned int min, gfp_t gfp)
  *
  * Allocate an ID between 0 and @max, inclusive.
  *
- * Context: Any context.
+ * Context: Any context. It is safe to call this function without
+ * synchronisation in your code.
  * Return: The allocated ID, or %-ENOMEM if memory could not be allocated,
  * or %-ENOSPC if there are no free IDs.
  */
@@ -311,6 +314,20 @@ static inline void ida_init(struct ida *ida)
 	xa_init_flags(&ida->xa, IDA_INIT_FLAGS);
 }
 
+/**
+ * ida_simple_get() - Allocate an unused ID between (start, end].
+ * @ida: IDA handle.
+ * @start: ID to start from (inclusive)
+ * @end: ID to stop at (exclusive). Use 0 to indicate %INT_MAX.
+ * @gfp: Memory allocation flags.
+ *
+ * Allocate an ID between (start, end].
+ *
+ * Context: Any context. It is safe to call this function without
+ * synchronisation in your code.
+ * Return: The allocated ID, or %-ENOMEM if memory could not be allocated,
+ * or %-ENOSPC if there are no free IDs.
+ */
 #define ida_simple_get(ida, start, end, gfp)	\
 			ida_alloc_range(ida, start, (end) - 1, gfp)
 #define ida_simple_remove(ida, id)	ida_free(ida, id)
diff --git a/lib/idr.c b/lib/idr.c
index 66a374892482..e8a5f47c0c78 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -381,7 +381,8 @@ EXPORT_SYMBOL(idr_replace);
  * Allocate an ID between @min and @max, inclusive.  The allocated ID will
  * not exceed %INT_MAX, even if @max is larger.
  *
- * Context: Any context.
+ * Context: Any context. It is safe to call this function without
+ * synchronisation in your code.
  * Return: The allocated ID, or %-ENOMEM if memory could not be allocated,
  * or %-ENOSPC if there are no free IDs.
  */
@@ -488,7 +489,8 @@ EXPORT_SYMBOL(ida_alloc_range);
  * @ida: IDA handle.
  * @id: Previously allocated ID.
  *
- * Context: Any context.
+ * Context: Any context. It is safe to call this function without
+ * synchronisation in your code.
  */
 void ida_free(struct ida *ida, unsigned int id)
 {
@@ -540,7 +542,8 @@ EXPORT_SYMBOL(ida_free);
  * or freed.  If the IDA is already empty, there is no need to call this
  * function.
  *
- * Context: Any context.
+ * Context: Any context. It is safe to call this function without
+ * synchronisation in your code.
  */
 void ida_destroy(struct ida *ida)
 {
-- 
Sent by a computer through tubes

Re: [PATCH] idr: Document calling context for IDA APIs mustn't use locks

[Matthew Wilcox]

On Tue, Jul 30, 2019 at 02:07:52PM -0700, Stephen Boyd wrote:
> The documentation for these functions indicates that callers don't need
> to hold a lock while calling them, but that documentation is only in one
> place under "IDA Usage". Let's state the same information on each IDA
> function so that it's clear what the calling context requires.
> Furthermore, let's document ida_simple_get() with the same information
> so that callers know how this API works.

I don't want people to use ida_simple_get() any more.  Use ida_alloc()
instead.

> - * Context: Any context.
> + * Context: Any context. It is safe to call this function without
> + * synchronisation in your code.

I prefer "without locking" to "without synchronisation" ...

Re: [PATCH] idr: Document calling context for IDA APIs mustn't use locks

[Stephen Boyd]

Quoting Matthew Wilcox (2019-07-30 14:12:50)
> On Tue, Jul 30, 2019 at 02:07:52PM -0700, Stephen Boyd wrote:
> > The documentation for these functions indicates that callers don't need
> > to hold a lock while calling them, but that documentation is only in one
> > place under "IDA Usage". Let's state the same information on each IDA
> > function so that it's clear what the calling context requires.
> > Furthermore, let's document ida_simple_get() with the same information
> > so that callers know how this API works.
> 
> I don't want people to use ida_simple_get() any more.  Use ida_alloc()
> instead.

Fair enough. I'll document it as deprecated in another patch.

> 
> > - * Context: Any context.
> > + * Context: Any context. It is safe to call this function without
> > + * synchronisation in your code.
> 
> I prefer "without locking" to "without synchronisation" ...
> 

Ok. Resending shortly.