diff mbox series

[RFC,7/8] fs/ext4: Only change S_DAX on inode load

Message ID 20200414040030.1802884-8-ira.weiny@intel.com (mailing list archive)
State Deferred, archived
Headers show
Series Enable ext4 support for per-file/directory DAX operations | expand

Commit Message

Ira Weiny April 14, 2020, 4 a.m. UTC 
From: Ira Weiny <ira.weiny@intel.com>

To prevent complications with in memory inodes we only set S_DAX on
inode load.  FS_XFLAG_DAX can be changed at any time and S_DAX will
change after inode eviction and reload.

Add init bool to ext4_set_inode_flags() to indicate if the inode is
being newly initialized.

Assert that S_DAX is not set on an inode which is just being loaded.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 fs/ext4/ext4.h   | 2 +-
 fs/ext4/ialloc.c | 2 +-
 fs/ext4/inode.c  | 8 +++++---
 fs/ext4/ioctl.c  | 3 ++-
 fs/ext4/super.c  | 4 ++--
 fs/ext4/verity.c | 2 +-
 6 files changed, 12 insertions(+), 9 deletions(-)

Comments

Jan Kara April 15, 2020, 2:03 p.m. UTC | #1 
On Mon 13-04-20 21:00:29, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> To prevent complications with in memory inodes we only set S_DAX on
> inode load.  FS_XFLAG_DAX can be changed at any time and S_DAX will
> change after inode eviction and reload.
> 
> Add init bool to ext4_set_inode_flags() to indicate if the inode is
> being newly initialized.
> 
> Assert that S_DAX is not set on an inode which is just being loaded.

> @@ -4408,11 +4408,13 @@ static bool ext4_enable_dax(struct inode *inode)
>  	return (flags & EXT4_DAX_FL) == EXT4_DAX_FL;
>  }
>  
> -void ext4_set_inode_flags(struct inode *inode)
> +void ext4_set_inode_flags(struct inode *inode, bool init)
>  {
>  	unsigned int flags = EXT4_I(inode)->i_flags;
>  	unsigned int new_fl = 0;
>  
> +	J_ASSERT(!(IS_DAX(inode) && init));
> +

WARN_ON or BUG_ON here? J_ASSERT is for journalling assertions...

Otherwise the patch looks good.

								Honza
Ira Weiny April 17, 2020, 5:18 p.m. UTC | #2 
On Wed, Apr 15, 2020 at 04:03:08PM +0200, Jan Kara wrote:
> On Mon 13-04-20 21:00:29, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > To prevent complications with in memory inodes we only set S_DAX on
> > inode load.  FS_XFLAG_DAX can be changed at any time and S_DAX will
> > change after inode eviction and reload.
> > 
> > Add init bool to ext4_set_inode_flags() to indicate if the inode is
> > being newly initialized.
> > 
> > Assert that S_DAX is not set on an inode which is just being loaded.
> 
> > @@ -4408,11 +4408,13 @@ static bool ext4_enable_dax(struct inode *inode)
> >  	return (flags & EXT4_DAX_FL) == EXT4_DAX_FL;
> >  }
> >  
> > -void ext4_set_inode_flags(struct inode *inode)
> > +void ext4_set_inode_flags(struct inode *inode, bool init)
> >  {
> >  	unsigned int flags = EXT4_I(inode)->i_flags;
> >  	unsigned int new_fl = 0;
> >  
> > +	J_ASSERT(!(IS_DAX(inode) && init));
> > +
> 
> WARN_ON or BUG_ON here? J_ASSERT is for journalling assertions...

Ah sorry, did not realize that J_ was specific.

Changed to WARN_ON_ONCE()

Ira

> 
> Otherwise the patch looks good.
> 
> 								Honza
> 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
diff mbox series

Patch

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index aadf33d5b37d..7f9234cce5b8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2699,7 +2699,7 @@  extern int ext4_can_truncate(struct inode *inode);
 extern int ext4_truncate(struct inode *);
 extern int ext4_break_layouts(struct inode *);
 extern int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length);
-extern void ext4_set_inode_flags(struct inode *);
+extern void ext4_set_inode_flags(struct inode *, bool init);
 extern int ext4_alloc_da_blocks(struct inode *inode);
 extern void ext4_set_aops(struct inode *inode);
 extern int ext4_writepage_trans_blocks(struct inode *);
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index f95ee99091e4..1519a4bf42fa 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1104,7 +1104,7 @@  struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
 	ei->i_block_group = group;
 	ei->i_last_alloc_group = ~0;
 
-	ext4_set_inode_flags(inode);
+	ext4_set_inode_flags(inode, true);
 	if (IS_DIRSYNC(inode))
 		ext4_handle_sync(handle);
 	if (insert_inode_locked(inode) < 0) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e9d582e516bc..e9dfdfd6c698 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4408,11 +4408,13 @@  static bool ext4_enable_dax(struct inode *inode)
 	return (flags & EXT4_DAX_FL) == EXT4_DAX_FL;
 }
 
-void ext4_set_inode_flags(struct inode *inode)
+void ext4_set_inode_flags(struct inode *inode, bool init)
 {
 	unsigned int flags = EXT4_I(inode)->i_flags;
 	unsigned int new_fl = 0;
 
+	J_ASSERT(!(IS_DAX(inode) && init));
+
 	if (flags & EXT4_SYNC_FL)
 		new_fl |= S_SYNC;
 	if (flags & EXT4_APPEND_FL)
@@ -4423,7 +4425,7 @@  void ext4_set_inode_flags(struct inode *inode)
 		new_fl |= S_NOATIME;
 	if (flags & EXT4_DIRSYNC_FL)
 		new_fl |= S_DIRSYNC;
-	if (ext4_enable_dax(inode))
+	if (init && ext4_enable_dax(inode))
 		new_fl |= S_DAX;
 	if (flags & EXT4_ENCRYPT_FL)
 		new_fl |= S_ENCRYPTED;
@@ -4639,7 +4641,7 @@  struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
 		 * not initialized on a new filesystem. */
 	}
 	ei->i_flags = le32_to_cpu(raw_inode->i_flags);
-	ext4_set_inode_flags(inode);
+	ext4_set_inode_flags(inode, true);
 	inode->i_blocks = ext4_inode_blocks(raw_inode, ei);
 	ei->i_file_acl = le32_to_cpu(raw_inode->i_file_acl_lo);
 	if (ext4_has_feature_64bit(sb))
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index ca07d5086f03..efe41dcc8807 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -393,7 +393,8 @@  static int ext4_ioctl_setflags(struct inode *inode,
 			ext4_clear_inode_flag(inode, i);
 	}
 
-	ext4_set_inode_flags(inode);
+	ext4_set_inode_flags(inode, false);
+
 	inode->i_ctime = current_time(inode);
 
 	err = ext4_mark_iloc_dirty(handle, inode, &iloc);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0175fbfeb2ea..43cc52ea7918 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1349,7 +1349,7 @@  static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 			ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
 			ext4_clear_inode_state(inode,
 					EXT4_STATE_MAY_INLINE_DATA);
-			ext4_set_inode_flags(inode);
+			ext4_set_inode_flags(inode, false);
 		}
 		return res;
 	}
@@ -1372,7 +1372,7 @@  static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 				    ctx, len, 0);
 	if (!res) {
 		ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
-		ext4_set_inode_flags(inode);
+		ext4_set_inode_flags(inode, false);
 		res = ext4_mark_inode_dirty(handle, inode);
 		if (res)
 			EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index ce3f9a198d3b..4f7d780ff6da 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -244,7 +244,7 @@  static int ext4_end_enable_verity(struct file *filp, const void *desc,
 		if (err)
 			goto out_stop;
 		ext4_set_inode_flag(inode, EXT4_INODE_VERITY);
-		ext4_set_inode_flags(inode);
+		ext4_set_inode_flags(inode, false);
 		err = ext4_mark_iloc_dirty(handle, inode, &iloc);
 	}
 out_stop: