Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* Re: ext4 fsck vs. kernel recovery policy
       [not found]   ` <20190829225348.GA13045@xps13.dannf>
@ 2019-08-30  1:22     ` Theodore Y. Ts'o
  2019-09-04 14:58       ` dann frazier
  0 siblings, 1 reply; 2+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-30  1:22 UTC (permalink / raw)
  To: dann frazier
  Cc: Andreas Dilger, linux-ext4, Jan Kara, Colin King, Ryan Harper

(Changing the cc from linux-fsdevel to linux-ext4.)

On Thu, Aug 29, 2019 at 04:53:48PM -0600, dann frazier wrote:
> JBD2: Invalid checksum recovering data block 517634 in log
> 
> So is it correct to say that the checksum errors were identifying
> filesystem correctness issues, and therefore e2fsck was needed to
> correct them?

That's correct.  More precisely, checksum errors for journal blocks
are presumed to mean that file system might be corrupt, so a full
e2fsck check was needed to make sure the file system was consistent.

> You're probably right - this issue is very easy to reproduce w/
> data=journal,journal_checksum. I was never able to reproduce it
> otherwise.

I've looked at the data block numbers that you've reported, and they
come from a journald file.  The problem is with data=journal +
journal_checksum + mmap.  Unfortunately, we don't handle that
combination correctly at the moment.

The fix is going to have to involve fixing __ext4_journalled_writepage()
to call set_page_writeback() before it unlocks the page, adding a list of
pages under data=journalled writeback which is attached to the
transaction handle, have the jbd2 commit hook call end_page_writeback()
on all of these pages, and then in the places where ext4 calls
wait_for_stable_page() or grab_cache_page_write_begin(),
we need to add:

	if (ext4_should_journal_data(inode))
		wait_on_page_writeback(page);

It's all relatively straightforward except for the part where we have to
attach a list of pages to the currently running transaction.  That
will require adding  some plumbing into the jbd2 layer.

Dann, any interest in trying to code this fix?

      	  	      	     	     	  - Ted

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: ext4 fsck vs. kernel recovery policy
  2019-08-30  1:22     ` ext4 fsck vs. kernel recovery policy Theodore Y. Ts'o
@ 2019-09-04 14:58       ` dann frazier
  0 siblings, 0 replies; 2+ messages in thread
From: dann frazier @ 2019-09-04 14:58 UTC (permalink / raw)
  To: Theodore Y. Ts'o
  Cc: Andreas Dilger, linux-ext4, Jan Kara, Colin King, Ryan Harper

On Thu, Aug 29, 2019 at 09:22:36PM -0400, Theodore Y. Ts'o wrote:
> (Changing the cc from linux-fsdevel to linux-ext4.)
> 
> On Thu, Aug 29, 2019 at 04:53:48PM -0600, dann frazier wrote:
> > JBD2: Invalid checksum recovering data block 517634 in log
> > 
> > So is it correct to say that the checksum errors were identifying
> > filesystem correctness issues, and therefore e2fsck was needed to
> > correct them?
> 
> That's correct.  More precisely, checksum errors for journal blocks
> are presumed to mean that file system might be corrupt, so a full
> e2fsck check was needed to make sure the file system was consistent.
> 
> > You're probably right - this issue is very easy to reproduce w/
> > data=journal,journal_checksum. I was never able to reproduce it
> > otherwise.
> 
> I've looked at the data block numbers that you've reported, and they
> come from a journald file.  The problem is with data=journal +
> journal_checksum + mmap.  Unfortunately, we don't handle that
> combination correctly at the moment.
> 
> The fix is going to have to involve fixing __ext4_journalled_writepage()
> to call set_page_writeback() before it unlocks the page, adding a list of
> pages under data=journalled writeback which is attached to the
> transaction handle, have the jbd2 commit hook call end_page_writeback()
> on all of these pages, and then in the places where ext4 calls
> wait_for_stable_page() or grab_cache_page_write_begin(),
> we need to add:
> 
> 	if (ext4_should_journal_data(inode))
> 		wait_on_page_writeback(page);
> 
> It's all relatively straightforward except for the part where we have to
> attach a list of pages to the currently running transaction.  That
> will require adding  some plumbing into the jbd2 layer.
> 
> Dann, any interest in trying to code this fix?

Thanks Ted. I've the interest, I'll see if I can find the time :)

  -dann

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CALdTtnuRqgZ=By1JQ0yJJYczUPxxYCWPkAey4BjBkmj77q7aaA@mail.gmail.com>
     [not found] ` <5FEB4E1B-B21B-418D-801D-81FF7C6C069F@dilger.ca>
     [not found]   ` <20190829225348.GA13045@xps13.dannf>
2019-08-30  1:22     ` ext4 fsck vs. kernel recovery policy Theodore Y. Ts'o
2019-09-04 14:58       ` dann frazier

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org linux-ext4@archiver.kernel.org
	public-inbox-index linux-ext4


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/ public-inbox