From: Jan Kara <jack@suse.cz>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: linux-kernel@vger.kernel.org,
"J. Bruce Fields" <bfields@fieldses.org>,
Theodore Ts'o <tytso@mit.edu>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Andreas Dilger <adilger.kernel@dilger.ca>,
Andrew Morton <akpm@linux-foundation.org>,
Dan Williams <dan.j.williams@intel.com>,
Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.com>,
Jeff Layton <jlayton@poochiereds.net>,
Jens Axboe <axboe@kernel.dk>,
Matthew Wilcox <willy@linux.intel.com>,
linux-block@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-nvdimm@lists.01.org, xfs@oss.sgi.com
Subject: Re: [PATCH v3 2/6] ext2, ext4: only set S_DAX for regular inodes
Date: Wed, 17 Feb 2016 22:33:25 +0100 [thread overview]
Message-ID: <20160217213325.GH14140@quack.suse.cz> (raw)
In-Reply-To: <1455680059-20126-3-git-send-email-ross.zwisler@linux.intel.com>
On Tue 16-02-16 20:34:15, Ross Zwisler wrote:
> When S_DAX is set on an inode we assume that if there are pages attached
> to the mapping (mapping->nrpages != 0), those pages are clean zero pages
> that were used to service reads from holes. Any dirty data associated with
> the inode should be in the form of DAX exceptional entries
> (mapping->nrexceptional) that is written back via
> dax_writeback_mapping_range().
>
> With the current code, though, this isn't always true. For example, ext2
> and ext4 directory inodes can have S_DAX set, but have their dirty data
> stored as dirty page cache entries. For these types of inodes, having
> S_DAX set doesn't really make sense since their I/O doesn't actually happen
> through the DAX code path.
>
> Instead, only allow S_DAX to be set for regular inodes for ext2 and ext4.
> This allows us to have strict DAX vs non-DAX paths in the writeback code.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext2/inode.c | 2 +-
> fs/ext4/inode.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 338eefd..27e2cdd 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1296,7 +1296,7 @@ void ext2_set_inode_flags(struct inode *inode)
> inode->i_flags |= S_NOATIME;
> if (flags & EXT2_DIRSYNC_FL)
> inode->i_flags |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> inode->i_flags |= S_DAX;
> }
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 83bc8bf..7088aa5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4127,7 +4127,7 @@ void ext4_set_inode_flags(struct inode *inode)
> new_fl |= S_NOATIME;
> if (flags & EXT4_DIRSYNC_FL)
> new_fl |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> new_fl |= S_DAX;
> inode_set_flags(inode, new_fl,
> S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_DAX);
> --
> 2.5.0
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: linux-kernel@vger.kernel.org,
"J. Bruce Fields" <bfields@fieldses.org>,
"Theodore Ts'o" <tytso@mit.edu>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Andreas Dilger <adilger.kernel@dilger.ca>,
Andrew Morton <akpm@linux-foundation.org>,
Dan Williams <dan.j.williams@intel.com>,
Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.com>,
Jeff Layton <jlayton@poochiereds.net>,
Jens Axboe <axboe@kernel.dk>,
Matthew Wilcox <willy@linux.intel.com>,
linux-block@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-nvdimm@ml01.01.org, xfs@oss.sgi.com
Subject: Re: [PATCH v3 2/6] ext2, ext4: only set S_DAX for regular inodes
Date: Wed, 17 Feb 2016 22:33:25 +0100 [thread overview]
Message-ID: <20160217213325.GH14140@quack.suse.cz> (raw)
In-Reply-To: <1455680059-20126-3-git-send-email-ross.zwisler@linux.intel.com>
On Tue 16-02-16 20:34:15, Ross Zwisler wrote:
> When S_DAX is set on an inode we assume that if there are pages attached
> to the mapping (mapping->nrpages != 0), those pages are clean zero pages
> that were used to service reads from holes. Any dirty data associated with
> the inode should be in the form of DAX exceptional entries
> (mapping->nrexceptional) that is written back via
> dax_writeback_mapping_range().
>
> With the current code, though, this isn't always true. For example, ext2
> and ext4 directory inodes can have S_DAX set, but have their dirty data
> stored as dirty page cache entries. For these types of inodes, having
> S_DAX set doesn't really make sense since their I/O doesn't actually happen
> through the DAX code path.
>
> Instead, only allow S_DAX to be set for regular inodes for ext2 and ext4.
> This allows us to have strict DAX vs non-DAX paths in the writeback code.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext2/inode.c | 2 +-
> fs/ext4/inode.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 338eefd..27e2cdd 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1296,7 +1296,7 @@ void ext2_set_inode_flags(struct inode *inode)
> inode->i_flags |= S_NOATIME;
> if (flags & EXT2_DIRSYNC_FL)
> inode->i_flags |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> inode->i_flags |= S_DAX;
> }
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 83bc8bf..7088aa5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4127,7 +4127,7 @@ void ext4_set_inode_flags(struct inode *inode)
> new_fl |= S_NOATIME;
> if (flags & EXT4_DIRSYNC_FL)
> new_fl |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> new_fl |= S_DAX;
> inode_set_flags(inode, new_fl,
> S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_DAX);
> --
> 2.5.0
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: linux-kernel@vger.kernel.org,
"J. Bruce Fields" <bfields@fieldses.org>,
Theodore Ts'o <tytso@mit.edu>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Andreas Dilger <adilger.kernel@dilger.ca>,
Andrew Morton <akpm@linux-foundation.org>,
Dan Williams <dan.j.williams@intel.com>,
Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.com>,
Jeff Layton <jlayton@poochiereds.net>,
Jens Axboe <axboe@kernel.dk>,
Matthew Wilcox <willy@linux.intel.com>,
linux-block@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-nvdimm@lists.01.org, xfs@oss.sgi.com
Subject: Re: [PATCH v3 2/6] ext2, ext4: only set S_DAX for regular inodes
Date: Wed, 17 Feb 2016 22:33:25 +0100 [thread overview]
Message-ID: <20160217213325.GH14140@quack.suse.cz> (raw)
In-Reply-To: <1455680059-20126-3-git-send-email-ross.zwisler@linux.intel.com>
On Tue 16-02-16 20:34:15, Ross Zwisler wrote:
> When S_DAX is set on an inode we assume that if there are pages attached
> to the mapping (mapping->nrpages != 0), those pages are clean zero pages
> that were used to service reads from holes. Any dirty data associated with
> the inode should be in the form of DAX exceptional entries
> (mapping->nrexceptional) that is written back via
> dax_writeback_mapping_range().
>
> With the current code, though, this isn't always true. For example, ext2
> and ext4 directory inodes can have S_DAX set, but have their dirty data
> stored as dirty page cache entries. For these types of inodes, having
> S_DAX set doesn't really make sense since their I/O doesn't actually happen
> through the DAX code path.
>
> Instead, only allow S_DAX to be set for regular inodes for ext2 and ext4.
> This allows us to have strict DAX vs non-DAX paths in the writeback code.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext2/inode.c | 2 +-
> fs/ext4/inode.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 338eefd..27e2cdd 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1296,7 +1296,7 @@ void ext2_set_inode_flags(struct inode *inode)
> inode->i_flags |= S_NOATIME;
> if (flags & EXT2_DIRSYNC_FL)
> inode->i_flags |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> inode->i_flags |= S_DAX;
> }
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 83bc8bf..7088aa5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4127,7 +4127,7 @@ void ext4_set_inode_flags(struct inode *inode)
> new_fl |= S_NOATIME;
> if (flags & EXT4_DIRSYNC_FL)
> new_fl |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> new_fl |= S_DAX;
> inode_set_flags(inode, new_fl,
> S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_DAX);
> --
> 2.5.0
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Jens Axboe <axboe@kernel.dk>,
Jeff Layton <jlayton@poochiereds.net>,
Theodore Ts'o <tytso@mit.edu>,
linux-nvdimm@lists.01.org, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, xfs@oss.sgi.com,
"J. Bruce Fields" <bfields@fieldses.org>,
linux-mm@kvack.org, Andreas Dilger <adilger.kernel@dilger.ca>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Jan Kara <jack@suse.com>,
linux-fsdevel@vger.kernel.org,
Matthew Wilcox <willy@linux.intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-ext4@vger.kernel.org,
Dan Williams <dan.j.williams@intel.com>
Subject: Re: [PATCH v3 2/6] ext2, ext4: only set S_DAX for regular inodes
Date: Wed, 17 Feb 2016 22:33:25 +0100 [thread overview]
Message-ID: <20160217213325.GH14140@quack.suse.cz> (raw)
In-Reply-To: <1455680059-20126-3-git-send-email-ross.zwisler@linux.intel.com>
On Tue 16-02-16 20:34:15, Ross Zwisler wrote:
> When S_DAX is set on an inode we assume that if there are pages attached
> to the mapping (mapping->nrpages != 0), those pages are clean zero pages
> that were used to service reads from holes. Any dirty data associated with
> the inode should be in the form of DAX exceptional entries
> (mapping->nrexceptional) that is written back via
> dax_writeback_mapping_range().
>
> With the current code, though, this isn't always true. For example, ext2
> and ext4 directory inodes can have S_DAX set, but have their dirty data
> stored as dirty page cache entries. For these types of inodes, having
> S_DAX set doesn't really make sense since their I/O doesn't actually happen
> through the DAX code path.
>
> Instead, only allow S_DAX to be set for regular inodes for ext2 and ext4.
> This allows us to have strict DAX vs non-DAX paths in the writeback code.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext2/inode.c | 2 +-
> fs/ext4/inode.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 338eefd..27e2cdd 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1296,7 +1296,7 @@ void ext2_set_inode_flags(struct inode *inode)
> inode->i_flags |= S_NOATIME;
> if (flags & EXT2_DIRSYNC_FL)
> inode->i_flags |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> inode->i_flags |= S_DAX;
> }
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 83bc8bf..7088aa5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4127,7 +4127,7 @@ void ext4_set_inode_flags(struct inode *inode)
> new_fl |= S_NOATIME;
> if (flags & EXT4_DIRSYNC_FL)
> new_fl |= S_DIRSYNC;
> - if (test_opt(inode->i_sb, DAX))
> + if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
> new_fl |= S_DAX;
> inode_set_flags(inode, new_fl,
> S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_DAX);
> --
> 2.5.0
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2016-02-17 21:33 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-17 3:34 [PATCH v3 0/6] DAX fixes, move flushing calls to FS Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` [PATCH v3 1/6] block: disable block device DAX by default Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 21:55 ` Jan Kara
2016-02-17 21:55 ` Jan Kara
2016-02-17 21:55 ` Jan Kara
2016-02-17 21:55 ` Jan Kara
2016-02-17 3:34 ` [PATCH v3 2/6] ext2, ext4: only set S_DAX for regular inodes Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 21:33 ` Jan Kara [this message]
2016-02-17 21:33 ` Jan Kara
2016-02-17 21:33 ` Jan Kara
2016-02-17 21:33 ` Jan Kara
2016-02-17 3:34 ` [PATCH v3 3/6] ext4: Online defrag not supported with DAX Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 21:34 ` Jan Kara
2016-02-17 21:34 ` Jan Kara
2016-02-17 21:34 ` Jan Kara
2016-02-17 21:50 ` Ross Zwisler
2016-02-17 21:50 ` Ross Zwisler
2016-02-17 21:50 ` Ross Zwisler
2016-02-17 22:10 ` Jan Kara
2016-02-17 22:10 ` Jan Kara
2016-02-17 22:10 ` Jan Kara
2016-02-17 22:10 ` Jan Kara
2016-02-18 0:12 ` Dave Chinner
2016-02-18 0:12 ` Dave Chinner
2016-02-18 0:12 ` Dave Chinner
2016-02-18 0:12 ` Dave Chinner
2016-02-17 3:34 ` [PATCH v3 4/6] dax: give DAX clearing code correct bdev Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 21:37 ` Jan Kara
2016-02-17 21:37 ` Jan Kara
2016-02-17 21:37 ` Jan Kara
2016-02-17 3:34 ` [PATCH v3 5/6] dax: move writeback calls into the filesystems Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` [PATCH v3 6/6] block: use dax_do_io() if blkdev_dax_capable() Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 3:34 ` Ross Zwisler
2016-02-17 21:54 ` Jan Kara
2016-02-17 21:54 ` Jan Kara
2016-02-17 21:54 ` Jan Kara
2016-02-17 21:54 ` Jan Kara
2016-02-17 22:18 ` Dan Williams
2016-02-17 22:18 ` Dan Williams
2016-02-17 22:18 ` Dan Williams
2016-02-17 22:18 ` Dan Williams
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=20160217213325.GH14140@quack.suse.cz \
--to=jack@suse.cz \
--cc=adilger.kernel@dilger.ca \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=bfields@fieldses.org \
--cc=dan.j.williams@intel.com \
--cc=david@fromorbit.com \
--cc=jack@suse.com \
--cc=jlayton@poochiereds.net \
--cc=linux-block@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nvdimm@lists.01.org \
--cc=ross.zwisler@linux.intel.com \
--cc=tytso@mit.edu \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@linux.intel.com \
--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.