Oops, sorry, responding from a different computer from what I normally
use due to Thanksgiving.
- Ted
----- Forwarded message from Ted Ts'o <tytso@xxxxxxx> -----
Date: Fri, 28 Nov 2014 13:26:23 -0500
From: Ted Ts'o <tytso@xxxxxxx>
To: Jan Kara <jack@xxxxxxx>
Subject: Re: [PATCH 2/5] vfs: use writeback lists to provide lazytime one day timeout
User-Agent: Mutt/1.5.21 (2010-09-15)
On Fri, Nov 28, 2014 at 06:20:46PM +0100, Jan Kara wrote:
> On Fri 28-11-14 01:00:08, Ted Tso wrote:
> > Queue inodes with dirty timestamps for writeback 24 hours after they
> > were initially dirtied.
> Oh I see, this patch should probably replace the other 2/5 patch.
Yes, I kept the patches separate so I could understand how much work
it would be to do things the other way and I forgot to rm -rf the
directory before I reran "git format-patch". Sorry for the confusion.
> > static void queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work)
> > {
> > int moved;
> > + unsigned long one_day_later = jiffies + (HZ * 86400);
> > +
> > assert_spin_locked(&wb->list_lock);
> > list_splice_init(&wb->b_more_io, &wb->b_io);
> > - moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, work);
> > + moved = move_expired_inodes(&wb->b_dirty, &wb->b_io,
> > + work->older_than_this);
> > + moved += move_expired_inodes(&wb->b_dirty_time, &wb->b_io,
> > + &one_day_later);
> I'd change this to:
> if (work->sync_mode == WB_SYNC_ALL) {
> moved += move_expired_inodes(&wb->b_dirty_time, &wb->b_io,
> work->older_than_this);
> } else {
> moved += move_expired_inodes(&wb->b_dirty_time, &wb->b_io,
> &one_day_later);
> }
That actually doesn't work because fs/sync.c uses WB_SYNC_NONE and
then waits for all of the inodes in a separate pass. But I can key
off of WB_REASON_SYNC, as I mentioned.
> > + /*
> > + * If the inode is marked dirty time but is not dirty,
> > + * then at last for ext3 and ext4 we need to call
> > + * mark_inode_dirty_sync in order to get the inode
> > + * timestamp transferred to the on disk inode, since
> > + * write_inode is a no-op for those file systems.
> > + */
> > + if ((inode->i_state & I_DIRTY_TIME) &&
> > + ((inode->i_state & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) == 0))
> > + mark_inode_dirty_sync(inode);
> > +
> This isn't necessary - you already handle this in
> __writeback_single_inode(). Or am I missing something?
Yes, good point, this can get dropped.
- Ted
----- End forwarded message -----
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
