linux-ext4 - [tytso@....edu: Re: [PATCH 2/5] vfs: use writeback lists to provide lazytime one day timeout]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20141128183803.GB24586@google.com>
Date:	Fri, 28 Nov 2014 13:38:03 -0500
From:	Ted Ts'o <tytso@....edu>
To:	linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	tytso@....edu
Subject: [tytso@....edu: Re: [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 <tytso@....edu> -----

Date: Fri, 28 Nov 2014 13:26:23 -0500
From: Ted Ts'o <tytso@....edu>
To: Jan Kara <jack@...e.cz>
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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html