lkml.org 
[lkml]   [2011]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [RFC] block integrity: Fix write after checksum calculation problem
    Date
    Excerpts from Jan Kara's message of 2011-03-21 10:04:51 -0400:
    > On Fri 18-03-11 17:07:55, Darrick J. Wong wrote:
    > > > > Ok, here's what I have so far. I took everyone's suggestions of where to add
    > > > > calls to wait_on_page_writeback, which seems to handle the multiple-write case
    > > > > adequately. Unfortunately, it is still possible to generate checksum errors by
    > > > > scribbling furiously on a mmap'd region, even after adding the writeback wait
    > > > > in the ext4 writepage function. Oddly, I couldn't break btrfs with mmap by
    > > > > removing its wait_for_page_writeback call, so I suspect there's a bit more
    > > > > going on in btrfs than I've been able to figure out.
    > >
    > > I wonder, is it possible for this to happen:
    > >
    > > 1. Thread A mmaps a page and tries to write to it. ext4_page_mkwrite executes,
    > > but there's no ongoing writeback, so it returns without delay.
    > > 2. Thread A starts writing furiously to the page.
    > > 3. Thread B runs fsync() or something that results in the page being
    > > checksummed and scheduled for writeout.
    > > 4. Thread A continues to write furiously(!) on that same page before the
    > > controller finishes the DMA transfer.
    > > 5. Disk gets the page, which now doesn't match its checksum, and *boom*
    > What happens on writepage (see mm/page-writeback.c:write_cache_pages())
    > is:
    > lock_page(page)
    > ...
    > clear_page_dirty_for_io() - removes PageDirty, marks page as read-only in
    > PTE
    > ...
    > set_page_writeback() (happens e.g. in __block_write_full_page() called
    > from filesystem's writepage implementation).
    > unlock_page(page)
    >
    > So if you compute the checksum after set_page_writeback() is done in the
    > writepage() implementation (you cannot use __block_write_full_page() in
    > that case) and you call wait_on_page_writeback() in ext4_page_mkwrite()
    > under page lock, you should be safe. If you do all this and still see
    > errors, something is broken I'd say...

    Looking at the ext4_page_mkwrite, it does this:

    lock the page
    check for holes
    unlock the page
    if (no_holes)
    return;

    write_begin/write_end
    return

    So, to have page_mkwrite work, you need to wait for writeback with the
    page locked in both the no holes case and after the
    write_begin/write_end. write_begin will dirty the page, so someone can
    wander in and start the IO while we are still in page_mkwrite.

    This is untested and uncompiled, but it should
    do the trick.

    Jan, did you get rid of all the buffer head based writeback for
    data=ordered in ext4? That's my only other idea, that someone is doing
    writeback directly without taking the page lock.

    diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
    index 9f7f9e4..8a75e12 100644
    --- a/fs/ext4/inode.c
    +++ b/fs/ext4/inode.c
    @@ -5880,6 +5880,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
    if (page_has_buffers(page)) {
    if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL,
    ext4_bh_unmapped)) {
    + wait_on_page_writeback(page);
    unlock_page(page);
    goto out_unlock;
    }
    @@ -5901,6 +5902,16 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
    if (ret < 0)
    goto out_unlock;
    ret = 0;
    +
    + /*
    + * write_begin/end might have created a dirty page and someone
    + * could wander in and start the IO. Make sure that hasn't
    + * happened
    + */
    + lock_page(page);
    + wait_on_page_writeback(page);
    + unlock_page(page);
    +
    out_unlock:
    if (ret)
    ret = VM_FAULT_SIGBUS;


    \
     
     \ /
      Last update: 2011-03-21 15:33    [W:4.170 / U:0.372 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site