| Message ID | 1455680059-20126-3-git-send-email-ross.zwisler@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > >
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);
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> --- fs/ext2/inode.c | 2 +- fs/ext4/inode.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)