lkml.org 
[lkml]   [2009]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] writeback: guard against jiffies wraparound on inode->dirtied_when checks (try #2)
    On Wed, Apr 01, 2009 at 08:48:43PM +0800, Jeff Layton wrote:
    > > > > --- mm.orig/fs/fs-writeback.c
    > > > > +++ mm/fs/fs-writeback.c
    > > > > @@ -196,7 +196,7 @@ static void redirty_tail(struct inode *i
    > > > > struct inode *tail_inode;
    > > > >
    > > > > tail_inode = list_entry(sb->s_dirty.next, struct inode, i_list);
    > > > > - if (!time_after_eq(inode->dirtied_when,
    > > > > + if (time_before(inode->dirtied_when,
    > > > > tail_inode->dirtied_when))
    > > > > inode->dirtied_when = jiffies;
    > > > > }
    > > >
    > > > I think we need a similar change in this function in order to maintain
    > > > the list order.
    > > >
    > > > Consider this case:
    > > >
    > > > We have an s_dirty list with a head inode that appears to be in the
    > > > future. We start writeback and clear out s_dirty (all of the inodes are
    > > > moved to s_io). A new inode is dirtied, and goes onto the empty s_dirty
    > > > list with a dirtied_when value that equals now. The inode with the
    > > > dirtied_when value that looks like it's in the future is redirtied while
    > > > being written and redirty_tail is called. It goes back on the list
    > > > without resetting dirtied_when even though it's actually older than the
    > > > inode at the tail.
    > >
    > > What's the difference? It _is_ the past because all 2 reference sites
    > > are now taught to think so.
    > >
    > > So s_dirty is still in order, and the writeback process won't be blocked.
    > >
    >
    > Sanity check -- my understanding is this:
    >
    > head == least-recently dirtied inode
    > tail == most-recently dirtied inode
    >
    > ...if so, then we are violating the list order if we don't make a
    > change to redirty_tail. We're putting an inode that's far in the past
    > back onto the tail of the list without resetting dirtied_when. A more
    > recently-dirtied inode will precede one that was dirtied less recently.
    >
    > Since the newly dirtied inode is closer to the head of the list, the
    > older inode that's constantly being redirtied won't be written out
    > until the newly dirtied one passes the older_than_this check (30s or
    > so in the usual case).

    If you call that out-of-order, yes it is. Sadly it cannot be improved
    by playing with dirtied_when: the _physical_ order is still the same.

    You know what? That's exactly the drawback of redirtying into s_dirty.
    It's irrelevant to the resetting of dirtied_when. A new s_more_io_wait
    is the only way to solve this problem.

    > > > There is another option too that I'll throw out here...
    > > >
    > > > We could just make dirtied_when a 64 bit value on 32 bit machines and
    > > > use jiffies_64 there. On the upside there is no "problematic
    > > > window" with that. The downside is that struct inode would grow by 4
    > > > bytes on 32 bit arches, and checking jiffies_64 on such an arch is
    > > > more computationally intensive. We'd also have to change the size of
    > > > older_than_this value in the writeback_control struct too if we want to
    > > > go this route...
    > >
    > > Yes that could eliminate the 30s or more temporary writeback stillness.
    > > The only problem is the extra costs for normal cases, especially the
    > > space cost.
    > >
    >
    > Correct. I'm not necessarily advocating that approach but it's one to
    > consider...
    >
    > If your s_more_io_wait patchset comes to fruition though then that
    > change really won't be needed, so maybe it's best not to go that route.

    Sorry for the delay. But I'm still curious about the redirty
    process&timing of NFS/XFS. The conflicts with Jens' per-bdi pdflush
    patches are another concern...

    Thanks,
    Fengguang


    \
     
     \ /
      Last update: 2009-04-01 15:13    [W:0.028 / U:64.408 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site