xas_for_each_marked() is using entry == NULL as a termination condition
of the iteration. When xas_for_each_marked() is used protected only by
RCU, this can however race with xas_store(xas, NULL) in the following
way:
TASK1 TASK2
page_cache_delete() find_get_pages_range_tag()
xas_for_each_marked()
xas_find_marked()
off = xas_find_chunk()
xas_store(&xas, NULL)
xas_init_marks(&xas);
...
rcu_assign_pointer(*slot, NULL);
entry = xa_entry(off);
And thus xas_for_each_marked() terminates prematurely possibly leading
to missed entries in the iteration (translating to missing writeback of
some pages or a similar problem).
Fix the problem by creating a special version of xas_find_marked() -
xas_find_valid_marked() - that does not return NULL marked entries and
changing xas_next_marked() in the same way.
CC: stable@xxxxxxxxxxxxxxx
Fixes: ef8e5717db01 "page cache: Convert delete_batch to XArray"
Signed-off-by: Jan Kara <jack@xxxxxxx>
---
include/linux/xarray.h | 64 ++++++++++++++++++++++++++++++++++++--------------
1 file changed, 47 insertions(+), 17 deletions(-)
diff --git a/include/linux/xarray.h b/include/linux/xarray.h
index f73e1775ded0..5370716d7010 100644
--- a/include/linux/xarray.h
+++ b/include/linux/xarray.h
@@ -1633,33 +1633,63 @@ static inline unsigned int xas_find_chunk(struct xa_state *xas, bool advance,
}
/**
- * xas_next_marked() - Advance iterator to next marked entry.
+ * xas_find_valid_marked() - Find the next marked valid entry in the XArray.
+ * @xas: XArray operation state.
+ * @max: Highest index to return.
+ * @mark: Mark number to search for.
+ *
+ * This is like xas_find_marked() except that we also skip over all %NULL
+ * marked entries.
+ *
+ * Return: The entry, if found, otherwise %NULL.
+ */
+static inline void *xas_find_valid_marked(struct xa_state *xas,
+ unsigned long max, xa_mark_t mark)
+{
+ void *entry;
+
+ do {
+ entry = xas_find_marked(xas, max, mark);
+ } while (unlikely(entry == NULL) && xas_valid(xas));
+
+ return entry;
+}
+
+/**
+ * xas_next_valid_marked() - Advance iterator to next valid marked entry.
* @xas: XArray operation state.
* @max: Highest index to return.
* @mark: Mark to search for.
*
- * xas_next_marked() is an inline function to optimise xarray traversal for
- * speed. It is equivalent to calling xas_find_marked(), and will call
- * xas_find_marked() for all the hard cases.
+ * xas_next_valid_marked() is an inline function to optimise xarray traversal
+ * for speed. It is equivalent to calling xas_find_valid_marked(), and will
+ * call xas_find_marked() for all the hard cases. The function skips over %NULL
+ * marked entries.
*
* Return: The next marked entry after the one currently referred to by @xas.
*/
-static inline void *xas_next_marked(struct xa_state *xas, unsigned long max,
- xa_mark_t mark)
+static inline void *xas_next_valid_marked(struct xa_state *xas,
+ unsigned long max, xa_mark_t mark)
{
struct xa_node *node = xas->xa_node;
unsigned int offset;
+ void *entry;
if (unlikely(xas_not_node(node) || node->shift))
- return xas_find_marked(xas, max, mark);
- offset = xas_find_chunk(xas, true, mark);
- xas->xa_offset = offset;
- xas->xa_index = (xas->xa_index & ~XA_CHUNK_MASK) + offset;
- if (xas->xa_index > max)
- return NULL;
- if (offset == XA_CHUNK_SIZE)
- return xas_find_marked(xas, max, mark);
- return xa_entry(xas->xa, node, offset);
+ return xas_find_valid_marked(xas, max, mark);
+
+ do {
+ offset = xas_find_chunk(xas, true, mark);
+ xas->xa_offset = offset;
+ xas->xa_index = (xas->xa_index & ~XA_CHUNK_MASK) + offset;
+ if (xas->xa_index > max)
+ return NULL;
+ if (offset == XA_CHUNK_SIZE)
+ return xas_find_valid_marked(xas, max, mark);
+ entry = xa_entry(xas->xa, node, offset);
+ } while (unlikely(!entry));
+
+ return entry;
}
/*
@@ -1702,8 +1732,8 @@ enum {
* xas_pause() first.
*/
#define xas_for_each_marked(xas, entry, max, mark) \
- for (entry = xas_find_marked(xas, max, mark); entry; \
- entry = xas_next_marked(xas, max, mark))
+ for (entry = xas_find_valid_marked(xas, max, mark); entry; \
+ entry = xas_next_valid_marked(xas, max, mark))
/**
* xas_for_each_conflict() - Iterate over a range of an XArray.
--
2.16.4
