ReiserFS / 2.4.6 / Data Corruption
On Saturday, July 28, 2001 03:28:05 AM +1000 Andrew Morton <[email protected]> wrote:
[ patch to trigger data writes before commit in reiserfs ]
Quoted 5 lines
There's no disruption to disk format - it just simulates the user typing `sync' at the right time. I think the concept is sound, and I'm sure Chris can find a more efficient way...
Well, its gets points for simplicity ;-)
What I think we need is for commit_write to put new buffers a per super list of new buffers, and then the journal code can flush that list on commit.
Since all the filesystems already mark things BH_New, it seems a good choice to let commit_write look for BH_New buffers and put them on this new list. But, the only place BH_New seems to get cleared right now is unmap_buffer, which only gets called from block_flushpage.
Is there any reason we can't just clear BH_New before writing the buffer out? It looks like a bug to leave it set the way we do now.
-chris
Quoted 7 lines · Chris Mason
On Saturday, July 28, 2001 03:28:05 AM +1000 Andrew Morton <[email protected]> wrote:
[ patch to trigger data writes before commit in reiserfs ]
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
There's no disruption to disk format - it just simulates the user typing `sync' at the right time. I think the concept is sound, and I'm sure Chris can find a more efficient way...
Well, its gets points for simplicity ;-)
Well, I tried system("/bin/sync"); but that didn't link.
Quoted 3 lines
What I think we need is for
commit_writeto put new buffers a per super list of new buffers, and then the journal code can flush that list on commit.
whee. Now there's an idea - If the fs keeps track of all its inodes then you can traverse those and flush out the i_dirty_buffers ring on each one.
writepage() output is a problem, but that never sits well with journalling. I guess one could do fdatasync/fdatawait against the same list of inodes.
Quoted 7 lines
Since all the filesystems already mark things
BH_New, it seems a good choice to letcommit_writelook forBH_Newbuffers and put them on this new list. But, the only placeBH_Newseems to get cleared right now isunmap_buffer, which only gets called fromblock_flushpage.Is there any reason we can't just clear
BH_Newbefore writing the buffer out? It looks like a bug to leave it set the way we do now.
I think it can be cleared as soon as the get_block() caller has looked at it, actually. test_and_clear_bit. The lifecycle of the various buffer_head fields is exhasperatingly fluffy.
I'd be reluctant to add another eight bytes to buffer_head though. It's 96 now, which is a nice number. b_inode can go - it's just a boolean. b_size and b_list can be crunched into a single byte..
How about just doing it via the inodes?
-
Quoted 17 lines · On Tuesday, July 31, 2001 01:47:25 AM +1000 Andrew Morton <[email protected]>…
Quoted 7 lines · Chris Mason
On Saturday, July 28, 2001 03:28:05 AM +1000 Andrew Morton <[email protected]> wrote:
[ patch to trigger data writes before commit in reiserfs ]
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
There's no disruption to disk format - it just simulates the user typing `sync' at the right time. I think the concept is sound, and I'm sure Chris can find a more efficient way...
Well, its gets points for simplicity ;-)
Well, I tried
system("/bin/sync"); but that didn't link.
Quoted 7 lines
Quoted 3 lines
What I think we need is for
commit_writeto put new buffers a per super list of new buffers, and then the journal code can flush that list on commit.whee. Now there's an idea - If the fs keeps track of all its inodes then you can traverse those and flush out the
i_dirty_buffersring on each one.
It won't work as well in a generic sense, but I was planning on just using the b_inode_buffers. Instead of going onto the inode's dirty buffer list, they go onto this special list instead (the reiserfs journal already has a dummy inode used for this).
The advantage is that nothing extra is needed on the buffer head, but the disadvantage is the buffer doesn't go on the inode's list. Somebody needs to flush the new list on fsync and such. It works for reiserfs, but not in general.
I think you're right about being able to clear BH_New once get_block tests it. Unless someone comes up with a reason against, I'll test it out. I'm guessing that we are wasting time rerunning unmap_underlying_metadata on buffers marked BH_New.
-chris