lkml.org 
[lkml]   [2011]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/3] writeback: introduce wbc.for_sync to cover the two sync stages
    On Sun, May 01, 2011 at 03:46:04PM +0800, Dave Chinner wrote:
    > On Sun, May 01, 2011 at 06:36:06AM +0800, Wu Fengguang wrote:
    > > sync(2) is performed in two stages: the WB_SYNC_NONE sync and the
    > > WB_SYNC_ALL sync. Tag both stages with wbc.for_sync for livelock
    > > prevention.
    > >
    > > Note that writeback_inodes_sb() is called by not only sync(), they
    > > are treated the same because the other callers need also need livelock
    > > prevention.
    > >
    > > Impacts:
    > >
    > > - it changes the order in which pages/inodes are synced to disk. Now in
    > > the WB_SYNC_NONE stage, it won't proceed to write the next inode until
    > > finished with the current inode.
    > >
    > > - this adds a new field to the writeback trace events and may possibly
    > > break some scripts.
    > .....
    > > --- linux-next.orig/mm/page-writeback.c 2011-05-01 06:35:16.000000000 +0800
    > > +++ linux-next/mm/page-writeback.c 2011-05-01 06:35:17.000000000 +0800
    > > @@ -892,12 +892,12 @@ int write_cache_pages(struct address_spa
    > > range_whole = 1;
    > > cycled = 1; /* ignore range_cyclic tests */
    > > }
    > > - if (wbc->sync_mode == WB_SYNC_ALL)
    > > + if (wbc->for_sync)
    > > tag = PAGECACHE_TAG_TOWRITE;
    > > else
    > > tag = PAGECACHE_TAG_DIRTY;
    > > retry:
    > > - if (wbc->sync_mode == WB_SYNC_ALL)
    > > + if (wbc->for_sync)
    > > tag_pages_for_writeback(mapping, index, end);
    > > done_index = index;
    > > while (!done && (index <= end)) {
    >
    > Doesn't that break anything that uses
    > filemap_write_and_wait{_range}() or filemap_fdatawrite{_range}()?
    > e.g. fsync, sync buffered writes, etc? i.e. everything that
    > currently relies on WB_SYNC_ALL for data integrity writeback is now
    > b0rken except for sync(1)?

    Right, they'll become livelockable.. Good catch, thanks! I'll update
    the patches to do

    - if (wbc->sync_mode == WB_SYNC_ALL)
    + if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_sync)

    The alternative is to ask the other WB_SYNC_ALL callers to set
    wbc.tagged_sync, but that seems more error prone.

    Thanks,
    Fengguang


    \
     
     \ /
      Last update: 2011-05-02 05:27    [W:0.022 / U:154.084 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site