On 2/4/20 6:25 AM, Jan Kara wrote:
> When storing NULL in xarray, xas_store() has been clearing all marks
> because it could otherwise confuse xas_for_each_marked(). That is
> however no longer true and no current user relies on this behavior.
However, let's not forget that the API was also documented to behave
in this way--it's not an accidental detail. Below...
> Furthermore it seems as a cleaner API to not do clearing behind caller's
> back in case we store NULL.
>
> This provides a nice boost to truncate numbers due to saving unnecessary
> tag initialization when clearing shadow entries. Sample benchmark
> showing time to truncate 128 files 1GB each on machine with 64GB of RAM
> (so about half of entries are shadow entries):
>
> AVG STDDEV
> Vanilla 4.825s 0.036
> Patched 4.516s 0.014
>
> So we can see about 6% reduction in overall truncate time.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> lib/xarray.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/lib/xarray.c b/lib/xarray.c
> index 4e32497c51bd..f165e83652f1 100644
> --- a/lib/xarray.c
> +++ b/lib/xarray.c
> @@ -799,17 +799,8 @@ void *xas_store(struct xa_state *xas, void *entry)
> if (xas->xa_sibs)
> xas_squash_marks(xas);
> }
> - if (!entry)
> - xas_init_marks(xas);
>
> for (;;) {
> - /*
> - * Must clear the marks before setting the entry to NULL,
> - * otherwise xas_for_each_marked may find a NULL entry and
> - * stop early. rcu_assign_pointer contains a release barrier
> - * so the mark clearing will appear to happen before the
> - * entry is set to NULL.
> - */
So if we do this, I think we'd also want something like this (probably with
better wording, this is just a first draft):
diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
index 640934b6f7b4..8adeaa8c012e 100644
--- a/Documentation/core-api/xarray.rst
+++ b/Documentation/core-api/xarray.rst
@@ -66,10 +66,11 @@ 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
+use xa_erase() plus xas_init_marks(), 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.
+been stored to and one that has been erased. Those, in turn, are the same
+as an entry that has had ``NULL`` stored to it and also had its marks
+erased via xas_init_marks().
You can conditionally replace an entry at an index by using
xa_cmpxchg(). Like cmpxchg(), it will only succeed if
> rcu_assign_pointer(*slot, entry);
> if (xa_is_node(next) && (!node || node->shift))
> xas_free_nodes(xas, xa_to_node(next));
>
thanks,
--
John Hubbard
NVIDIA
