* [PATCH next 1/3] truncate,shmem: Fix data loss when hole punched in folio
@ 2022-01-03 1:32 Hugh Dickins
2022-01-03 7:51 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Hugh Dickins @ 2022-01-03 1:32 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Andrew Morton, Christoph Hellwig, Jan Kara, William Kucharski,
linux-fsdevel, linux-mm
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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH next 1/3] truncate,shmem: Fix data loss when hole punched in folio
2022-01-03 1:32 [PATCH next 1/3] truncate,shmem: Fix data loss when hole punched in folio Hugh Dickins
@ 2022-01-03 7:51 ` Christoph Hellwig
2022-01-03 20:17 ` Hugh Dickins
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2022-01-03 7:51 UTC (permalink / raw)
To: Hugh Dickins
Cc: Matthew Wilcox, Andrew Morton, Christoph Hellwig, Jan Kara,
William Kucharski, linux-fsdevel, linux-mm
On Sun, Jan 02, 2022 at 05:32:28PM -0800, Hugh Dickins wrote:
>
> + same_folio = (lstart >> PAGE_SHIFT) == (lend >> PAGE_SHIFT);
Should this move to the else branch?
Same for the other copy of this code. Otherwise this looks sane.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH next 1/3] truncate,shmem: Fix data loss when hole punched in folio
2022-01-03 7:51 ` Christoph Hellwig
@ 2022-01-03 20:17 ` Hugh Dickins
0 siblings, 0 replies; 3+ messages in thread
From: Hugh Dickins @ 2022-01-03 20:17 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Hugh Dickins, Matthew Wilcox, Andrew Morton, Jan Kara,
William Kucharski, linux-fsdevel, linux-mm
On Sun, 2 Jan 2022, Christoph Hellwig wrote:
> On Sun, Jan 02, 2022 at 05:32:28PM -0800, Hugh Dickins wrote:
> >
> > + same_folio = (lstart >> PAGE_SHIFT) == (lend >> PAGE_SHIFT);
>
> Should this move to the else branch?
We could add an else branch and move it there, yes. I liked
Matthew's else-less style with partial_end, and followed that.
Whatever: since he posted that diff yesterday, I imagine he'll
just merge this into the fixed patch, in whatever style he prefers.
>
> Same for the other copy of this code. Otherwise this looks sane.
Thanks,
Hugh
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-01-03 20:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-03 1:32 [PATCH next 1/3] truncate,shmem: Fix data loss when hole punched in folio Hugh Dickins
2022-01-03 7:51 ` Christoph Hellwig
2022-01-03 20:17 ` Hugh Dickins
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.