linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Matthew Wilcox <willy@infradead.org>
Cc: <linux-fsdevel@vger.kernel.org>, <linux-mm@kvack.org>,
	Jan Kara <jack@suse.cz>,
	stable@vger.kernel.org
Subject: [PATCH 1/8] xarray: Fix premature termination of xas_for_each_marked()
Date: Tue,  4 Feb 2020 15:25:07 +0100	[thread overview]
Message-ID: <20200204142514.15826-2-jack@suse.cz> (raw)
In-Reply-To: <20200204142514.15826-1-jack@suse.cz>

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@vger.kernel.org
Fixes: ef8e5717db01 "page cache: Convert delete_batch to XArray"
Signed-off-by: Jan Kara <jack@suse.cz>
---
 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



  reply	other threads:[~2020-02-04 14:25 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-04 14:25 [PATCH 0/8] mm: Speedup page cache truncation Jan Kara
2020-02-04 14:25 ` Jan Kara [this message]
2020-03-12 21:45   ` [PATCH 1/8] xarray: Fix premature termination of xas_for_each_marked() Matthew Wilcox
2020-03-16  9:16     ` Jan Kara
2020-02-04 14:25 ` [PATCH 2/8] xarray: Provide xas_erase() helper Jan Kara
2020-03-14 19:54   ` Matthew Wilcox
2020-03-16  9:21     ` Jan Kara
2020-03-17 15:28   ` Matthew Wilcox
2020-04-15 16:12     ` Jan Kara
2020-02-04 14:25 ` [PATCH 3/8] xarray: Explicitely set XA_FREE_MARK in __xa_cmpxchg() Jan Kara
2020-02-05 18:45   ` Jason Gunthorpe
2020-02-06  8:03     ` Jan Kara
2020-03-17 15:12   ` Matthew Wilcox
2020-02-04 14:25 ` [PATCH 4/8] mm: Use xas_erase() in page_cache_delete_batch() Jan Kara
2020-02-04 14:25 ` [PATCH 5/8] dax: Use xas_erase() in __dax_invalidate_entry() Jan Kara
2020-02-04 14:25 ` [PATCH 6/8] idr: Use xas_erase() in ida_destroy() Jan Kara
2020-02-04 14:25 ` [PATCH 7/8] mm: Use xas_erase() in collapse_file() Jan Kara
2020-02-04 14:25 ` [PATCH 8/8] xarray: Don't clear marks in xas_store() Jan Kara
2020-02-05 18:43   ` Jason Gunthorpe
2020-02-05 21:59     ` Matthew Wilcox
2020-02-06 13:49       ` Jason Gunthorpe
2020-02-06 14:36         ` Jan Kara
2020-02-06 14:49           ` Jason Gunthorpe
2020-02-05 22:19   ` John Hubbard
2020-02-06  2:21     ` Matthew Wilcox
2020-02-06  3:48       ` John Hubbard
2020-02-06  4:28         ` Matthew Wilcox
2020-02-06  4:37           ` John Hubbard
2020-02-06  8:36           ` Jan Kara
2020-02-06  8:04     ` Jan Kara
2020-02-06 14:40 ` [PATCH 0/8] mm: Speedup page cache truncation David Sterba
2020-02-18  9:25 ` Jan Kara

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=20200204142514.15826-2-jack@suse.cz \
    --to=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.org \
    --cc=willy@infradead.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 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).