linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] XArray: minor documentation improvements
@ 2024-10-09 19:36 Tamir Duberstein
  2024-10-09 20:44 ` Darrick J. Wong
  0 siblings, 1 reply; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-09 19:36 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Tamir Duberstein, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

- Replace "they" with "you" where "you" is used in the preceding
  sentence fragment.
- Use "erasing" rather than "storing `NULL`" when describing multi-index
  entries. Split this into a separate sentence.
- Add "call" parentheses on "xa_store" for consistency and
  linkification.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 Documentation/core-api/xarray.rst | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
index 77e0ece2b1d6..d79d4e4ceff6 100644
--- a/Documentation/core-api/xarray.rst
+++ b/Documentation/core-api/xarray.rst
@@ -42,8 +42,8 @@ call xa_tag_pointer() to create an entry with a tag, xa_untag_pointer()
 to turn a tagged entry back into an untagged pointer and xa_pointer_tag()
 to retrieve the tag of an entry.  Tagged pointers use the same bits that
 are used to distinguish value entries from normal pointers, so you must
-decide whether they want to store value entries or tagged pointers in
-any particular XArray.
+decide whether use want to store value entries or tagged pointers in any
+particular XArray.
 
 The XArray does not support storing IS_ERR() pointers as some
 conflict with value entries or internal entries.
@@ -52,8 +52,8 @@ An unusual feature of the XArray is the ability to create entries which
 occupy a range of indices.  Once stored to, looking up any index in
 the range will return the same entry as looking up any other index in
 the range.  Storing to any index will store to all of them.  Multi-index
-entries can be explicitly split into smaller entries, or storing ``NULL``
-into any entry will cause the XArray to forget about the range.
+entries can be explicitly split into smaller entries. Erasing any entry
+will cause the XArray to forget about the range.
 
 Normal API
 ==========
@@ -64,7 +64,7 @@ allocated ones.  A freshly-initialised XArray contains a ``NULL``
 pointer at every index.
 
 You can then set entries using xa_store() and get entries
-using xa_load().  xa_store will overwrite any entry with the
+using xa_load().  xa_store() will overwrite any entry with the
 new entry and return the previous entry stored at that index.  You can
 use xa_erase() instead of calling xa_store() with a
 ``NULL`` entry.  There is no difference between an entry that has never
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] XArray: minor documentation improvements
  2024-10-09 19:36 [PATCH] XArray: minor documentation improvements Tamir Duberstein
@ 2024-10-09 20:44 ` Darrick J. Wong
  2024-10-09 20:51   ` Tamir Duberstein
  0 siblings, 1 reply; 18+ messages in thread
From: Darrick J. Wong @ 2024-10-09 20:44 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

On Wed, Oct 09, 2024 at 03:36:03PM -0400, Tamir Duberstein wrote:
> - Replace "they" with "you" where "you" is used in the preceding
>   sentence fragment.
> - Use "erasing" rather than "storing `NULL`" when describing multi-index
>   entries. Split this into a separate sentence.
> - Add "call" parentheses on "xa_store" for consistency and
>   linkification.
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  Documentation/core-api/xarray.rst | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
> index 77e0ece2b1d6..d79d4e4ceff6 100644
> --- a/Documentation/core-api/xarray.rst
> +++ b/Documentation/core-api/xarray.rst
> @@ -42,8 +42,8 @@ call xa_tag_pointer() to create an entry with a tag, xa_untag_pointer()
>  to turn a tagged entry back into an untagged pointer and xa_pointer_tag()
>  to retrieve the tag of an entry.  Tagged pointers use the same bits that
>  are used to distinguish value entries from normal pointers, so you must
> -decide whether they want to store value entries or tagged pointers in
> -any particular XArray.
> +decide whether use want to store value entries or tagged pointers in any

"...so you must decide if *you* want to store value entries or..."

> +particular XArray.
>  
>  The XArray does not support storing IS_ERR() pointers as some
>  conflict with value entries or internal entries.
> @@ -52,8 +52,8 @@ An unusual feature of the XArray is the ability to create entries which
>  occupy a range of indices.  Once stored to, looking up any index in
>  the range will return the same entry as looking up any other index in
>  the range.  Storing to any index will store to all of them.  Multi-index
> -entries can be explicitly split into smaller entries, or storing ``NULL``
> -into any entry will cause the XArray to forget about the range.
> +entries can be explicitly split into smaller entries. Erasing any entry
> +will cause the XArray to forget about the range.

n00b question: is xa_store(..., NULL) the same as xa_erase?

If it is, then should the documentation mention that xa_store(NULL) is
the same as xa_erase, and that both of these operations will cause the
xarray to forget about that range?

--D

>  Normal API
>  ==========
> @@ -64,7 +64,7 @@ allocated ones.  A freshly-initialised XArray contains a ``NULL``
>  pointer at every index.
>  
>  You can then set entries using xa_store() and get entries
> -using xa_load().  xa_store will overwrite any entry with the
> +using xa_load().  xa_store() will overwrite any entry with the
>  new entry and return the previous entry stored at that index.  You can
>  use xa_erase() instead of calling xa_store() with a
>  ``NULL`` entry.  There is no difference between an entry that has never
> -- 
> 2.47.0
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] XArray: minor documentation improvements
  2024-10-09 20:44 ` Darrick J. Wong
@ 2024-10-09 20:51   ` Tamir Duberstein
  2024-10-09 20:52     ` [PATCH v2] " Tamir Duberstein
  0 siblings, 1 reply; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-09 20:51 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

On Wed, Oct 9, 2024 at 4:44 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> n00b question: is xa_store(..., NULL) the same as xa_erase?
>
> If it is, then should the documentation mention that xa_store(NULL) is
> the same as xa_erase, and that both of these operations will cause the
> xarray to forget about that range?

They're not quite the same in the presence of `XA_FLAGS_ALLOC`. Per the docs:

> Unlike a normal XArray, storing NULL will mark the entry as being in use [...]

See https://docs.kernel.org/core-api/xarray.html#allocating-xarrays.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] XArray: minor documentation improvements
  2024-10-09 20:51   ` Tamir Duberstein
@ 2024-10-09 20:52     ` Tamir Duberstein
  2024-10-09 21:25       ` Darrick J. Wong
                         ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-09 20:52 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Tamir Duberstein, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

- Replace "they" with "you" where "you" is used in the preceding
  sentence fragment.
- Use "erasing" rather than "storing `NULL`" when describing multi-index
  entries. Split this into a separate sentence.
- Add "call" parentheses on "xa_store" for consistency and
  linkification.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
V1 -> V2: s/use/you/ (Darrick J. Wong)

 Documentation/core-api/xarray.rst | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
index 77e0ece2b1d6..75c83b37e88f 100644
--- a/Documentation/core-api/xarray.rst
+++ b/Documentation/core-api/xarray.rst
@@ -42,8 +42,8 @@ call xa_tag_pointer() to create an entry with a tag, xa_untag_pointer()
 to turn a tagged entry back into an untagged pointer and xa_pointer_tag()
 to retrieve the tag of an entry.  Tagged pointers use the same bits that
 are used to distinguish value entries from normal pointers, so you must
-decide whether they want to store value entries or tagged pointers in
-any particular XArray.
+decide whether you want to store value entries or tagged pointers in any
+particular XArray.
 
 The XArray does not support storing IS_ERR() pointers as some
 conflict with value entries or internal entries.
@@ -52,8 +52,8 @@ An unusual feature of the XArray is the ability to create entries which
 occupy a range of indices.  Once stored to, looking up any index in
 the range will return the same entry as looking up any other index in
 the range.  Storing to any index will store to all of them.  Multi-index
-entries can be explicitly split into smaller entries, or storing ``NULL``
-into any entry will cause the XArray to forget about the range.
+entries can be explicitly split into smaller entries. Erasing any entry
+will cause the XArray to forget about the range.
 
 Normal API
 ==========
@@ -64,7 +64,7 @@ allocated ones.  A freshly-initialised XArray contains a ``NULL``
 pointer at every index.
 
 You can then set entries using xa_store() and get entries
-using xa_load().  xa_store will overwrite any entry with the
+using xa_load().  xa_store() will overwrite any entry with the
 new entry and return the previous entry stored at that index.  You can
 use xa_erase() instead of calling xa_store() with a
 ``NULL`` entry.  There is no difference between an entry that has never
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] XArray: minor documentation improvements
  2024-10-09 20:52     ` [PATCH v2] " Tamir Duberstein
@ 2024-10-09 21:25       ` Darrick J. Wong
  2024-10-09 21:59       ` Jonathan Corbet
  2024-10-09 22:22       ` Randy Dunlap
  2 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2024-10-09 21:25 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

On Wed, Oct 09, 2024 at 04:52:38PM -0400, Tamir Duberstein wrote:
> - Replace "they" with "you" where "you" is used in the preceding
>   sentence fragment.
> - Use "erasing" rather than "storing `NULL`" when describing multi-index
>   entries. Split this into a separate sentence.
> - Add "call" parentheses on "xa_store" for consistency and
>   linkification.
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>

/me reads about XA_FLAGS_ALLOC and is ok with this now.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> V1 -> V2: s/use/you/ (Darrick J. Wong)
> 
>  Documentation/core-api/xarray.rst | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
> index 77e0ece2b1d6..75c83b37e88f 100644
> --- a/Documentation/core-api/xarray.rst
> +++ b/Documentation/core-api/xarray.rst
> @@ -42,8 +42,8 @@ call xa_tag_pointer() to create an entry with a tag, xa_untag_pointer()
>  to turn a tagged entry back into an untagged pointer and xa_pointer_tag()
>  to retrieve the tag of an entry.  Tagged pointers use the same bits that
>  are used to distinguish value entries from normal pointers, so you must
> -decide whether they want to store value entries or tagged pointers in
> -any particular XArray.
> +decide whether you want to store value entries or tagged pointers in any
> +particular XArray.
>  
>  The XArray does not support storing IS_ERR() pointers as some
>  conflict with value entries or internal entries.
> @@ -52,8 +52,8 @@ An unusual feature of the XArray is the ability to create entries which
>  occupy a range of indices.  Once stored to, looking up any index in
>  the range will return the same entry as looking up any other index in
>  the range.  Storing to any index will store to all of them.  Multi-index
> -entries can be explicitly split into smaller entries, or storing ``NULL``
> -into any entry will cause the XArray to forget about the range.
> +entries can be explicitly split into smaller entries. Erasing any entry
> +will cause the XArray to forget about the range.
>  
>  Normal API
>  ==========
> @@ -64,7 +64,7 @@ allocated ones.  A freshly-initialised XArray contains a ``NULL``
>  pointer at every index.
>  
>  You can then set entries using xa_store() and get entries
> -using xa_load().  xa_store will overwrite any entry with the
> +using xa_load().  xa_store() will overwrite any entry with the
>  new entry and return the previous entry stored at that index.  You can
>  use xa_erase() instead of calling xa_store() with a
>  ``NULL`` entry.  There is no difference between an entry that has never
> -- 
> 2.47.0
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] XArray: minor documentation improvements
  2024-10-09 20:52     ` [PATCH v2] " Tamir Duberstein
  2024-10-09 21:25       ` Darrick J. Wong
@ 2024-10-09 21:59       ` Jonathan Corbet
  2024-10-09 22:16         ` Tamir Duberstein
  2024-10-09 22:22       ` Randy Dunlap
  2 siblings, 1 reply; 18+ messages in thread
From: Jonathan Corbet @ 2024-10-09 21:59 UTC (permalink / raw)
  To: Tamir Duberstein, Matthew Wilcox
  Cc: Tamir Duberstein, linux-fsdevel, linux-doc, linux-kernel

Tamir Duberstein <tamird@gmail.com> writes:

> - Replace "they" with "you" where "you" is used in the preceding
>   sentence fragment.
> - Use "erasing" rather than "storing `NULL`" when describing multi-index
>   entries. Split this into a separate sentence.
> - Add "call" parentheses on "xa_store" for consistency and
>   linkification.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> V1 -> V2: s/use/you/ (Darrick J. Wong)
>
>  Documentation/core-api/xarray.rst | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

So sorry to pick nits, but it's that kind of patch...:)

> diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
> index 77e0ece2b1d6..75c83b37e88f 100644
> --- a/Documentation/core-api/xarray.rst
> +++ b/Documentation/core-api/xarray.rst
> @@ -42,8 +42,8 @@ call xa_tag_pointer() to create an entry with a tag, xa_untag_pointer()
>  to turn a tagged entry back into an untagged pointer and xa_pointer_tag()
>  to retrieve the tag of an entry.  Tagged pointers use the same bits that
>  are used to distinguish value entries from normal pointers, so you must
> -decide whether they want to store value entries or tagged pointers in
> -any particular XArray.
> +decide whether you want to store value entries or tagged pointers in any
> +particular XArray.
>  
>  The XArray does not support storing IS_ERR() pointers as some
>  conflict with value entries or internal entries.
> @@ -52,8 +52,8 @@ An unusual feature of the XArray is the ability to create entries which
>  occupy a range of indices.  Once stored to, looking up any index in
>  the range will return the same entry as looking up any other index in
>  the range.  Storing to any index will store to all of them.  Multi-index
> -entries can be explicitly split into smaller entries, or storing ``NULL``
> -into any entry will cause the XArray to forget about the range.
> +entries can be explicitly split into smaller entries. Erasing any entry
> +will cause the XArray to forget about the range.

I'm not convinced that this is better.  This is programmer
documentation, and "storing NULL" says exactly what is going on.
"Erasing" is, IMO, less clear.

Whatever; if Willy's happy I'll certainly apply this.

Thanks,

jon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] XArray: minor documentation improvements
  2024-10-09 21:59       ` Jonathan Corbet
@ 2024-10-09 22:16         ` Tamir Duberstein
  0 siblings, 0 replies; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-09 22:16 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: Matthew Wilcox, linux-fsdevel, linux-doc, linux-kernel

On Wed, Oct 9, 2024 at 5:59 PM Jonathan Corbet <corbet@lwn.net> wrote:
>
> I'm not convinced that this is better.  This is programmer
> documentation, and "storing NULL" says exactly what is going on.
> "Erasing" is, IMO, less clear.

Both are verbs that appear in function names:
"storing NULL" is to `xa_store(NULL)` as "erasing" is to `xa_erase()`.

Cheers.
Tamir

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] XArray: minor documentation improvements
  2024-10-09 20:52     ` [PATCH v2] " Tamir Duberstein
  2024-10-09 21:25       ` Darrick J. Wong
  2024-10-09 21:59       ` Jonathan Corbet
@ 2024-10-09 22:22       ` Randy Dunlap
  2024-10-10 14:11         ` Tamir Duberstein
  2 siblings, 1 reply; 18+ messages in thread
From: Randy Dunlap @ 2024-10-09 22:22 UTC (permalink / raw)
  To: Tamir Duberstein, Matthew Wilcox
  Cc: Jonathan Corbet, linux-fsdevel, linux-doc, linux-kernel



On 10/9/24 1:52 PM, Tamir Duberstein wrote:
> - Replace "they" with "you" where "you" is used in the preceding
>   sentence fragment.
> - Use "erasing" rather than "storing `NULL`" when describing multi-index
>   entries. Split this into a separate sentence.
> - Add "call" parentheses on "xa_store" for consistency and
>   linkification.
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> V1 -> V2: s/use/you/ (Darrick J. Wong)
> 
>  Documentation/core-api/xarray.rst | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
> index 77e0ece2b1d6..75c83b37e88f 100644
> --- a/Documentation/core-api/xarray.rst
> +++ b/Documentation/core-api/xarray.rst

> @@ -52,8 +52,8 @@ An unusual feature of the XArray is the ability to create entries which
>  occupy a range of indices.  Once stored to, looking up any index in
>  the range will return the same entry as looking up any other index in
>  the range.  Storing to any index will store to all of them.  Multi-index
> -entries can be explicitly split into smaller entries, or storing ``NULL``

Is storing %NULL does by making a function call or just by doing
	*xa1 = NULL;

?

> -into any entry will cause the XArray to forget about the range.
> +entries can be explicitly split into smaller entries. Erasing any entry
> +will cause the XArray to forget about the range.

Clearing any entry by calling xa_erase() will cause the XArray to forget about the range.


>  
>  Normal API
>  ==========


-- 
~Randy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] XArray: minor documentation improvements
  2024-10-09 22:22       ` Randy Dunlap
@ 2024-10-10 14:11         ` Tamir Duberstein
  2024-10-10 14:12           ` [PATCH v3] " Tamir Duberstein
  0 siblings, 1 reply; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-10 14:11 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

On Wed, Oct 9, 2024 at 6:22 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Is storing %NULL does by making a function call or just by doing
>         *xa1 = NULL;
>
> ?

No, you cannot interact with XArray this way.

> > -into any entry will cause the XArray to forget about the range.
> > +entries can be explicitly split into smaller entries. Erasing any entry
> > +will cause the XArray to forget about the range.
>
> Clearing any entry by calling xa_erase() will cause the XArray to forget about the range.

Will send v3 in a moment with new phrasing.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3] XArray: minor documentation improvements
  2024-10-10 14:11         ` Tamir Duberstein
@ 2024-10-10 14:12           ` Tamir Duberstein
  2024-10-10 21:34             ` Randy Dunlap
  0 siblings, 1 reply; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-10 14:12 UTC (permalink / raw)
  To: tamird
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

- Replace "they" with "you" where "you" is used in the preceding
  sentence fragment.
- Mention `xa_erase` in discussion of multi-index entries.  Split this
  into a separate sentence.
- Add "call" parentheses on "xa_store" for consistency and
  linkification.
- Add caveat that `xa_store` and `xa_erase` are not equivalent in the
  presence of `XA_FLAGS_ALLOC`.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
V2 -> V3:
  - metion `xa_erase`/`xa_store(NULL)` in multi-index entry discussion.
  - mention non-equivalent of `xa_erase`/`xa_store(NULL)` in the
    presence of `XA_FLAGS_ALLOC`.
V1 -> V2: s/use/you/ (Darrick J. Wong)

 Documentation/core-api/xarray.rst | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
index 77e0ece2b1d6..78bbb031de91 100644
--- a/Documentation/core-api/xarray.rst
+++ b/Documentation/core-api/xarray.rst
@@ -42,8 +42,8 @@ call xa_tag_pointer() to create an entry with a tag, xa_untag_pointer()
 to turn a tagged entry back into an untagged pointer and xa_pointer_tag()
 to retrieve the tag of an entry.  Tagged pointers use the same bits that
 are used to distinguish value entries from normal pointers, so you must
-decide whether they want to store value entries or tagged pointers in
-any particular XArray.
+decide whether you want to store value entries or tagged pointers in any
+particular XArray.
 
 The XArray does not support storing IS_ERR() pointers as some
 conflict with value entries or internal entries.
@@ -52,8 +52,9 @@ An unusual feature of the XArray is the ability to create entries which
 occupy a range of indices.  Once stored to, looking up any index in
 the range will return the same entry as looking up any other index in
 the range.  Storing to any index will store to all of them.  Multi-index
-entries can be explicitly split into smaller entries, or storing ``NULL``
-into any entry will cause the XArray to forget about the range.
+entries can be explicitly split into smaller entries. Unsetting (using
+xa_erase() or xa_store() with ``NULL``) any entry will cause the XArray
+to forget about the range.
 
 Normal API
 ==========
@@ -63,13 +64,14 @@ for statically allocated XArrays or xa_init() for dynamically
 allocated ones.  A freshly-initialised XArray contains a ``NULL``
 pointer at every index.
 
-You can then set entries using xa_store() and get entries
-using xa_load().  xa_store will overwrite any entry with the
-new entry and return the previous entry stored at that index.  You can
-use xa_erase() instead of calling xa_store() with a
+You can then set entries using xa_store() and get entries using
+xa_load().  xa_store() will overwrite any entry with the new entry and
+return the previous entry stored at that index.  You can unset entries
+using xa_erase() or by setting the entry to ``NULL`` using xa_store().
 ``NULL`` entry.  There is no difference between an entry that has never
-been stored to, one that has been erased and one that has most recently
-had ``NULL`` stored to it.
+been stored to and one that has been erased with xa_erase(); an entry
+that has most recently had ``NULL`` stored to it is also equivalent
+except if the XArray was initialized with ``XA_FLAGS_ALLOC``.
 
 You can conditionally replace an entry at an index by using
 xa_cmpxchg().  Like cmpxchg(), it will only succeed if
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] XArray: minor documentation improvements
  2024-10-10 14:12           ` [PATCH v3] " Tamir Duberstein
@ 2024-10-10 21:34             ` Randy Dunlap
  2024-10-10 21:39               ` Tamir Duberstein
  0 siblings, 1 reply; 18+ messages in thread
From: Randy Dunlap @ 2024-10-10 21:34 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

Hi Tamir,

On 10/10/24 7:12 AM, Tamir Duberstein wrote:
>  Normal API
>  ==========
> @@ -63,13 +64,14 @@ for statically allocated XArrays or xa_init() for dynamically
>  allocated ones.  A freshly-initialised XArray contains a ``NULL``
>  pointer at every index.
>  
> -You can then set entries using xa_store() and get entries
> -using xa_load().  xa_store will overwrite any entry with the
> -new entry and return the previous entry stored at that index.  You can
> -use xa_erase() instead of calling xa_store() with a
> +You can then set entries using xa_store() and get entries using
> +xa_load().  xa_store() will overwrite any entry with the new entry and
> +return the previous entry stored at that index.  You can unset entries
> +using xa_erase() or by setting the entry to ``NULL`` using xa_store().
>  ``NULL`` entry.  There is no difference between an entry that has never

Is the line above supposed to be here?
Confusing to me.
Thanks.

> -been stored to, one that has been erased and one that has most recently
> -had ``NULL`` stored to it.
> +been stored to and one that has been erased with xa_erase(); an entry
> +that has most recently had ``NULL`` stored to it is also equivalent
> +except if the XArray was initialized with ``XA_FLAGS_ALLOC``.

-- 
~Randy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] XArray: minor documentation improvements
  2024-10-10 21:34             ` Randy Dunlap
@ 2024-10-10 21:39               ` Tamir Duberstein
  2024-10-10 21:41                 ` [PATCH v4] " Tamir Duberstein
  0 siblings, 1 reply; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-10 21:39 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

Hi Randy!

On Thu, Oct 10, 2024 at 5:35 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > -You can then set entries using xa_store() and get entries
> > -using xa_load().  xa_store will overwrite any entry with the
> > -new entry and return the previous entry stored at that index.  You can
> > -use xa_erase() instead of calling xa_store() with a
> > +You can then set entries using xa_store() and get entries using
> > +xa_load().  xa_store() will overwrite any entry with the new entry and
> > +return the previous entry stored at that index.  You can unset entries
> > +using xa_erase() or by setting the entry to ``NULL`` using xa_store().
> >  ``NULL`` entry.  There is no difference between an entry that has never
>
> Is the line above supposed to be here?
> Confusing to me.
> Thanks.

Ah I think there's a latent sentence fragment there.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4] XArray: minor documentation improvements
  2024-10-10 21:39               ` Tamir Duberstein
@ 2024-10-10 21:41                 ` Tamir Duberstein
  2024-10-10 21:50                   ` Randy Dunlap
  2024-10-17  0:34                   ` Bagas Sanjaya
  0 siblings, 2 replies; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-10 21:41 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

- Replace "they" with "you" where "you" is used in the preceding
  sentence fragment.
- Mention `xa_erase` in discussion of multi-index entries.  Split this
  into a separate sentence.
- Add "call" parentheses on "xa_store" for consistency and
  linkification.
- Add caveat that `xa_store` and `xa_erase` are not equivalent in the
  presence of `XA_FLAGS_ALLOC`.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
V3 -> V4: Remove latent sentence fragment.
V2 -> V3:
  - metion `xa_erase`/`xa_store(NULL)` in multi-index entry discussion.
  - mention non-equivalent of `xa_erase`/`xa_store(NULL)` in the
    presence of `XA_FLAGS_ALLOC`.
V1 -> V2: s/use/you/ (Darrick J. Wong)

 Documentation/core-api/xarray.rst | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
index 77e0ece2b1d6..f6a3eef4fe7f 100644
--- a/Documentation/core-api/xarray.rst
+++ b/Documentation/core-api/xarray.rst
@@ -42,8 +42,8 @@ call xa_tag_pointer() to create an entry with a tag, xa_untag_pointer()
 to turn a tagged entry back into an untagged pointer and xa_pointer_tag()
 to retrieve the tag of an entry.  Tagged pointers use the same bits that
 are used to distinguish value entries from normal pointers, so you must
-decide whether they want to store value entries or tagged pointers in
-any particular XArray.
+decide whether you want to store value entries or tagged pointers in any
+particular XArray.
 
 The XArray does not support storing IS_ERR() pointers as some
 conflict with value entries or internal entries.
@@ -52,8 +52,9 @@ An unusual feature of the XArray is the ability to create entries which
 occupy a range of indices.  Once stored to, looking up any index in
 the range will return the same entry as looking up any other index in
 the range.  Storing to any index will store to all of them.  Multi-index
-entries can be explicitly split into smaller entries, or storing ``NULL``
-into any entry will cause the XArray to forget about the range.
+entries can be explicitly split into smaller entries. Unsetting (using
+xa_erase() or xa_store() with ``NULL``) any entry will cause the XArray
+to forget about the range.
 
 Normal API
 ==========
@@ -63,13 +64,14 @@ for statically allocated XArrays or xa_init() for dynamically
 allocated ones.  A freshly-initialised XArray contains a ``NULL``
 pointer at every index.
 
-You can then set entries using xa_store() and get entries
-using xa_load().  xa_store will overwrite any entry with the
-new entry and return the previous entry stored at that index.  You can
-use xa_erase() instead of calling xa_store() with a
-``NULL`` entry.  There is no difference between an entry that has never
-been stored to, one that has been erased and one that has most recently
-had ``NULL`` stored to it.
+You can then set entries using xa_store() and get entries using
+xa_load().  xa_store() will overwrite any entry with the new entry and
+return the previous entry stored at that index.  You can unset entries
+using xa_erase() or by setting the entry to ``NULL`` using xa_store().
+There is no difference between an entry that has never been stored to
+and one that has been erased with xa_erase(); an entry that has most
+recently had ``NULL`` stored to it is also equivalent except if the
+XArray was initialized with ``XA_FLAGS_ALLOC``.
 
 You can conditionally replace an entry at an index by using
 xa_cmpxchg().  Like cmpxchg(), it will only succeed if
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] XArray: minor documentation improvements
  2024-10-10 21:41                 ` [PATCH v4] " Tamir Duberstein
@ 2024-10-10 21:50                   ` Randy Dunlap
  2024-10-10 22:41                     ` Matthew Wilcox
  2024-10-17  0:34                   ` Bagas Sanjaya
  1 sibling, 1 reply; 18+ messages in thread
From: Randy Dunlap @ 2024-10-10 21:50 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel



On 10/10/24 2:41 PM, Tamir Duberstein wrote:
> - Replace "they" with "you" where "you" is used in the preceding
>   sentence fragment.
> - Mention `xa_erase` in discussion of multi-index entries.  Split this
>   into a separate sentence.
> - Add "call" parentheses on "xa_store" for consistency and
>   linkification.
> - Add caveat that `xa_store` and `xa_erase` are not equivalent in the
>   presence of `XA_FLAGS_ALLOC`.
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> V3 -> V4: Remove latent sentence fragment.
> V2 -> V3:
>   - metion `xa_erase`/`xa_store(NULL)` in multi-index entry discussion.
>   - mention non-equivalent of `xa_erase`/`xa_store(NULL)` in the
>     presence of `XA_FLAGS_ALLOC`.
> V1 -> V2: s/use/you/ (Darrick J. Wong)
> 
>  Documentation/core-api/xarray.rst | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 

I'm satisfied with this change although obviously it's up to Matthew.
Thanks.

Acked-by: Randy Dunlap <rdunlap@infradead.org>

-- 
~Randy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] XArray: minor documentation improvements
  2024-10-10 21:50                   ` Randy Dunlap
@ 2024-10-10 22:41                     ` Matthew Wilcox
  2024-10-16 12:51                       ` Tamir Duberstein
  0 siblings, 1 reply; 18+ messages in thread
From: Matthew Wilcox @ 2024-10-10 22:41 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Tamir Duberstein, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

On Thu, Oct 10, 2024 at 02:50:24PM -0700, Randy Dunlap wrote:
> I'm satisfied with this change although obviously it's up to Matthew.

Matthew's on holiday and will be back on Tuesday, thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] XArray: minor documentation improvements
  2024-10-10 22:41                     ` Matthew Wilcox
@ 2024-10-16 12:51                       ` Tamir Duberstein
  2024-10-24 19:52                         ` Tamir Duberstein
  0 siblings, 1 reply; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-16 12:51 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Randy Dunlap, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

On Thu, Oct 10, 2024 at 6:41 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Thu, Oct 10, 2024 at 02:50:24PM -0700, Randy Dunlap wrote:
> > I'm satisfied with this change although obviously it's up to Matthew.
>
> Matthew's on holiday and will be back on Tuesday, thanks.

Hi Matthew, could you give this a look please? Thank you.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] XArray: minor documentation improvements
  2024-10-10 21:41                 ` [PATCH v4] " Tamir Duberstein
  2024-10-10 21:50                   ` Randy Dunlap
@ 2024-10-17  0:34                   ` Bagas Sanjaya
  1 sibling, 0 replies; 18+ messages in thread
From: Bagas Sanjaya @ 2024-10-17  0:34 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Matthew Wilcox, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Thu, Oct 10, 2024 at 05:41:51PM -0400, Tamir Duberstein wrote:
> - Replace "they" with "you" where "you" is used in the preceding
>   sentence fragment.
> - Mention `xa_erase` in discussion of multi-index entries.  Split this
>   into a separate sentence.
> - Add "call" parentheses on "xa_store" for consistency and
>   linkification.
> - Add caveat that `xa_store` and `xa_erase` are not equivalent in the
>   presence of `XA_FLAGS_ALLOC`.
> 

LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] XArray: minor documentation improvements
  2024-10-16 12:51                       ` Tamir Duberstein
@ 2024-10-24 19:52                         ` Tamir Duberstein
  0 siblings, 0 replies; 18+ messages in thread
From: Tamir Duberstein @ 2024-10-24 19:52 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Randy Dunlap, Jonathan Corbet, linux-fsdevel, linux-doc,
	linux-kernel

On Wed, Oct 16, 2024 at 8:51 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Thu, Oct 10, 2024 at 6:41 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Thu, Oct 10, 2024 at 02:50:24PM -0700, Randy Dunlap wrote:
> > > I'm satisfied with this change although obviously it's up to Matthew.
> >
> > Matthew's on holiday and will be back on Tuesday, thanks.
>
> Hi Matthew, could you give this a look please? Thank you.

Hi Matthew, could you please have a look at this?

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-10-24 19:53 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 19:36 [PATCH] XArray: minor documentation improvements Tamir Duberstein
2024-10-09 20:44 ` Darrick J. Wong
2024-10-09 20:51   ` Tamir Duberstein
2024-10-09 20:52     ` [PATCH v2] " Tamir Duberstein
2024-10-09 21:25       ` Darrick J. Wong
2024-10-09 21:59       ` Jonathan Corbet
2024-10-09 22:16         ` Tamir Duberstein
2024-10-09 22:22       ` Randy Dunlap
2024-10-10 14:11         ` Tamir Duberstein
2024-10-10 14:12           ` [PATCH v3] " Tamir Duberstein
2024-10-10 21:34             ` Randy Dunlap
2024-10-10 21:39               ` Tamir Duberstein
2024-10-10 21:41                 ` [PATCH v4] " Tamir Duberstein
2024-10-10 21:50                   ` Randy Dunlap
2024-10-10 22:41                     ` Matthew Wilcox
2024-10-16 12:51                       ` Tamir Duberstein
2024-10-24 19:52                         ` Tamir Duberstein
2024-10-17  0:34                   ` Bagas Sanjaya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).