All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>, Jan Kara <jack@suse.cz>,
	William Kucharski <william.kucharski@oracle.com>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org
Subject: [PATCH next 1/3] truncate,shmem: Fix data loss when hole punched in folio
Date: Sun, 2 Jan 2022 17:32:28 -0800 (PST)	[thread overview]
Message-ID: <43bf0e3-6ad8-9f67-7296-786c7b3b852f@google.com> (raw)

As reported before in
https://lore.kernel.org/lkml/alpine.LSU.2.11.2011160128001.1206@eggly.anvils/
shmem_undo_range() mods sometimes caused good data to be zeroed when
running my tmpfs swapping loads.  Only the ext4-on-loop0-on-tmpfs mount
was seen to suffer, and that is mounted with "-o discard": which punches
holes in the underlying tmpfs file.

shmem_undo_range() partial_end handling was wrong: if (lend + 1) aligned
to page but not to folio, the second shmem_get_folio() could be skipped,
then the whole of that folio punched out instead of treated partially.

Rename same_page to same_folio (like in truncate.c), and rely on that
instead of partial_end: fewer variables, less confusion.  And considering
an off-by-one in setting same_folio initially, pointed to an off-by-one
in the second shmem_get_folio(): it should be on (lend >> PAGE_SHIFT) not
end - which had caused no data loss, but could split folio unnecessarily.

And apply these same fixes in truncate_inode_pages_range().

Fixes: 8842c9c23524 ("truncate,shmem: Handle truncates that split large folios")
Signed-off-by: Hugh Dickins <hughd@google.com>
---

 mm/shmem.c    |   16 ++++++----------
 mm/truncate.c |   15 +++++++--------
 2 files changed, 13 insertions(+), 18 deletions(-)

--- next-20211224/mm/shmem.c
+++ hughd1/mm/shmem.c
@@ -908,10 +908,10 @@ static void shmem_undo_range(struct inod
 	struct folio_batch fbatch;
 	pgoff_t indices[PAGEVEC_SIZE];
 	struct folio *folio;
+	bool same_folio;
 	long nr_swaps_freed = 0;
 	pgoff_t index;
 	int i;
-	bool partial_end;
 
 	if (lend == -1)
 		end = -1;	/* unsigned, so actually very big */
@@ -947,18 +947,14 @@ static void shmem_undo_range(struct inod
 		index++;
 	}
 
-	partial_end = ((lend + 1) % PAGE_SIZE) > 0;
+	same_folio = (lstart >> PAGE_SHIFT) == (lend >> PAGE_SHIFT);
 	shmem_get_folio(inode, lstart >> PAGE_SHIFT, &folio, SGP_READ);
 	if (folio) {
-		bool same_page;
-
-		same_page = lend < folio_pos(folio) + folio_size(folio);
-		if (same_page)
-			partial_end = false;
+		same_folio = lend < folio_pos(folio) + folio_size(folio);
 		folio_mark_dirty(folio);
 		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
 			start = folio->index + folio_nr_pages(folio);
-			if (same_page)
+			if (same_folio)
 				end = folio->index;
 		}
 		folio_unlock(folio);
@@ -966,8 +962,8 @@ static void shmem_undo_range(struct inod
 		folio = NULL;
 	}
 
-	if (partial_end)
-		shmem_get_folio(inode, end, &folio, SGP_READ);
+	if (!same_folio)
+		shmem_get_folio(inode, lend >> PAGE_SHIFT, &folio, SGP_READ);
 	if (folio) {
 		folio_mark_dirty(folio);
 		if (!truncate_inode_partial_folio(folio, lstart, lend))
--- next-20211224/mm/truncate.c
+++ hughd1/mm/truncate.c
@@ -347,8 +347,8 @@ void truncate_inode_pages_range(struct a
 	pgoff_t		indices[PAGEVEC_SIZE];
 	pgoff_t		index;
 	int		i;
-	struct folio *	folio;
-	bool partial_end;
+	struct folio	*folio;
+	bool		same_folio;
 
 	if (mapping_empty(mapping))
 		goto out;
@@ -385,12 +385,10 @@ void truncate_inode_pages_range(struct a
 		cond_resched();
 	}
 
-	partial_end = ((lend + 1) % PAGE_SIZE) > 0;
+	same_folio = (lstart >> PAGE_SHIFT) == (lend >> PAGE_SHIFT);
 	folio = __filemap_get_folio(mapping, lstart >> PAGE_SHIFT, FGP_LOCK, 0);
 	if (folio) {
-		bool same_folio = lend < folio_pos(folio) + folio_size(folio);
-		if (same_folio)
-			partial_end = false;
+		same_folio = lend < folio_pos(folio) + folio_size(folio);
 		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
 			start = folio->index + folio_nr_pages(folio);
 			if (same_folio)
@@ -401,8 +399,9 @@ void truncate_inode_pages_range(struct a
 		folio = NULL;
 	}
 
-	if (partial_end)
-		folio = __filemap_get_folio(mapping, end, FGP_LOCK, 0);
+	if (!same_folio)
+		folio = __filemap_get_folio(mapping, lend >> PAGE_SHIFT,
+						FGP_LOCK, 0);
 	if (folio) {
 		if (!truncate_inode_partial_folio(folio, lstart, lend))
 			end = folio->index;

             reply	other threads:[~2022-01-03  1:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-03  1:32 Hugh Dickins [this message]
2022-01-03  7:51 ` [PATCH next 1/3] truncate,shmem: Fix data loss when hole punched in folio Christoph Hellwig
2022-01-03 20:17   ` Hugh Dickins

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=43bf0e3-6ad8-9f67-7296-786c7b3b852f@google.com \
    --to=hughd@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=william.kucharski@oracle.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.