linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ext3: Use bitops to read/modify EXT3_I(inode)->i_state
@ 2010-01-06 21:27 Jan Kara
  2010-01-24 19:35 ` [PATCH] ext4: Use bitops to read/modify EXT4_I(inode)->i_state Theodore Ts'o
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2010-01-06 21:27 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso, Jan Kara

At several places we modify EXT3_I(inode)->i_state without holding i_mutex
(ext3_release_file, ext3_bmap, ext3_journalled_writepage, ext3_do_update_inode,
...). These modifications are racy and we can lose updates to i_state. So
convert handling of i_state to use bitops which are atomic.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext3/file.c            |    4 ++--
 fs/ext3/inode.c           |   18 +++++++++---------
 fs/ext3/xattr.c           |   14 +++++++-------
 include/linux/ext3_fs.h   |   33 +++++++++++++++++++++++++--------
 include/linux/ext3_fs_i.h |    2 +-
 5 files changed, 44 insertions(+), 27 deletions(-)

 Ted, I can create a similar patch for ext4 if you wish...

diff --git a/fs/ext3/file.c b/fs/ext3/file.c
index 388bbdf..a86d330 100644
--- a/fs/ext3/file.c
+++ b/fs/ext3/file.c
@@ -33,9 +33,9 @@
  */
 static int ext3_release_file (struct inode * inode, struct file * filp)
 {
-	if (EXT3_I(inode)->i_state & EXT3_STATE_FLUSH_ON_CLOSE) {
+	if (ext3_test_inode_state(inode, EXT3_STATE_FLUSH_ON_CLOSE)) {
 		filemap_flush(inode->i_mapping);
-		EXT3_I(inode)->i_state &= ~EXT3_STATE_FLUSH_ON_CLOSE;
+		ext3_clear_inode_state(inode, EXT3_STATE_FLUSH_ON_CLOSE);
 	}
 	/* if we are the last writer on the inode, drop the block reservation */
 	if ((filp->f_mode & FMODE_WRITE) &&
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 455e6e6..44b5338 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -1378,7 +1378,7 @@ static int ext3_journalled_write_end(struct file *file,
 	 */
 	if (pos + len > inode->i_size && ext3_can_truncate(inode))
 		ext3_orphan_add(handle, inode);
-	EXT3_I(inode)->i_state |= EXT3_STATE_JDATA;
+	ext3_set_inode_state(inode, EXT3_STATE_JDATA);
 	if (inode->i_size > EXT3_I(inode)->i_disksize) {
 		EXT3_I(inode)->i_disksize = inode->i_size;
 		ret2 = ext3_mark_inode_dirty(handle, inode);
@@ -1417,7 +1417,7 @@ static sector_t ext3_bmap(struct address_space *mapping, sector_t block)
 	journal_t *journal;
 	int err;
 
-	if (EXT3_I(inode)->i_state & EXT3_STATE_JDATA) {
+	if (ext3_test_inode_state(inode, EXT3_STATE_JDATA)) {
 		/*
 		 * This is a REALLY heavyweight approach, but the use of
 		 * bmap on dirty files is expected to be extremely rare:
@@ -1436,7 +1436,7 @@ static sector_t ext3_bmap(struct address_space *mapping, sector_t block)
 		 * everything they get.
 		 */
 
-		EXT3_I(inode)->i_state &= ~EXT3_STATE_JDATA;
+		ext3_clear_inode_state(inode, EXT3_STATE_JDATA);
 		journal = EXT3_JOURNAL(inode);
 		journal_lock_updates(journal);
 		err = journal_flush(journal);
@@ -1670,7 +1670,7 @@ static int ext3_journalled_writepage(struct page *page,
 				PAGE_CACHE_SIZE, NULL, write_end_fn);
 		if (ret == 0)
 			ret = err;
-		EXT3_I(inode)->i_state |= EXT3_STATE_JDATA;
+		ext3_set_inode_state(inode, EXT3_STATE_JDATA);
 		unlock_page(page);
 	} else {
 		/*
@@ -2402,7 +2402,7 @@ void ext3_truncate(struct inode *inode)
 		goto out_notrans;
 
 	if (inode->i_size == 0 && ext3_should_writeback_data(inode))
-		ei->i_state |= EXT3_STATE_FLUSH_ON_CLOSE;
+		ext3_set_inode_state(inode, EXT3_STATE_FLUSH_ON_CLOSE);
 
 	/*
 	 * We have to lock the EOF page here, because lock_page() nests
@@ -2721,7 +2721,7 @@ int ext3_get_inode_loc(struct inode *inode, struct ext3_iloc *iloc)
 {
 	/* We have all inode data except xattrs in memory here. */
 	return __ext3_get_inode_loc(inode, iloc,
-		!(EXT3_I(inode)->i_state & EXT3_STATE_XATTR));
+		!ext3_test_inode_state(inode, EXT3_STATE_XATTR));
 }
 
 void ext3_set_inode_flags(struct inode *inode)
@@ -2893,7 +2893,7 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino)
 					EXT3_GOOD_OLD_INODE_SIZE +
 					ei->i_extra_isize;
 			if (*magic == cpu_to_le32(EXT3_XATTR_MAGIC))
-				 ei->i_state |= EXT3_STATE_XATTR;
+				 ext3_set_inode_state(inode, EXT3_STATE_XATTR);
 		}
 	} else
 		ei->i_extra_isize = 0;
@@ -2955,7 +2955,7 @@ again:
 
 	/* For fields not not tracking in the in-memory inode,
 	 * initialise them to zero for new inodes. */
-	if (ei->i_state & EXT3_STATE_NEW)
+	if (ext3_test_inode_state(inode, EXT3_STATE_NEW))
 		memset(raw_inode, 0, EXT3_SB(inode->i_sb)->s_inode_size);
 
 	ext3_get_inode_flags(ei);
@@ -3052,7 +3052,7 @@ again:
 	rc = ext3_journal_dirty_metadata(handle, bh);
 	if (!err)
 		err = rc;
-	ei->i_state &= ~EXT3_STATE_NEW;
+	ext3_clear_inode_state(inode, EXT3_STATE_NEW);
 
 	atomic_set(&ei->i_sync_tid, handle->h_transaction->t_tid);
 out_brelse:
diff --git a/fs/ext3/xattr.c b/fs/ext3/xattr.c
index 66895cc..2d2fb2a 100644
--- a/fs/ext3/xattr.c
+++ b/fs/ext3/xattr.c
@@ -274,7 +274,7 @@ ext3_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
 	void *end;
 	int error;
 
-	if (!(EXT3_I(inode)->i_state & EXT3_STATE_XATTR))
+	if (!ext3_test_inode_state(inode, EXT3_STATE_XATTR))
 		return -ENODATA;
 	error = ext3_get_inode_loc(inode, &iloc);
 	if (error)
@@ -403,7 +403,7 @@ ext3_xattr_ibody_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	void *end;
 	int error;
 
-	if (!(EXT3_I(inode)->i_state & EXT3_STATE_XATTR))
+	if (!ext3_test_inode_state(inode, EXT3_STATE_XATTR))
 		return 0;
 	error = ext3_get_inode_loc(inode, &iloc);
 	if (error)
@@ -882,7 +882,7 @@ ext3_xattr_ibody_find(struct inode *inode, struct ext3_xattr_info *i,
 	is->s.base = is->s.first = IFIRST(header);
 	is->s.here = is->s.first;
 	is->s.end = (void *)raw_inode + EXT3_SB(inode->i_sb)->s_inode_size;
-	if (EXT3_I(inode)->i_state & EXT3_STATE_XATTR) {
+	if (ext3_test_inode_state(inode, EXT3_STATE_XATTR)) {
 		error = ext3_xattr_check_names(IFIRST(header), is->s.end);
 		if (error)
 			return error;
@@ -914,10 +914,10 @@ ext3_xattr_ibody_set(handle_t *handle, struct inode *inode,
 	header = IHDR(inode, ext3_raw_inode(&is->iloc));
 	if (!IS_LAST_ENTRY(s->first)) {
 		header->h_magic = cpu_to_le32(EXT3_XATTR_MAGIC);
-		EXT3_I(inode)->i_state |= EXT3_STATE_XATTR;
+		ext3_set_inode_state(inode, EXT3_STATE_XATTR);
 	} else {
 		header->h_magic = cpu_to_le32(0);
-		EXT3_I(inode)->i_state &= ~EXT3_STATE_XATTR;
+		ext3_clear_inode_state(inode, EXT3_STATE_XATTR);
 	}
 	return 0;
 }
@@ -967,10 +967,10 @@ ext3_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 	if (error)
 		goto cleanup;
 
-	if (EXT3_I(inode)->i_state & EXT3_STATE_NEW) {
+	if (ext3_test_inode_state(inode, EXT3_STATE_NEW)) {
 		struct ext3_inode *raw_inode = ext3_raw_inode(&is.iloc);
 		memset(raw_inode, 0, EXT3_SB(inode->i_sb)->s_inode_size);
-		EXT3_I(inode)->i_state &= ~EXT3_STATE_NEW;
+		ext3_clear_inode_state(inode, EXT3_STATE_NEW);
 	}
 
 	error = ext3_xattr_ibody_find(inode, &i, &is);
diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 6b04903..e6590f8 100644
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -202,14 +202,6 @@ static inline __u32 ext3_mask_flags(umode_t mode, __u32 flags)
 		return flags & EXT3_OTHER_FLMASK;
 }
 
-/*
- * Inode dynamic state flags
- */
-#define EXT3_STATE_JDATA		0x00000001 /* journaled data exists */
-#define EXT3_STATE_NEW			0x00000002 /* inode is newly created */
-#define EXT3_STATE_XATTR		0x00000004 /* has in-inode xattrs */
-#define EXT3_STATE_FLUSH_ON_CLOSE	0x00000008
-
 /* Used to pass group descriptor data when online resize is done */
 struct ext3_new_group_input {
 	__u32 group;            /* Group number for this data */
@@ -560,6 +552,31 @@ static inline int ext3_valid_inum(struct super_block *sb, unsigned long ino)
 		(ino >= EXT3_FIRST_INO(sb) &&
 		 ino <= le32_to_cpu(EXT3_SB(sb)->s_es->s_inodes_count));
 }
+
+/*
+ * Inode dynamic state flags
+ */
+enum {
+	EXT3_STATE_JDATA,		/* journaled data exists */
+	EXT3_STATE_NEW,			/* inode is newly created */
+	EXT3_STATE_XATTR,		/* has in-inode xattrs */
+	EXT3_STATE_FLUSH_ON_CLOSE,	/* flush dirty pages on close */
+};
+
+static inline int ext3_test_inode_state(struct inode *inode, int bit)
+{
+	return test_bit(bit, &EXT3_I(inode)->i_state);
+}
+
+static inline void ext3_set_inode_state(struct inode *inode, int bit)
+{
+	set_bit(bit, &EXT3_I(inode)->i_state);
+}
+
+static inline void ext3_clear_inode_state(struct inode *inode, int bit)
+{
+	clear_bit(bit, &EXT3_I(inode)->i_state);
+}
 #else
 /* Assume that user mode programs are passing in an ext3fs superblock, not
  * a kernel struct super_block.  This will allow us to call the feature-test
diff --git a/include/linux/ext3_fs_i.h b/include/linux/ext3_fs_i.h
index 93e7428..7679acd 100644
--- a/include/linux/ext3_fs_i.h
+++ b/include/linux/ext3_fs_i.h
@@ -87,7 +87,7 @@ struct ext3_inode_info {
 	 * near to their parent directory's inode.
 	 */
 	__u32	i_block_group;
-	__u32	i_state;		/* Dynamic state flags for ext3 */
+	unsigned long	i_state;	/* Dynamic state flags for ext3 */
 
 	/* block reservation info */
 	struct ext3_block_alloc_info *i_block_alloc_info;
-- 
1.6.4.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] ext4: Use bitops to read/modify EXT4_I(inode)->i_state
  2010-01-06 21:27 [PATCH] ext3: Use bitops to read/modify EXT3_I(inode)->i_state Jan Kara
@ 2010-01-24 19:35 ` Theodore Ts'o
  2010-01-26 20:54   ` Jan Kara
  0 siblings, 1 reply; 3+ messages in thread
From: Theodore Ts'o @ 2010-01-24 19:35 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o, Jan Kara

At several places we modify EXT4_I(inode)->i_state without holding
i_mutex (ext4_release_file, ext4_bmap, ext4_journalled_writepage,
ext4_do_update_inode, ...). These modifications are racy and we can
lose updates to i_state. So convert handling of i_state to use bitops
which are atomic.

Cc: Jan Kara <jack@suse.cz>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/ext4.h    |   41 +++++++++++++++++++++++++++++------------
 fs/ext4/extents.c |    8 ++++----
 fs/ext4/file.c    |    4 ++--
 fs/ext4/ialloc.c  |    3 ++-
 fs/ext4/inode.c   |   38 ++++++++++++++++++++------------------
 fs/ext4/migrate.c |    6 +++---
 fs/ext4/xattr.c   |   22 +++++++++++-----------
 7 files changed, 71 insertions(+), 51 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 2ca1b41..ac000a3 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -313,17 +313,6 @@ static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
 		return flags & EXT4_OTHER_FLMASK;
 }
 
-/*
- * Inode dynamic state flags
- */
-#define EXT4_STATE_JDATA		0x00000001 /* journaled data exists */
-#define EXT4_STATE_NEW			0x00000002 /* inode is newly created */
-#define EXT4_STATE_XATTR		0x00000004 /* has in-inode xattrs */
-#define EXT4_STATE_NO_EXPAND		0x00000008 /* No space for expansion */
-#define EXT4_STATE_DA_ALLOC_CLOSE	0x00000010 /* Alloc DA blks on close */
-#define EXT4_STATE_EXT_MIGRATE		0x00000020 /* Inode is migrating */
-#define EXT4_STATE_DIO_UNWRITTEN	0x00000040 /* need convert on dio done*/
-
 /* Used to pass group descriptor data when online resize is done */
 struct ext4_new_group_input {
 	__u32 group;		/* Group number for this data */
@@ -631,7 +620,7 @@ struct ext4_inode_info {
 	 * near to their parent directory's inode.
 	 */
 	ext4_group_t	i_block_group;
-	__u32	i_state;		/* Dynamic state flags for ext4 */
+	unsigned long	i_state_flags;		/* Dynamic state flags */
 
 	ext4_lblk_t		i_dir_start_lookup;
 #ifdef CONFIG_EXT4_FS_XATTR
@@ -1051,6 +1040,34 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino)
 		(ino >= EXT4_FIRST_INO(sb) &&
 		 ino <= le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count));
 }
+
+/*
+ * Inode dynamic state flags
+ */
+enum {
+	EXT4_STATE_JDATA,		/* journaled data exists */
+	EXT4_STATE_NEW,			/* inode is newly created */
+	EXT4_STATE_XATTR,		/* has in-inode xattrs */
+	EXT4_STATE_NO_EXPAND,		/* No space for expansion */
+	EXT4_STATE_DA_ALLOC_CLOSE,	/* Alloc DA blks on close */
+	EXT4_STATE_EXT_MIGRATE,		/* Inode is migrating */
+	EXT4_STATE_DIO_UNWRITTEN,	/* need convert on dio done*/
+};
+
+static inline int ext4_test_inode_state(struct inode *inode, int bit)
+{
+	return test_bit(bit, &EXT4_I(inode)->i_state_flags);
+}
+
+static inline void ext4_set_inode_state(struct inode *inode, int bit)
+{
+	set_bit(bit, &EXT4_I(inode)->i_state_flags);
+}
+
+static inline void ext4_clear_inode_state(struct inode *inode, int bit)
+{
+	clear_bit(bit, &EXT4_I(inode)->i_state_flags);
+}
 #else
 /* Assume that user mode programs are passing in an ext4fs superblock, not
  * a kernel struct super_block.  This will allow us to call the feature-test
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 8a20a5e..5a5a47a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3068,7 +3068,7 @@ ext4_ext_handle_uninitialized_extents(handle_t *handle, struct inode *inode,
 		if (io)
 			io->flag = DIO_AIO_UNWRITTEN;
 		else
-			EXT4_I(inode)->i_state |= EXT4_STATE_DIO_UNWRITTEN;
+			ext4_set_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN);
 		goto out;
 	}
 	/* async DIO end_io complete, convert the filled extent to written */
@@ -3342,8 +3342,8 @@ int ext4_ext_get_blocks(handle_t *handle, struct inode *inode,
 			if (io)
 				io->flag = DIO_AIO_UNWRITTEN;
 			else
-				EXT4_I(inode)->i_state |=
-					EXT4_STATE_DIO_UNWRITTEN;;
+				ext4_set_inode_state(inode,
+						     EXT4_STATE_DIO_UNWRITTEN);
 		}
 	}
 	err = ext4_ext_insert_extent(handle, inode, path, &newex, flags);
@@ -3721,7 +3721,7 @@ static int ext4_xattr_fiemap(struct inode *inode,
 	int error = 0;
 
 	/* in-inode? */
-	if (EXT4_I(inode)->i_state & EXT4_STATE_XATTR) {
+	if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
 		struct ext4_iloc iloc;
 		int offset;	/* offset of xattr in inode */
 
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 9630583..f6071ce 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -35,9 +35,9 @@
  */
 static int ext4_release_file(struct inode *inode, struct file *filp)
 {
-	if (EXT4_I(inode)->i_state & EXT4_STATE_DA_ALLOC_CLOSE) {
+	if (ext4_test_inode_state(inode, EXT4_STATE_DA_ALLOC_CLOSE)) {
 		ext4_alloc_da_blocks(inode);
-		EXT4_I(inode)->i_state &= ~EXT4_STATE_DA_ALLOC_CLOSE;
+		ext4_clear_inode_state(inode, EXT4_STATE_DA_ALLOC_CLOSE);
 	}
 	/* if we are the last writer on the inode, drop the block reservation */
 	if ((filp->f_mode & FMODE_WRITE) &&
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index f3624ea..2fab5ad 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1029,7 +1029,8 @@ got:
 	inode->i_generation = sbi->s_next_generation++;
 	spin_unlock(&sbi->s_next_gen_lock);
 
-	ei->i_state = EXT4_STATE_NEW;
+	ei->i_state_flags = 0;
+	ext4_set_inode_state(inode, EXT4_STATE_NEW);
 
 	ei->i_extra_isize = EXT4_SB(sb)->s_want_extra_isize;
 
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1f432b5..30b814f 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1307,7 +1307,7 @@ int ext4_get_blocks(handle_t *handle, struct inode *inode, sector_t block,
 			 * i_data's format changing.  Force the migrate
 			 * to fail by clearing migrate flags
 			 */
-			EXT4_I(inode)->i_state &= ~EXT4_STATE_EXT_MIGRATE;
+			ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
 		}
 
 		/*
@@ -1794,7 +1794,7 @@ static int ext4_journalled_write_end(struct file *file,
 	new_i_size = pos + copied;
 	if (new_i_size > inode->i_size)
 		i_size_write(inode, pos+copied);
-	EXT4_I(inode)->i_state |= EXT4_STATE_JDATA;
+	ext4_set_inode_state(inode, EXT4_STATE_JDATA);
 	if (new_i_size > EXT4_I(inode)->i_disksize) {
 		ext4_update_i_disksize(inode, new_i_size);
 		ret2 = ext4_mark_inode_dirty(handle, inode);
@@ -2616,7 +2616,7 @@ static int __ext4_journalled_writepage(struct page *page,
 		ret = err;
 
 	walk_page_buffers(handle, page_bufs, 0, len, NULL, bput_one);
-	EXT4_I(inode)->i_state |= EXT4_STATE_JDATA;
+	ext4_set_inode_state(inode, EXT4_STATE_JDATA);
 out:
 	return ret;
 }
@@ -3287,7 +3287,8 @@ static sector_t ext4_bmap(struct address_space *mapping, sector_t block)
 		filemap_write_and_wait(mapping);
 	}
 
-	if (EXT4_JOURNAL(inode) && EXT4_I(inode)->i_state & EXT4_STATE_JDATA) {
+	if (EXT4_JOURNAL(inode) &&
+	    ext4_test_inode_state(inode, EXT4_STATE_JDATA)) {
 		/*
 		 * This is a REALLY heavyweight approach, but the use of
 		 * bmap on dirty files is expected to be extremely rare:
@@ -3306,7 +3307,7 @@ static sector_t ext4_bmap(struct address_space *mapping, sector_t block)
 		 * everything they get.
 		 */
 
-		EXT4_I(inode)->i_state &= ~EXT4_STATE_JDATA;
+		ext4_clear_inode_state(inode, EXT4_STATE_JDATA);
 		journal = EXT4_JOURNAL(inode);
 		jbd2_journal_lock_updates(journal);
 		err = jbd2_journal_flush(journal);
@@ -3774,8 +3775,8 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
 		if (ret != -EIOCBQUEUED && ret <= 0 && iocb->private) {
 			ext4_free_io_end(iocb->private);
 			iocb->private = NULL;
-		} else if (ret > 0 && (EXT4_I(inode)->i_state &
-				       EXT4_STATE_DIO_UNWRITTEN)) {
+		} else if (ret > 0 && ext4_test_inode_state(inode,
+						EXT4_STATE_DIO_UNWRITTEN)) {
 			int err;
 			/*
 			 * for non AIO case, since the IO is already
@@ -3785,7 +3786,7 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
 							     offset, ret);
 			if (err < 0)
 				ret = err;
-			EXT4_I(inode)->i_state &= ~EXT4_STATE_DIO_UNWRITTEN;
+			ext4_clear_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN);
 		}
 		return ret;
 	}
@@ -4441,7 +4442,7 @@ void ext4_truncate(struct inode *inode)
 		return;
 
 	if (inode->i_size == 0 && !test_opt(inode->i_sb, NO_AUTO_DA_ALLOC))
-		ei->i_state |= EXT4_STATE_DA_ALLOC_CLOSE;
+		ext4_set_inode_state(inode, EXT4_STATE_DA_ALLOC_CLOSE);
 
 	if (EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL) {
 		ext4_ext_truncate(inode);
@@ -4727,7 +4728,7 @@ int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc)
 {
 	/* We have all inode data except xattrs in memory here. */
 	return __ext4_get_inode_loc(inode, iloc,
-		!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR));
+		!ext4_test_inode_state(inode, EXT4_STATE_XATTR));
 }
 
 void ext4_set_inode_flags(struct inode *inode)
@@ -4821,7 +4822,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 	}
 	inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
 
-	ei->i_state = 0;
+	ei->i_state_flags = 0;
 	ei->i_dir_start_lookup = 0;
 	ei->i_dtime = le32_to_cpu(raw_inode->i_dtime);
 	/* We now have enough fields to check if the inode was active or not.
@@ -4904,7 +4905,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 					EXT4_GOOD_OLD_INODE_SIZE +
 					ei->i_extra_isize;
 			if (*magic == cpu_to_le32(EXT4_XATTR_MAGIC))
-				ei->i_state |= EXT4_STATE_XATTR;
+				ext4_set_inode_state(inode, EXT4_STATE_XATTR);
 		}
 	} else
 		ei->i_extra_isize = 0;
@@ -5044,7 +5045,7 @@ static int ext4_do_update_inode(handle_t *handle,
 
 	/* For fields not not tracking in the in-memory inode,
 	 * initialise them to zero for new inodes. */
-	if (ei->i_state & EXT4_STATE_NEW)
+	if (ext4_test_inode_state(inode, EXT4_STATE_NEW))
 		memset(raw_inode, 0, EXT4_SB(inode->i_sb)->s_inode_size);
 
 	ext4_get_inode_flags(ei);
@@ -5140,7 +5141,7 @@ static int ext4_do_update_inode(handle_t *handle,
 	rc = ext4_handle_dirty_metadata(handle, inode, bh);
 	if (!err)
 		err = rc;
-	ei->i_state &= ~EXT4_STATE_NEW;
+	ext4_clear_inode_state(inode, EXT4_STATE_NEW);
 
 	ext4_update_inode_fsync_trans(handle, inode, 0);
 out_brelse:
@@ -5564,8 +5565,8 @@ static int ext4_expand_extra_isize(struct inode *inode,
 	entry = IFIRST(header);
 
 	/* No extended attributes present */
-	if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR) ||
-		header->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC)) {
+	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) ||
+	    header->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC)) {
 		memset((void *)raw_inode + EXT4_GOOD_OLD_INODE_SIZE, 0,
 			new_extra_isize);
 		EXT4_I(inode)->i_extra_isize = new_extra_isize;
@@ -5609,7 +5610,7 @@ int ext4_mark_inode_dirty(handle_t *handle, struct inode *inode)
 	err = ext4_reserve_inode_write(handle, inode, &iloc);
 	if (ext4_handle_valid(handle) &&
 	    EXT4_I(inode)->i_extra_isize < sbi->s_want_extra_isize &&
-	    !(EXT4_I(inode)->i_state & EXT4_STATE_NO_EXPAND)) {
+	    !ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND)) {
 		/*
 		 * We need extra buffer credits since we may write into EA block
 		 * with this same handle. If journal_extend fails, then it will
@@ -5623,7 +5624,8 @@ int ext4_mark_inode_dirty(handle_t *handle, struct inode *inode)
 						      sbi->s_want_extra_isize,
 						      iloc, handle);
 			if (ret) {
-				EXT4_I(inode)->i_state |= EXT4_STATE_NO_EXPAND;
+				ext4_set_inode_state(inode,
+						     EXT4_STATE_NO_EXPAND);
 				if (mnt_count !=
 					le16_to_cpu(sbi->s_es->s_mnt_count)) {
 					ext4_warning(inode->i_sb, __func__,
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 8141581..46a4101 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -365,12 +365,12 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
 	 * happened after we started the migrate. We need to
 	 * fail the migrate
 	 */
-	if (!(EXT4_I(inode)->i_state & EXT4_STATE_EXT_MIGRATE)) {
+	if (!ext4_test_inode_state(inode, EXT4_STATE_EXT_MIGRATE)) {
 		retval = -EAGAIN;
 		up_write(&EXT4_I(inode)->i_data_sem);
 		goto err_out;
 	} else
-		EXT4_I(inode)->i_state &= ~EXT4_STATE_EXT_MIGRATE;
+		ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
 	/*
 	 * We have the extent map build with the tmp inode.
 	 * Now copy the i_data across
@@ -533,7 +533,7 @@ int ext4_ext_migrate(struct inode *inode)
 	 * allocation.
 	 */
 	down_read((&EXT4_I(inode)->i_data_sem));
-	EXT4_I(inode)->i_state |= EXT4_STATE_EXT_MIGRATE;
+	ext4_set_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
 	up_read((&EXT4_I(inode)->i_data_sem));
 
 	handle = ext4_journal_start(inode, 1);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index f3a2f7e..c619a7e 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -267,7 +267,7 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
 	void *end;
 	int error;
 
-	if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR))
+	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
 		return -ENODATA;
 	error = ext4_get_inode_loc(inode, &iloc);
 	if (error)
@@ -396,7 +396,7 @@ ext4_xattr_ibody_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	void *end;
 	int error;
 
-	if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR))
+	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
 		return 0;
 	error = ext4_get_inode_loc(inode, &iloc);
 	if (error)
@@ -908,7 +908,7 @@ ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
 	is->s.base = is->s.first = IFIRST(header);
 	is->s.here = is->s.first;
 	is->s.end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
-	if (EXT4_I(inode)->i_state & EXT4_STATE_XATTR) {
+	if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
 		error = ext4_xattr_check_names(IFIRST(header), is->s.end);
 		if (error)
 			return error;
@@ -940,10 +940,10 @@ ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
 	header = IHDR(inode, ext4_raw_inode(&is->iloc));
 	if (!IS_LAST_ENTRY(s->first)) {
 		header->h_magic = cpu_to_le32(EXT4_XATTR_MAGIC);
-		EXT4_I(inode)->i_state |= EXT4_STATE_XATTR;
+		ext4_set_inode_state(inode, EXT4_STATE_XATTR);
 	} else {
 		header->h_magic = cpu_to_le32(0);
-		EXT4_I(inode)->i_state &= ~EXT4_STATE_XATTR;
+		ext4_clear_inode_state(inode, EXT4_STATE_XATTR);
 	}
 	return 0;
 }
@@ -986,8 +986,8 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 	if (strlen(name) > 255)
 		return -ERANGE;
 	down_write(&EXT4_I(inode)->xattr_sem);
-	no_expand = EXT4_I(inode)->i_state & EXT4_STATE_NO_EXPAND;
-	EXT4_I(inode)->i_state |= EXT4_STATE_NO_EXPAND;
+	no_expand = ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND);
+	ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
 
 	error = ext4_get_inode_loc(inode, &is.iloc);
 	if (error)
@@ -997,10 +997,10 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 	if (error)
 		goto cleanup;
 
-	if (EXT4_I(inode)->i_state & EXT4_STATE_NEW) {
+	if (ext4_test_inode_state(inode, EXT4_STATE_NEW)) {
 		struct ext4_inode *raw_inode = ext4_raw_inode(&is.iloc);
 		memset(raw_inode, 0, EXT4_SB(inode->i_sb)->s_inode_size);
-		EXT4_I(inode)->i_state &= ~EXT4_STATE_NEW;
+		ext4_clear_inode_state(inode, EXT4_STATE_NEW);
 	}
 
 	error = ext4_xattr_ibody_find(inode, &i, &is);
@@ -1052,7 +1052,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 		ext4_xattr_update_super_block(handle, inode->i_sb);
 		inode->i_ctime = ext4_current_time(inode);
 		if (!value)
-			EXT4_I(inode)->i_state &= ~EXT4_STATE_NO_EXPAND;
+			ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
 		error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
 		/*
 		 * The bh is consumed by ext4_mark_iloc_dirty, even with
@@ -1067,7 +1067,7 @@ cleanup:
 	brelse(is.iloc.bh);
 	brelse(bs.bh);
 	if (no_expand == 0)
-		EXT4_I(inode)->i_state &= ~EXT4_STATE_NO_EXPAND;
+		ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
 	up_write(&EXT4_I(inode)->xattr_sem);
 	return error;
 }
-- 
1.6.6.1.1.g974db.dirty


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ext4: Use bitops to read/modify EXT4_I(inode)->i_state
  2010-01-24 19:35 ` [PATCH] ext4: Use bitops to read/modify EXT4_I(inode)->i_state Theodore Ts'o
@ 2010-01-26 20:54   ` Jan Kara
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Kara @ 2010-01-26 20:54 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Ext4 Developers List, Jan Kara

On Sun 24-01-10 14:35:57, Theodore Ts'o wrote:
> At several places we modify EXT4_I(inode)->i_state without holding
> i_mutex (ext4_release_file, ext4_bmap, ext4_journalled_writepage,
> ext4_do_update_inode, ...). These modifications are racy and we can
> lose updates to i_state. So convert handling of i_state to use bitops
> which are atomic.
> 
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
  The patch looks good.
Acked-by: Jan Kara <jack@suse.cz>

								Honza
> ---
>  fs/ext4/ext4.h    |   41 +++++++++++++++++++++++++++++------------
>  fs/ext4/extents.c |    8 ++++----
>  fs/ext4/file.c    |    4 ++--
>  fs/ext4/ialloc.c  |    3 ++-
>  fs/ext4/inode.c   |   38 ++++++++++++++++++++------------------
>  fs/ext4/migrate.c |    6 +++---
>  fs/ext4/xattr.c   |   22 +++++++++++-----------
>  7 files changed, 71 insertions(+), 51 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 2ca1b41..ac000a3 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -313,17 +313,6 @@ static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
>  		return flags & EXT4_OTHER_FLMASK;
>  }
>  
> -/*
> - * Inode dynamic state flags
> - */
> -#define EXT4_STATE_JDATA		0x00000001 /* journaled data exists */
> -#define EXT4_STATE_NEW			0x00000002 /* inode is newly created */
> -#define EXT4_STATE_XATTR		0x00000004 /* has in-inode xattrs */
> -#define EXT4_STATE_NO_EXPAND		0x00000008 /* No space for expansion */
> -#define EXT4_STATE_DA_ALLOC_CLOSE	0x00000010 /* Alloc DA blks on close */
> -#define EXT4_STATE_EXT_MIGRATE		0x00000020 /* Inode is migrating */
> -#define EXT4_STATE_DIO_UNWRITTEN	0x00000040 /* need convert on dio done*/
> -
>  /* Used to pass group descriptor data when online resize is done */
>  struct ext4_new_group_input {
>  	__u32 group;		/* Group number for this data */
> @@ -631,7 +620,7 @@ struct ext4_inode_info {
>  	 * near to their parent directory's inode.
>  	 */
>  	ext4_group_t	i_block_group;
> -	__u32	i_state;		/* Dynamic state flags for ext4 */
> +	unsigned long	i_state_flags;		/* Dynamic state flags */
>  
>  	ext4_lblk_t		i_dir_start_lookup;
>  #ifdef CONFIG_EXT4_FS_XATTR
> @@ -1051,6 +1040,34 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino)
>  		(ino >= EXT4_FIRST_INO(sb) &&
>  		 ino <= le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count));
>  }
> +
> +/*
> + * Inode dynamic state flags
> + */
> +enum {
> +	EXT4_STATE_JDATA,		/* journaled data exists */
> +	EXT4_STATE_NEW,			/* inode is newly created */
> +	EXT4_STATE_XATTR,		/* has in-inode xattrs */
> +	EXT4_STATE_NO_EXPAND,		/* No space for expansion */
> +	EXT4_STATE_DA_ALLOC_CLOSE,	/* Alloc DA blks on close */
> +	EXT4_STATE_EXT_MIGRATE,		/* Inode is migrating */
> +	EXT4_STATE_DIO_UNWRITTEN,	/* need convert on dio done*/
> +};
> +
> +static inline int ext4_test_inode_state(struct inode *inode, int bit)
> +{
> +	return test_bit(bit, &EXT4_I(inode)->i_state_flags);
> +}
> +
> +static inline void ext4_set_inode_state(struct inode *inode, int bit)
> +{
> +	set_bit(bit, &EXT4_I(inode)->i_state_flags);
> +}
> +
> +static inline void ext4_clear_inode_state(struct inode *inode, int bit)
> +{
> +	clear_bit(bit, &EXT4_I(inode)->i_state_flags);
> +}
>  #else
>  /* Assume that user mode programs are passing in an ext4fs superblock, not
>   * a kernel struct super_block.  This will allow us to call the feature-test
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 8a20a5e..5a5a47a 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3068,7 +3068,7 @@ ext4_ext_handle_uninitialized_extents(handle_t *handle, struct inode *inode,
>  		if (io)
>  			io->flag = DIO_AIO_UNWRITTEN;
>  		else
> -			EXT4_I(inode)->i_state |= EXT4_STATE_DIO_UNWRITTEN;
> +			ext4_set_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN);
>  		goto out;
>  	}
>  	/* async DIO end_io complete, convert the filled extent to written */
> @@ -3342,8 +3342,8 @@ int ext4_ext_get_blocks(handle_t *handle, struct inode *inode,
>  			if (io)
>  				io->flag = DIO_AIO_UNWRITTEN;
>  			else
> -				EXT4_I(inode)->i_state |=
> -					EXT4_STATE_DIO_UNWRITTEN;;
> +				ext4_set_inode_state(inode,
> +						     EXT4_STATE_DIO_UNWRITTEN);
>  		}
>  	}
>  	err = ext4_ext_insert_extent(handle, inode, path, &newex, flags);
> @@ -3721,7 +3721,7 @@ static int ext4_xattr_fiemap(struct inode *inode,
>  	int error = 0;
>  
>  	/* in-inode? */
> -	if (EXT4_I(inode)->i_state & EXT4_STATE_XATTR) {
> +	if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
>  		struct ext4_iloc iloc;
>  		int offset;	/* offset of xattr in inode */
>  
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 9630583..f6071ce 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -35,9 +35,9 @@
>   */
>  static int ext4_release_file(struct inode *inode, struct file *filp)
>  {
> -	if (EXT4_I(inode)->i_state & EXT4_STATE_DA_ALLOC_CLOSE) {
> +	if (ext4_test_inode_state(inode, EXT4_STATE_DA_ALLOC_CLOSE)) {
>  		ext4_alloc_da_blocks(inode);
> -		EXT4_I(inode)->i_state &= ~EXT4_STATE_DA_ALLOC_CLOSE;
> +		ext4_clear_inode_state(inode, EXT4_STATE_DA_ALLOC_CLOSE);
>  	}
>  	/* if we are the last writer on the inode, drop the block reservation */
>  	if ((filp->f_mode & FMODE_WRITE) &&
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index f3624ea..2fab5ad 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -1029,7 +1029,8 @@ got:
>  	inode->i_generation = sbi->s_next_generation++;
>  	spin_unlock(&sbi->s_next_gen_lock);
>  
> -	ei->i_state = EXT4_STATE_NEW;
> +	ei->i_state_flags = 0;
> +	ext4_set_inode_state(inode, EXT4_STATE_NEW);
>  
>  	ei->i_extra_isize = EXT4_SB(sb)->s_want_extra_isize;
>  
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 1f432b5..30b814f 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1307,7 +1307,7 @@ int ext4_get_blocks(handle_t *handle, struct inode *inode, sector_t block,
>  			 * i_data's format changing.  Force the migrate
>  			 * to fail by clearing migrate flags
>  			 */
> -			EXT4_I(inode)->i_state &= ~EXT4_STATE_EXT_MIGRATE;
> +			ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
>  		}
>  
>  		/*
> @@ -1794,7 +1794,7 @@ static int ext4_journalled_write_end(struct file *file,
>  	new_i_size = pos + copied;
>  	if (new_i_size > inode->i_size)
>  		i_size_write(inode, pos+copied);
> -	EXT4_I(inode)->i_state |= EXT4_STATE_JDATA;
> +	ext4_set_inode_state(inode, EXT4_STATE_JDATA);
>  	if (new_i_size > EXT4_I(inode)->i_disksize) {
>  		ext4_update_i_disksize(inode, new_i_size);
>  		ret2 = ext4_mark_inode_dirty(handle, inode);
> @@ -2616,7 +2616,7 @@ static int __ext4_journalled_writepage(struct page *page,
>  		ret = err;
>  
>  	walk_page_buffers(handle, page_bufs, 0, len, NULL, bput_one);
> -	EXT4_I(inode)->i_state |= EXT4_STATE_JDATA;
> +	ext4_set_inode_state(inode, EXT4_STATE_JDATA);
>  out:
>  	return ret;
>  }
> @@ -3287,7 +3287,8 @@ static sector_t ext4_bmap(struct address_space *mapping, sector_t block)
>  		filemap_write_and_wait(mapping);
>  	}
>  
> -	if (EXT4_JOURNAL(inode) && EXT4_I(inode)->i_state & EXT4_STATE_JDATA) {
> +	if (EXT4_JOURNAL(inode) &&
> +	    ext4_test_inode_state(inode, EXT4_STATE_JDATA)) {
>  		/*
>  		 * This is a REALLY heavyweight approach, but the use of
>  		 * bmap on dirty files is expected to be extremely rare:
> @@ -3306,7 +3307,7 @@ static sector_t ext4_bmap(struct address_space *mapping, sector_t block)
>  		 * everything they get.
>  		 */
>  
> -		EXT4_I(inode)->i_state &= ~EXT4_STATE_JDATA;
> +		ext4_clear_inode_state(inode, EXT4_STATE_JDATA);
>  		journal = EXT4_JOURNAL(inode);
>  		jbd2_journal_lock_updates(journal);
>  		err = jbd2_journal_flush(journal);
> @@ -3774,8 +3775,8 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
>  		if (ret != -EIOCBQUEUED && ret <= 0 && iocb->private) {
>  			ext4_free_io_end(iocb->private);
>  			iocb->private = NULL;
> -		} else if (ret > 0 && (EXT4_I(inode)->i_state &
> -				       EXT4_STATE_DIO_UNWRITTEN)) {
> +		} else if (ret > 0 && ext4_test_inode_state(inode,
> +						EXT4_STATE_DIO_UNWRITTEN)) {
>  			int err;
>  			/*
>  			 * for non AIO case, since the IO is already
> @@ -3785,7 +3786,7 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
>  							     offset, ret);
>  			if (err < 0)
>  				ret = err;
> -			EXT4_I(inode)->i_state &= ~EXT4_STATE_DIO_UNWRITTEN;
> +			ext4_clear_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN);
>  		}
>  		return ret;
>  	}
> @@ -4441,7 +4442,7 @@ void ext4_truncate(struct inode *inode)
>  		return;
>  
>  	if (inode->i_size == 0 && !test_opt(inode->i_sb, NO_AUTO_DA_ALLOC))
> -		ei->i_state |= EXT4_STATE_DA_ALLOC_CLOSE;
> +		ext4_set_inode_state(inode, EXT4_STATE_DA_ALLOC_CLOSE);
>  
>  	if (EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL) {
>  		ext4_ext_truncate(inode);
> @@ -4727,7 +4728,7 @@ int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc)
>  {
>  	/* We have all inode data except xattrs in memory here. */
>  	return __ext4_get_inode_loc(inode, iloc,
> -		!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR));
> +		!ext4_test_inode_state(inode, EXT4_STATE_XATTR));
>  }
>  
>  void ext4_set_inode_flags(struct inode *inode)
> @@ -4821,7 +4822,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
>  	}
>  	inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
>  
> -	ei->i_state = 0;
> +	ei->i_state_flags = 0;
>  	ei->i_dir_start_lookup = 0;
>  	ei->i_dtime = le32_to_cpu(raw_inode->i_dtime);
>  	/* We now have enough fields to check if the inode was active or not.
> @@ -4904,7 +4905,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
>  					EXT4_GOOD_OLD_INODE_SIZE +
>  					ei->i_extra_isize;
>  			if (*magic == cpu_to_le32(EXT4_XATTR_MAGIC))
> -				ei->i_state |= EXT4_STATE_XATTR;
> +				ext4_set_inode_state(inode, EXT4_STATE_XATTR);
>  		}
>  	} else
>  		ei->i_extra_isize = 0;
> @@ -5044,7 +5045,7 @@ static int ext4_do_update_inode(handle_t *handle,
>  
>  	/* For fields not not tracking in the in-memory inode,
>  	 * initialise them to zero for new inodes. */
> -	if (ei->i_state & EXT4_STATE_NEW)
> +	if (ext4_test_inode_state(inode, EXT4_STATE_NEW))
>  		memset(raw_inode, 0, EXT4_SB(inode->i_sb)->s_inode_size);
>  
>  	ext4_get_inode_flags(ei);
> @@ -5140,7 +5141,7 @@ static int ext4_do_update_inode(handle_t *handle,
>  	rc = ext4_handle_dirty_metadata(handle, inode, bh);
>  	if (!err)
>  		err = rc;
> -	ei->i_state &= ~EXT4_STATE_NEW;
> +	ext4_clear_inode_state(inode, EXT4_STATE_NEW);
>  
>  	ext4_update_inode_fsync_trans(handle, inode, 0);
>  out_brelse:
> @@ -5564,8 +5565,8 @@ static int ext4_expand_extra_isize(struct inode *inode,
>  	entry = IFIRST(header);
>  
>  	/* No extended attributes present */
> -	if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR) ||
> -		header->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC)) {
> +	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) ||
> +	    header->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC)) {
>  		memset((void *)raw_inode + EXT4_GOOD_OLD_INODE_SIZE, 0,
>  			new_extra_isize);
>  		EXT4_I(inode)->i_extra_isize = new_extra_isize;
> @@ -5609,7 +5610,7 @@ int ext4_mark_inode_dirty(handle_t *handle, struct inode *inode)
>  	err = ext4_reserve_inode_write(handle, inode, &iloc);
>  	if (ext4_handle_valid(handle) &&
>  	    EXT4_I(inode)->i_extra_isize < sbi->s_want_extra_isize &&
> -	    !(EXT4_I(inode)->i_state & EXT4_STATE_NO_EXPAND)) {
> +	    !ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND)) {
>  		/*
>  		 * We need extra buffer credits since we may write into EA block
>  		 * with this same handle. If journal_extend fails, then it will
> @@ -5623,7 +5624,8 @@ int ext4_mark_inode_dirty(handle_t *handle, struct inode *inode)
>  						      sbi->s_want_extra_isize,
>  						      iloc, handle);
>  			if (ret) {
> -				EXT4_I(inode)->i_state |= EXT4_STATE_NO_EXPAND;
> +				ext4_set_inode_state(inode,
> +						     EXT4_STATE_NO_EXPAND);
>  				if (mnt_count !=
>  					le16_to_cpu(sbi->s_es->s_mnt_count)) {
>  					ext4_warning(inode->i_sb, __func__,
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 8141581..46a4101 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -365,12 +365,12 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>  	 * happened after we started the migrate. We need to
>  	 * fail the migrate
>  	 */
> -	if (!(EXT4_I(inode)->i_state & EXT4_STATE_EXT_MIGRATE)) {
> +	if (!ext4_test_inode_state(inode, EXT4_STATE_EXT_MIGRATE)) {
>  		retval = -EAGAIN;
>  		up_write(&EXT4_I(inode)->i_data_sem);
>  		goto err_out;
>  	} else
> -		EXT4_I(inode)->i_state &= ~EXT4_STATE_EXT_MIGRATE;
> +		ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
>  	/*
>  	 * We have the extent map build with the tmp inode.
>  	 * Now copy the i_data across
> @@ -533,7 +533,7 @@ int ext4_ext_migrate(struct inode *inode)
>  	 * allocation.
>  	 */
>  	down_read((&EXT4_I(inode)->i_data_sem));
> -	EXT4_I(inode)->i_state |= EXT4_STATE_EXT_MIGRATE;
> +	ext4_set_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
>  	up_read((&EXT4_I(inode)->i_data_sem));
>  
>  	handle = ext4_journal_start(inode, 1);
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index f3a2f7e..c619a7e 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -267,7 +267,7 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
>  	void *end;
>  	int error;
>  
> -	if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR))
> +	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
>  		return -ENODATA;
>  	error = ext4_get_inode_loc(inode, &iloc);
>  	if (error)
> @@ -396,7 +396,7 @@ ext4_xattr_ibody_list(struct dentry *dentry, char *buffer, size_t buffer_size)
>  	void *end;
>  	int error;
>  
> -	if (!(EXT4_I(inode)->i_state & EXT4_STATE_XATTR))
> +	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
>  		return 0;
>  	error = ext4_get_inode_loc(inode, &iloc);
>  	if (error)
> @@ -908,7 +908,7 @@ ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
>  	is->s.base = is->s.first = IFIRST(header);
>  	is->s.here = is->s.first;
>  	is->s.end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> -	if (EXT4_I(inode)->i_state & EXT4_STATE_XATTR) {
> +	if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
>  		error = ext4_xattr_check_names(IFIRST(header), is->s.end);
>  		if (error)
>  			return error;
> @@ -940,10 +940,10 @@ ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
>  	header = IHDR(inode, ext4_raw_inode(&is->iloc));
>  	if (!IS_LAST_ENTRY(s->first)) {
>  		header->h_magic = cpu_to_le32(EXT4_XATTR_MAGIC);
> -		EXT4_I(inode)->i_state |= EXT4_STATE_XATTR;
> +		ext4_set_inode_state(inode, EXT4_STATE_XATTR);
>  	} else {
>  		header->h_magic = cpu_to_le32(0);
> -		EXT4_I(inode)->i_state &= ~EXT4_STATE_XATTR;
> +		ext4_clear_inode_state(inode, EXT4_STATE_XATTR);
>  	}
>  	return 0;
>  }
> @@ -986,8 +986,8 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
>  	if (strlen(name) > 255)
>  		return -ERANGE;
>  	down_write(&EXT4_I(inode)->xattr_sem);
> -	no_expand = EXT4_I(inode)->i_state & EXT4_STATE_NO_EXPAND;
> -	EXT4_I(inode)->i_state |= EXT4_STATE_NO_EXPAND;
> +	no_expand = ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND);
> +	ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
>  
>  	error = ext4_get_inode_loc(inode, &is.iloc);
>  	if (error)
> @@ -997,10 +997,10 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
>  	if (error)
>  		goto cleanup;
>  
> -	if (EXT4_I(inode)->i_state & EXT4_STATE_NEW) {
> +	if (ext4_test_inode_state(inode, EXT4_STATE_NEW)) {
>  		struct ext4_inode *raw_inode = ext4_raw_inode(&is.iloc);
>  		memset(raw_inode, 0, EXT4_SB(inode->i_sb)->s_inode_size);
> -		EXT4_I(inode)->i_state &= ~EXT4_STATE_NEW;
> +		ext4_clear_inode_state(inode, EXT4_STATE_NEW);
>  	}
>  
>  	error = ext4_xattr_ibody_find(inode, &i, &is);
> @@ -1052,7 +1052,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
>  		ext4_xattr_update_super_block(handle, inode->i_sb);
>  		inode->i_ctime = ext4_current_time(inode);
>  		if (!value)
> -			EXT4_I(inode)->i_state &= ~EXT4_STATE_NO_EXPAND;
> +			ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
>  		error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
>  		/*
>  		 * The bh is consumed by ext4_mark_iloc_dirty, even with
> @@ -1067,7 +1067,7 @@ cleanup:
>  	brelse(is.iloc.bh);
>  	brelse(bs.bh);
>  	if (no_expand == 0)
> -		EXT4_I(inode)->i_state &= ~EXT4_STATE_NO_EXPAND;
> +		ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
>  	up_write(&EXT4_I(inode)->xattr_sem);
>  	return error;
>  }
> -- 
> 1.6.6.1.1.g974db.dirty
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-01-26 20:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-06 21:27 [PATCH] ext3: Use bitops to read/modify EXT3_I(inode)->i_state Jan Kara
2010-01-24 19:35 ` [PATCH] ext4: Use bitops to read/modify EXT4_I(inode)->i_state Theodore Ts'o
2010-01-26 20:54   ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).