From: Ben Myers <bpm@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 8/8] xfs: add back fdatasync support
Date: Thu, 8 Mar 2012 14:04:34 -0600 [thread overview]
Message-ID: <20120308200434.GW7762@sgi.com> (raw)
In-Reply-To: <20120221003907.518904261@bombadil.infradead.org>
On Mon, Feb 20, 2012 at 07:38:32PM -0500, Christoph Hellwig wrote:
> Add an in-memory only flag to say we logged timestamps only, and use it to
> check if fdatasync can optimize away the log force.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok. I was a little concerned that xfs_inode_item_format doesn't
know what an XFS_ILOG_TIMESTAMP is, until it dawned that we always log
the inode core.
Reviewed-by: Ben Myers <bpm@sgi.com>
>
> ---
> fs/xfs/xfs_file.c | 7 +++++--
> fs/xfs/xfs_inode_item.c | 3 ++-
> fs/xfs/xfs_inode_item.h | 12 +++++++++++-
> fs/xfs/xfs_super.c | 2 +-
> 4 files changed, 19 insertions(+), 5 deletions(-)
>
> Index: xfs/fs/xfs/xfs_inode_item.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_inode_item.c 2012-02-20 12:08:44.379988903 -0800
> +++ xfs/fs/xfs/xfs_inode_item.c 2012-02-20 12:10:27.239988625 -0800
> @@ -439,7 +439,8 @@ out:
> * games in recovery easier, which isn't a big deal as just about any
> * transaction would dirty it anyway.
> */
> - iip->ili_format.ilf_fields = XFS_ILOG_CORE | iip->ili_fields;
> + iip->ili_format.ilf_fields = XFS_ILOG_CORE |
> + (iip->ili_fields & ~XFS_ILOG_TIMESTAMP);
> iip->ili_format.ilf_size = nvecs;
> }
>
> Index: xfs/fs/xfs/xfs_inode_item.h
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_inode_item.h 2012-02-20 12:08:44.383322236 -0800
> +++ xfs/fs/xfs/xfs_inode_item.h 2012-02-20 12:10:27.239988625 -0800
> @@ -86,6 +86,16 @@ typedef struct xfs_inode_log_format_64 {
> #define XFS_ILOG_AEXT 0x080 /* log i_af.if_extents */
> #define XFS_ILOG_ABROOT 0x100 /* log i_af.i_broot */
>
> +
> +/*
> + * The timestamps in the core are dirty, but not nessecarily anything
> + * else.
> + *
> + * This is an incore only value store in ilf_fields & co, which must
> + * never make it to disk, unlike the other fields above.
> + */
> +#define XFS_ILOG_TIMESTAMP 0x4000
> +
> #define XFS_ILOG_NONCORE (XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
> XFS_ILOG_DBROOT | XFS_ILOG_DEV | \
> XFS_ILOG_UUID | XFS_ILOG_ADATA | \
> @@ -101,7 +111,7 @@ typedef struct xfs_inode_log_format_64 {
> XFS_ILOG_DEXT | XFS_ILOG_DBROOT | \
> XFS_ILOG_DEV | XFS_ILOG_UUID | \
> XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
> - XFS_ILOG_ABROOT)
> + XFS_ILOG_ABROOT | XFS_ILOG_TIMESTAMP)
>
> static inline int xfs_ilog_fbroot(int w)
> {
> Index: xfs/fs/xfs/xfs_file.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_file.c 2012-02-20 12:08:35.513322261 -0800
> +++ xfs/fs/xfs/xfs_file.c 2012-02-20 12:11:40.246655094 -0800
> @@ -197,8 +197,11 @@ xfs_file_fsync(
> * to flush the log up to the latest LSN that touched the inode.
> */
> xfs_ilock(ip, XFS_ILOCK_SHARED);
> - if (xfs_ipincount(ip))
> - lsn = ip->i_itemp->ili_last_lsn;
> + if (xfs_ipincount(ip)) {
> + if (!datasync ||
> + (ip->i_itemp->ili_fields & ~XFS_ILOG_TIMESTAMP))
> + lsn = ip->i_itemp->ili_last_lsn;
> + }
> xfs_iunlock(ip, XFS_ILOCK_SHARED);
>
> if (lsn)
> Index: xfs/fs/xfs/xfs_super.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_super.c 2012-02-20 12:08:35.526655594 -0800
> +++ xfs/fs/xfs/xfs_super.c 2012-02-20 12:10:27.239988625 -0800
> @@ -907,7 +907,7 @@ xfs_fs_dirty_inode(
> ip->i_d.di_mtime.t_nsec = (__int32_t)inode->i_mtime.tv_nsec;
>
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> - xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
> error = xfs_trans_commit(tp, 0);
> if (error)
> goto trouble;
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
prev parent reply other threads:[~2012-03-08 20:04 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-21 0:38 [PATCH 0/8] log all inode updates V2 Christoph Hellwig
2012-02-21 0:38 ` [PATCH 1/8] xfs: use per-filesystem I/O completion workqueues Christoph Hellwig
2012-02-21 4:58 ` Dave Chinner
2012-02-28 16:13 ` Mark Tinguely
2012-02-21 0:38 ` [PATCH 2/8] xfs: do not require an ioend for new EOF calculation Christoph Hellwig
2012-02-28 16:14 ` Mark Tinguely
2012-02-21 0:38 ` [PATCH 6/8] xfs: make xfs_inode_item_size idempotent Christoph Hellwig
2012-02-21 5:14 ` Dave Chinner
2012-02-28 11:08 ` Christoph Hellwig
2012-02-21 0:38 ` [PATCH 7/8] xfs: split in-core and on-disk inode log item fields Christoph Hellwig
2012-02-21 5:18 ` Dave Chinner
2012-02-21 0:38 ` [PATCH 8/8] xfs: add back fdatasync support Christoph Hellwig
2012-02-21 5:23 ` Dave Chinner
2012-03-08 20:04 ` Ben Myers [this message]
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=20120308200434.GW7762@sgi.com \
--to=bpm@sgi.com \
--cc=hch@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox