All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "Theodore Ts'o" <tytso@mit.edu>
Cc: linux-fsdevel@vger.kernel.org,
	Ext4 Developers List <linux-ext4@vger.kernel.org>,
	xfs@oss.sgi.com, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 3/4] vfs: don't let the dirty time inodes get more than a day stale
Date: Thu, 27 Nov 2014 09:39:01 +1100	[thread overview]
Message-ID: <20141126223901.GF9561@dastard> (raw)
In-Reply-To: <20141126102017.GJ28449@thunk.org>

On Wed, Nov 26, 2014 at 05:20:17AM -0500, Theodore Ts'o wrote:
> On Wed, Nov 26, 2014 at 10:48:51AM +1100, Dave Chinner wrote:
> > No abuse necessary at all. Just a different inode_dirtied_after()
> > check is requires if the inode is on the time dirty list in
> > move_expired_inodes().
> 
> I'm still not sure what you have in mind here.  When would this be
> checked? 

Have you looked at where move_expired_inodes() gets called from?
It's called periodically from background writeback by queue_io(),
and sync uses the same infrastructure to expire all inodes on the
dirty list....

> It sounds like you want to set a timeout such that when an
> inode which had its timestamps updated lazily 24 hours earlier, the
> inode would get written out.  Yes?  But that implies something is
> going to have to scan the list of inodes on the dirty time list
> periodically.  When are you proposing that this take place?

The writeback code already does this for dirty inodes. it does it in
move_expired_inodes() to move the inodes with i_dirtied_when is
older than 30s. It's *trivial* to add a time dirty inode list and
scan that at the same time to pull off inodes that are older than
24hrs.

> The various approaches that come to mind all seem more complex than
> what I have in this patch 3 of 4, and I'm not sure it's worth the
> complexity.

the "once a day" stuff you've added is a horrible, nasty hack. I
wasn't going to say anything about it (i.e. if you can't say
anything nice...). The existing dirty inode writeback expiry code
does *everything* we need already, we just need to plumb in a new
list and add an expiry check of that list to move inodes to the b_io
list when they have been timestamp dirty for more than 24 hours...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: Theodore Ts'o <tytso@mit.edu>
Cc: linux-fsdevel@vger.kernel.org,
	Ext4 Developers List <linux-ext4@vger.kernel.org>,
	linux-btrfs@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: [PATCH 3/4] vfs: don't let the dirty time inodes get more than a day stale
Date: Thu, 27 Nov 2014 09:39:01 +1100	[thread overview]
Message-ID: <20141126223901.GF9561@dastard> (raw)
In-Reply-To: <20141126102017.GJ28449@thunk.org>

On Wed, Nov 26, 2014 at 05:20:17AM -0500, Theodore Ts'o wrote:
> On Wed, Nov 26, 2014 at 10:48:51AM +1100, Dave Chinner wrote:
> > No abuse necessary at all. Just a different inode_dirtied_after()
> > check is requires if the inode is on the time dirty list in
> > move_expired_inodes().
> 
> I'm still not sure what you have in mind here.  When would this be
> checked? 

Have you looked at where move_expired_inodes() gets called from?
It's called periodically from background writeback by queue_io(),
and sync uses the same infrastructure to expire all inodes on the
dirty list....

> It sounds like you want to set a timeout such that when an
> inode which had its timestamps updated lazily 24 hours earlier, the
> inode would get written out.  Yes?  But that implies something is
> going to have to scan the list of inodes on the dirty time list
> periodically.  When are you proposing that this take place?

The writeback code already does this for dirty inodes. it does it in
move_expired_inodes() to move the inodes with i_dirtied_when is
older than 30s. It's *trivial* to add a time dirty inode list and
scan that at the same time to pull off inodes that are older than
24hrs.

> The various approaches that come to mind all seem more complex than
> what I have in this patch 3 of 4, and I'm not sure it's worth the
> complexity.

the "once a day" stuff you've added is a horrible, nasty hack. I
wasn't going to say anything about it (i.e. if you can't say
anything nice...). The existing dirty inode writeback expiry code
does *everything* we need already, we just need to plumb in a new
list and add an expiry check of that list to move inodes to the b_io
list when they have been timestamp dirty for more than 24 hours...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2014-11-26 22:39 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-21 19:59 [PATCH 0/4] add support for a lazytime mount option Theodore Ts'o
2014-11-21 19:59 ` Theodore Ts'o
2014-11-21 19:59 ` [PATCH 1/4] fs: split update_time() into update_time() and write_time() Theodore Ts'o
2014-11-21 19:59   ` Theodore Ts'o
2014-11-21 20:08   ` Chris Mason
2014-11-21 20:08     ` Chris Mason
2014-11-21 21:42     ` Theodore Ts'o
2014-11-21 21:42       ` Theodore Ts'o
2014-11-24 16:38       ` David Sterba
2014-11-24 16:38         ` David Sterba
2014-11-24 17:22         ` Theodore Ts'o
2014-11-24 17:22           ` Theodore Ts'o
2014-11-24 18:09           ` David Sterba
2014-11-24 18:09             ` David Sterba
2014-11-24 15:21   ` Christoph Hellwig
2014-11-24 15:21     ` Christoph Hellwig
2014-11-24 15:56     ` Theodore Ts'o
2014-11-24 15:56       ` Theodore Ts'o
2014-11-24 17:34     ` David Sterba
2014-11-24 17:34       ` David Sterba
2014-11-25 15:51       ` David Sterba
2014-11-25 15:51         ` David Sterba
2014-11-25 17:01         ` Christoph Hellwig
2014-11-21 19:59 ` [PATCH 2/4] vfs: add support for a lazytime mount option Theodore Ts'o
2014-11-21 19:59   ` Theodore Ts'o
2014-11-25  1:52   ` Dave Chinner
2014-11-25  1:52     ` Dave Chinner
2014-11-25  4:33     ` Theodore Ts'o
2014-11-25  4:33       ` Theodore Ts'o
2014-11-25 15:32       ` Boaz Harrosh
2014-11-25 15:32         ` Boaz Harrosh
2014-11-25 17:19       ` Jan Kara
2014-11-25 17:19         ` Jan Kara
2014-11-25 17:57         ` Theodore Ts'o
2014-11-25 17:57           ` Theodore Ts'o
2014-11-25 20:18           ` Jan Kara
2014-11-25 20:18             ` Jan Kara
2014-11-25 17:30       ` Jan Kara
2014-11-25 17:30         ` Jan Kara
2014-11-25 19:26         ` Theodore Ts'o
2014-11-25 19:26           ` Theodore Ts'o
2014-11-26  0:24       ` Dave Chinner
2014-11-26  0:24         ` Dave Chinner
2014-11-21 19:59 ` [PATCH 3/4] vfs: don't let the dirty time inodes get more than a day stale Theodore Ts'o
2014-11-21 19:59   ` Theodore Ts'o
2014-11-21 20:19   ` Andreas Dilger
2014-11-21 20:19     ` Andreas Dilger
2014-11-21 21:36     ` Theodore Ts'o
2014-11-21 21:36       ` Theodore Ts'o
2014-11-21 23:09       ` Andreas Dilger
2014-11-25  1:53   ` Dave Chinner
2014-11-25  1:53     ` Dave Chinner
2014-11-25  4:45     ` Theodore Ts'o
2014-11-25  4:45       ` Theodore Ts'o
2014-11-25 23:48       ` Dave Chinner
2014-11-25 23:48         ` Dave Chinner
2014-11-26 10:20         ` Theodore Ts'o
2014-11-26 10:20           ` Theodore Ts'o
2014-11-26 22:39           ` Dave Chinner [this message]
2014-11-26 22:39             ` Dave Chinner
2014-11-25 17:31   ` Jan Kara
2014-11-25 17:31     ` Jan Kara
2014-11-21 19:59 ` [PATCH 4/4] ext4: add support for a lazytime mount option Theodore Ts'o
2014-11-21 19:59   ` Theodore Ts'o
2014-11-25 17:34   ` Jan Kara
2014-11-25 17:34     ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141126223901.GF9561@dastard \
    --to=david@fromorbit.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.