vfs: use writeback lists to provide lazytime one day timeout]
PATCH 2/5 vfs: use writeback lists to provide lazytime one day timeout]
Oops, sorry, responding from a different computer from what I normally use due to Thanksgiving.
- Ted
----- Forwarded message from Ted Ts'o <[email protected]> -----
Date: Fri, 28 Nov 2014 13:26:23 -0500
From: Ted Ts'o <[email protected]>
To: Jan Kara <[email protected]>
Subject: Re: [PATCH 2/5] vfs: use writeback lists to provide lazytime one day timeout
User-Agent: Mutt/1.5.21 (2010-09-15)
Quoted 4 lines · Jan Kara
Quoted 2 lines · Ted Tso
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.
Quoted 13 lines
Review
Quoted 12 lines
static void
queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work){(quoted diff) +6
-1int 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:
Quoted 7 lines
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.
Quoted 5 lines
Quoted 5 lines
(quoted diff) +5
-0+ /* + * 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
Quoted 8 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ * 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 -----