If we write to holes in the file via mmap, we endup allocating
new blocks. This block allocation happens without taking inode->i_mutex.
Since migrate is protected by i_mutex and migrate expect no
new blocks get allocated during migrate, fail migrate if new blocks
get allocated.
We can't take inode->i_mutex in the mmap write path because that
would result in a locking order violation between i_mutex and mmap_sem.
Also adding a seprate rw_sempahore for protecion is really high overhead
for a rare operation such as migrate.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
---
fs/ext4/inode.c | 17 ++++++++++++-----
fs/ext4/migrate.c | 28 +++++++++++++++++++++++++---
include/linux/ext4_fs.h | 1 +
3 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 059f2fc..f947251 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -986,6 +986,16 @@ int ext4_get_blocks_wrap(handle_t *handle, struct inode *inode, sector_t block,
retval = ext4_get_blocks_handle(handle, inode, block,
max_blocks, bh, create, extend_disksize);
}
+
+ if (retval > 0) {
+ /*
+ * We allocated new blocks which will result in i_data
+ * format to change. Force the migrate to fail by
+ * clearing migrate flags
+ */
+ EXT4_I(inode)->i_flags = EXT4_I(inode)->i_flags &
+ ~EXT4_EXT_MIGRATE;
+ }
up_write((&EXT4_I(inode)->i_data_sem));
return retval;
}
@@ -2962,7 +2972,8 @@ static int ext4_do_update_inode(handle_t *handle,
if (ext4_inode_blocks_set(handle, raw_inode, ei))
goto out_brelse;
raw_inode->i_dtime = cpu_to_le32(ei->i_dtime);
- raw_inode->i_flags = cpu_to_le32(ei->i_flags);
+ /* clear the migrate flag in the raw_inode */
+ raw_inode->i_flags = cpu_to_le32(ei->i_flags & ~EXT4_EXT_MIGRATE);
if (EXT4_SB(inode->i_sb)->s_es->s_creator_os !=
cpu_to_le32(EXT4_OS_HURD))
raw_inode->i_file_acl_high =
@@ -3502,9 +3513,5 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct page *page)
* access and zero out the page. The journal handle get initialized
* in ext4_get_block.
*/
- /* FIXME!! should we take inode->i_mutex ? Currently we can't because
- * it has a circular locking dependency with DIO. But migrate expect
- * i_mutex to ensure no i_data changes
- */
return block_page_mkwrite(vma, page, ext4_get_block);
}
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 5c1e27d..f4c9e78 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -327,7 +327,7 @@ static int free_ind_block(handle_t *handle, struct inode *inode, __le32 *i_data)
}
static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
- struct inode *tmp_inode)
+ struct inode *tmp_inode)
{
int retval;
__le32 i_data[3];
@@ -351,6 +351,18 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
down_write(&EXT4_I(inode)->i_data_sem);
/*
+ * if EXT4_EXT_MIGRATE is cleared a block allocation
+ * happened after we started the migrate. We need to
+ * fail the migrate
+ */
+ if (!(EXT4_I(inode)->i_flags & EXT4_EXT_MIGRATE)) {
+ retval = -EAGAIN;
+ up_write(&EXT4_I(inode)->i_data_sem);
+ goto err_out;
+ } else
+ EXT4_I(inode)->i_flags = EXT4_I(inode)->i_flags &
+ ~EXT4_EXT_MIGRATE;
+ /*
* We have the extent map build with the tmp inode.
* Now copy the i_data across
*/
@@ -508,6 +520,17 @@ int ext4_ext_migrate(struct inode *inode, struct file *filp,
* switch the inode format to prevent read.
*/
mutex_lock(&(inode->i_mutex));
+ /*
+ * Even though we take i_mutex we can still cause block allocation
+ * via mmap write to holes. If we have allocated new blocks we fail
+ * migrate. New block allocation will clear EXT4_EXT_MIGRATE flag
+ * The flag is updated with i_data_sem held to prevent racing with
+ * block allocation.
+ */
+ down_read((&EXT4_I(inode)->i_data_sem));
+ EXT4_I(inode)->i_flags = EXT4_I(inode)->i_flags | EXT4_EXT_MIGRATE;
+ up_read((&EXT4_I(inode)->i_data_sem));
+
handle = ext4_journal_start(inode, 1);
ei = EXT4_I(inode);
@@ -560,8 +583,7 @@ err_out:
*/
free_ext_block(handle, tmp_inode);
else
- retval = ext4_ext_swap_inode_data(handle, inode,
- tmp_inode);
+ retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode);
/* We mark the tmp_inode dirty via ext4_ext_tree_init. */
if (ext4_journal_extend(handle, 1) != 0)
diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
index 8f5a563..2d15f16 100644
--- a/include/linux/ext4_fs.h
+++ b/include/linux/ext4_fs.h
@@ -240,6 +240,7 @@ struct flex_groups {
#define EXT4_TOPDIR_FL 0x00020000 /* Top of directory hierarchies*/
#define EXT4_HUGE_FILE_FL 0x00040000 /* Set to each huge file */
#define EXT4_EXTENTS_FL 0x00080000 /* Inode uses extents */
+#define EXT4_EXT_MIGRATE 0x00100000 /* Inode is migrating */
#define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
#define EXT4_FL_USER_VISIBLE 0x000BDFFF /* User visible flags */
--
1.5.4.4.532.ga6828.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
