From: "Theodore Ts'o" <tytso@mit.edu>
To: Jan Kara <jack@suse.cz>
Cc: Dave Chinner <david@fromorbit.com>,
Andreas Dilger <adilger@dilger.ca>,
Linux Filesystem Development List <linux-fsdevel@vger.kernel.org>,
Ext4 Developers List <linux-ext4@vger.kernel.org>,
Linux btrfs Developers List <linux-btrfs@vger.kernel.org>,
XFS Developers <xfs@oss.sgi.com>
Subject: Re: [PATCH-v4 6/7] ext4: add support for a lazytime mount option
Date: Thu, 27 Nov 2014 15:13:16 -0500 [thread overview]
Message-ID: <20141127201316.GE14091@thunk.org> (raw)
In-Reply-To: <20141127154159.GA11922@quack.suse.cz>
On Thu, Nov 27, 2014 at 04:41:59PM +0100, Jan Kara wrote:
> Hum, but this puts lots of stuff under inode_hash_lock, including
> writeback list lock. I don't like this too much. I understand that getting
> handle for each inode is rather more CPU intensive but it should still be a
> clear win over the current situation and avoids entangling locks like this.
Hmm, if we dropped the inode_requeue_dirtytime(), then we can avoid
taking the writeback list lock. The net result is that we would have
some inodes still on the b_dirty_time list that were no longer
I_DIRTY_TIME, but since I_DIRTY_TIME wouldn't be set, it's mostly
harmless since when we do iterate over the b_dirty_time list, those
inodes can be quickly identified and skipped over. (And if the inode
ever gets dirtied for real, then it will get moved onto the b_dirty
list and that will be that.)
The problem with getting a handle on the inode is not just that it is
more CPU intensive, but that can't let the iput_final() call happen
until after we have finished the transaction handle. We could keep a
linked list of inodes attached to the handle, and then only call iput
on them once ext4_journal_stop(handle) gets called, but that's a
complication I'd like to avoid if at all possible.
Being able to opporunistically write the timestamps when we are
journalling an inode table block is a pretty big win, so if we end up
extending the hold time on inode_hash_lock (only when we come across a
I_DIRTY_TIME inode that we can clear) a tiny bit, there will be a lot
of workloads where I think it's a worthwhile tradeoff. If we can
avoid entangling the writebakc list lock, does that make you happier
about this approach?
- Ted
WARNING: multiple messages have this Message-ID (diff)
From: Theodore Ts'o <tytso@mit.edu>
To: Jan Kara <jack@suse.cz>
Cc: Andreas Dilger <adilger@dilger.ca>,
XFS Developers <xfs@oss.sgi.com>,
Linux Filesystem Development List <linux-fsdevel@vger.kernel.org>,
Ext4 Developers List <linux-ext4@vger.kernel.org>,
Linux btrfs Developers List <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH-v4 6/7] ext4: add support for a lazytime mount option
Date: Thu, 27 Nov 2014 15:13:16 -0500 [thread overview]
Message-ID: <20141127201316.GE14091@thunk.org> (raw)
In-Reply-To: <20141127154159.GA11922@quack.suse.cz>
On Thu, Nov 27, 2014 at 04:41:59PM +0100, Jan Kara wrote:
> Hum, but this puts lots of stuff under inode_hash_lock, including
> writeback list lock. I don't like this too much. I understand that getting
> handle for each inode is rather more CPU intensive but it should still be a
> clear win over the current situation and avoids entangling locks like this.
Hmm, if we dropped the inode_requeue_dirtytime(), then we can avoid
taking the writeback list lock. The net result is that we would have
some inodes still on the b_dirty_time list that were no longer
I_DIRTY_TIME, but since I_DIRTY_TIME wouldn't be set, it's mostly
harmless since when we do iterate over the b_dirty_time list, those
inodes can be quickly identified and skipped over. (And if the inode
ever gets dirtied for real, then it will get moved onto the b_dirty
list and that will be that.)
The problem with getting a handle on the inode is not just that it is
more CPU intensive, but that can't let the iput_final() call happen
until after we have finished the transaction handle. We could keep a
linked list of inodes attached to the handle, and then only call iput
on them once ext4_journal_stop(handle) gets called, but that's a
complication I'd like to avoid if at all possible.
Being able to opporunistically write the timestamps when we are
journalling an inode table block is a pretty big win, so if we end up
extending the hold time on inode_hash_lock (only when we come across a
I_DIRTY_TIME inode that we can clear) a tiny bit, there will be a lot
of workloads where I think it's a worthwhile tradeoff. If we can
avoid entangling the writebakc list lock, does that make you happier
about this approach?
- Ted
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2014-11-27 20:13 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-26 10:23 [PATCH-v4 0/7] add support for a lazytime mount option Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
2014-11-26 10:23 ` [PATCH-v4 1/7] vfs: split update_time() into update_time() and write_time() Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
2014-11-26 19:23 ` Christoph Hellwig
2014-11-26 19:23 ` Christoph Hellwig
2014-11-27 12:34 ` Jan Kara
2014-11-27 12:34 ` Jan Kara
2014-11-27 15:25 ` Christoph Hellwig
2014-11-27 15:25 ` Christoph Hellwig
2014-11-27 14:41 ` Theodore Ts'o
2014-11-27 14:41 ` Theodore Ts'o
2014-11-27 15:28 ` Christoph Hellwig
2014-11-27 15:28 ` Christoph Hellwig
2014-11-27 15:33 ` Theodore Ts'o
2014-11-27 15:33 ` Theodore Ts'o
2014-11-27 16:49 ` Christoph Hellwig
2014-11-27 16:49 ` Christoph Hellwig
2014-11-27 20:27 ` Theodore Ts'o
2014-11-27 20:27 ` Theodore Ts'o
2014-12-01 9:28 ` Christoph Hellwig
2014-12-01 9:28 ` Christoph Hellwig
2014-12-01 15:04 ` Theodore Ts'o
2014-12-01 15:04 ` Theodore Ts'o
2014-12-01 17:18 ` David Sterba
2014-12-01 17:18 ` David Sterba
2014-12-02 9:20 ` Christoph Hellwig
2014-12-02 9:20 ` Christoph Hellwig
2014-12-02 15:09 ` Theodore Ts'o
2014-12-02 15:09 ` Theodore Ts'o
2014-11-26 10:23 ` [PATCH-v4 2/7] vfs: add support for a lazytime mount option Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
2014-11-27 13:14 ` Jan Kara
2014-11-27 13:14 ` Jan Kara
2014-11-27 20:19 ` Theodore Ts'o
2014-11-27 20:19 ` Theodore Ts'o
2014-11-28 12:41 ` Jan Kara
2014-11-28 12:41 ` Jan Kara
2014-11-27 23:00 ` Theodore Ts'o
2014-11-27 23:00 ` Theodore Ts'o
2014-11-28 5:36 ` Theodore Ts'o
2014-11-28 5:36 ` Theodore Ts'o
2014-11-28 16:24 ` Jan Kara
2014-11-28 16:24 ` Jan Kara
2014-11-26 10:23 ` [PATCH-v4 3/7] vfs: don't let the dirty time inodes get more than a day stale Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
2014-11-26 10:23 ` [PATCH-v4 4/7] vfs: add lazytime tracepoints for better debugging Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
2014-11-26 10:23 ` [PATCH-v4 5/7] vfs: add find_active_inode_nowait() function Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
2014-11-26 10:23 ` [PATCH-v4 6/7] ext4: add support for a lazytime mount option Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
2014-11-26 19:24 ` Christoph Hellwig
2014-11-26 19:24 ` Christoph Hellwig
2014-11-26 22:48 ` Dave Chinner
2014-11-26 22:48 ` Dave Chinner
2014-11-26 23:10 ` Andreas Dilger
2014-11-26 23:10 ` Andreas Dilger
2014-11-26 23:35 ` Dave Chinner
2014-11-26 23:35 ` Dave Chinner
2014-11-27 13:27 ` Jan Kara
2014-11-27 13:27 ` Jan Kara
2014-11-27 13:32 ` Jan Kara
2014-11-27 13:32 ` Jan Kara
2014-11-27 15:25 ` Theodore Ts'o
2014-11-27 15:25 ` Theodore Ts'o
2014-11-27 15:41 ` Jan Kara
2014-11-27 15:41 ` Jan Kara
2014-11-27 20:13 ` Theodore Ts'o [this message]
2014-11-27 20:13 ` Theodore Ts'o
2014-11-26 10:23 ` [PATCH-v4 7/7] btrfs: add an is_readonly() so btrfs can use common code for update_time() Theodore Ts'o
2014-11-26 10:23 ` Theodore Ts'o
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=20141127201316.GE14091@thunk.org \
--to=tytso@mit.edu \
--cc=adilger@dilger.ca \
--cc=david@fromorbit.com \
--cc=jack@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--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.