Skip to contentSkip to conversation

FAILED: patch "[PATCH] ocfs2: fix data corruption after failed write" failed to apply to 5.10-stable tree

Ocfs2-devel New gregkh--- via Ocfs2-devel 1 participant 5 messages Mar 20, 2023 – Mar 21, 2023

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 90410bcf873cf05f54a32183afff0161f44f9715
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '[email protected]' --subject-prefix 'PATCH 5.10.y' HEAD^..

Possible dependencies:

90410bcf873c ("ocfs2: fix data corruption after failed write")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 90410bcf873cf05f54a32183afff0161f44f9715 Mon Sep 17 00:00:00 2001
From: Jan Kara via Ocfs2-devel <[email protected]>
Date: Thu, 2 Mar 2023 16:38:43 +0100
Subject: [PATCH] ocfs2: fix data corruption after failed write

When buffered write fails to copy data into underlying page cache page, ocfs2_write_end_nolock() just zeroes out and dirties the page. This can leave dirty page beyond EOF and if page writeback tries to write this page before write succeeds and expands i_size, page gets into inconsistent state where page dirty bit is clear but buffer dirty bits stay set resulting in page data never getting written and so data copied to the page is lost. Fix the problem by invalidating page beyond EOF after failed write.

Fixes 6dbf7bb55598 ("fs: Don't invalidate page buffers in block_write_full_page()")
Cc Mark Fasheh <[email protected]>
Cc Joel Becker <[email protected]>
Cc Junxiao Bi <[email protected]>
Cc Changwei Ge <[email protected]>
Cc Gang He <[email protected]>
Cc Jun Piao <[email protected]>
fs/ocfs2/aops.c +17 -2
index 1d65f6ef00ca..0394505fdce3 100644
@@ -1977,11 +1977,26 @@ int ocfs2_write_end_nolock(struct address_space *mapping,
 	}
 
 	if (unlikely(copied < len) && wc->w_target_page) {
+		loff_t new_isize;
+
 		if (!PageUptodate(wc->w_target_page))
 			copied = 0;
 
-		ocfs2_zero_new_buffers(wc->w_target_page, start+copied,
-				       start+len);
+		new_isize = max_t(loff_t, i_size_read(inode), pos + copied);
+		if (new_isize > page_offset(wc->w_target_page))
+			ocfs2_zero_new_buffers(wc->w_target_page, start+copied,
+					       start+len);
+		else {
+			/*
+			 * When page is fully beyond new isize (data copy
+			 * failed), do not bother zeroing the page. Invalidate
+			 * it instead so that writeback does not get confused
+			 * put page & buffer dirty bits into inconsistent
+			 * state.
+			 */
+			block_invalidate_folio(page_folio(wc->w_target_page),
+						0, PAGE_SIZE);
+		}
 	}
 	if (wc->w_target_page)
 		flush_dcache_page(wc->w_target_page);
#1 lore raw

Hi Greg,
It can be cleanly applied for linux-5.10.y and linux-4.19.y in my desktop. I'm not sure how it happens.

Thanks,
Joseph

Quoted 91 lines · On 3/20/23 7:57 PM, gregkh--- via Ocfs2-devel wrote:

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 90410bcf873cf05f54a32183afff0161f44f9715
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '[email protected]' --subject-prefix 'PATCH 5.10.y' HEAD^..

Possible dependencies:

90410bcf873c ("ocfs2: fix data corruption after failed write")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 90410bcf873cf05f54a32183afff0161f44f9715 Mon Sep 17 00:00:00 2001
From: Jan Kara via Ocfs2-devel <[email protected]>
Date: Thu, 2 Mar 2023 16:38:43 +0100
Subject: [PATCH] ocfs2: fix data corruption after failed write

When buffered write fails to copy data into underlying page cache page, ocfs2_write_end_nolock() just zeroes out and dirties the page. This can leave dirty page beyond EOF and if page writeback tries to write this page before write succeeds and expands i_size, page gets into inconsistent state where page dirty bit is clear but buffer dirty bits stay set resulting in page data never getting written and so data copied to the page is lost. Fix the problem by invalidating page beyond EOF after failed write.

Fixes 6dbf7bb55598 ("fs: Don't invalidate page buffers in block_write_full_page()")
Cc Mark Fasheh <[email protected]>
Cc Joel Becker <[email protected]>
Cc Junxiao Bi <[email protected]>
Cc Changwei Ge <[email protected]>
Cc Gang He <[email protected]>
Cc Jun Piao <[email protected]>
fs/ocfs2/aops.c +17 -2
index 1d65f6ef00ca..0394505fdce3 100644
@@ -1977,11 +1977,26 @@ int ocfs2_write_end_nolock(struct address_space *mapping,
 	}
 
 	if (unlikely(copied < len) && wc->w_target_page) {
+		loff_t new_isize;
+
 		if (!PageUptodate(wc->w_target_page))
 			copied = 0;
 
-		ocfs2_zero_new_buffers(wc->w_target_page, start+copied,
-				       start+len);
+		new_isize = max_t(loff_t, i_size_read(inode), pos + copied);
+		if (new_isize > page_offset(wc->w_target_page))
+			ocfs2_zero_new_buffers(wc->w_target_page, start+copied,
+					       start+len);
+		else {
+			/*
+			 * When page is fully beyond new isize (data copy
+			 * failed), do not bother zeroing the page. Invalidate
+			 * it instead so that writeback does not get confused
+			 * put page & buffer dirty bits into inconsistent
+			 * state.
+			 */
+			block_invalidate_folio(page_folio(wc->w_target_page),
+						0, PAGE_SIZE);
+		}
 	}
 	if (wc->w_target_page)
 		flush_dcache_page(wc->w_target_page);

_______________________________________________
Ocfs2-devel mailing list
[email protected]
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

#2 lore raw
Quoted 4 lines · Joseph Qi

Hi Greg,
It can be cleanly applied for linux-5.10.y and linux-4.19.y in my desktop. I'm not sure how it happens.

If you can apply the backport, why don't you post it here?

Signature · 1 line
An old man doll... just what I always wanted! - Clara
#3 lore raw
Quoted 8 lines · Bagas Sanjaya
Quoted 4 lines · Joseph Qi

Hi Greg,
It can be cleanly applied for linux-5.10.y and linux-4.19.y in my desktop. I'm not sure how it happens.

If you can apply the backport, why don't you post it here?

My mistake, it requires feature folio.
Will adapt and post it later.

Thanks,
Joseph

_______________________________________________
Ocfs2-devel mailing list
[email protected]
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

#4 lore raw

PATCH 1/1 ocfs2: fix data corruption after failed write

In reply to gregkh--- via Ocfs2-devel

From: Jan Kara via Ocfs2-devel <[email protected]>

commit 90410bcf873cf05f54a32183afff0161f44f9715 upstream.

When buffered write fails to copy data into underlying page cache page, ocfs2_write_end_nolock() just zeroes out and dirties the page. This can leave dirty page beyond EOF and if page writeback tries to write this page before write succeeds and expands i_size, page gets into inconsistent state where page dirty bit is clear but buffer dirty bits stay set resulting in page data never getting written and so data copied to the page is lost. Fix the problem by invalidating page beyond EOF after failed write.

Fixes 6dbf7bb55598 ("fs: Don't invalidate page buffers in block_write_full_page()")
Cc Mark Fasheh <[email protected]>
Cc Joel Becker <[email protected]>
Cc Junxiao Bi <[email protected]>
Cc Changwei Ge <[email protected]>
Cc Gang He <[email protected]>
Cc Jun Piao <[email protected]>
[ replace block_invalidate_folio to block_invalidatepage ]
---
 fs/ocfs2/aops.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
fs/ocfs2/aops.c +16 -2
index ad20403b383f..9b23e74036eb 100644
@@ -1981,11 +1981,25 @@ int ocfs2_write_end_nolock(struct address_space *mapping,
 	}
 
 	if (unlikely(copied < len) && wc->w_target_page) {
+		loff_t new_isize;
+
 		if (!PageUptodate(wc->w_target_page))
 			copied = 0;
 
-		ocfs2_zero_new_buffers(wc->w_target_page, start+copied,
-				       start+len);
+		new_isize = max_t(loff_t, i_size_read(inode), pos + copied);
+		if (new_isize > page_offset(wc->w_target_page))
+			ocfs2_zero_new_buffers(wc->w_target_page, start+copied,
+					       start+len);
+		else {
+			/*
+			 * When page is fully beyond new isize (data copy
+			 * failed), do not bother zeroing the page. Invalidate
+			 * it instead so that writeback does not get confused
+			 * put page & buffer dirty bits into inconsistent
+			 * state.
+			 */
+			block_invalidatepage(wc->w_target_page, 0, PAGE_SIZE);
+		}
 	}
 	if (wc->w_target_page)
 		flush_dcache_page(wc->w_target_page);
Signature · 7 lines
2.24.4


_______________________________________________
Ocfs2-devel mailing list
[email protected]
https://oss.oracle.com/mailman/listinfo/ocfs2-devel