There was race condition between creating new inode and OI scrub:
the OI scrub may find the new created inode just after the creator
creating it but before setting the LMA EA. Originally, to resolve
such trouble, the creator will set the new created inode's state
as LDISKFS_STATE_LUSTRE_NOSCRUB. But such state is set after the
new inode unlocked. So the OI scrub still has some chance to find
the new created inode with neither LDISKFS_STATE_LUSTRE_NOSCRUB
nor LMA EA.
Be as improvement, this patch makes the ldiskfs_create_inode() to
return the new created inode with lock. The caller can set more
state (not only for LFSCK, but also for other purposes in future)
on the new created inode before unlock it.
Signed-off-by: Fan Yong <fan.yong@intel.com>
Change-Id: Idc1a8fbd3701f7e431ef4b7858cfdf4674d74add
Reviewed-on: http://review.whamcloud.com/13187
Tested-by: Jenkins
Reviewed-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
/*
* DIR_NLINK feature is set if 1) nlinks > EXT4_LINK_MAX or 2) nlinks == 2,
* since this indicates that nlinks count was previously 1.
/*
* DIR_NLINK feature is set if 1) nlinks > EXT4_LINK_MAX or 2) nlinks == 2,
* since this indicates that nlinks count was previously 1.
-@@ -1806,6 +1809,27 @@ static unsigned ext4_dentry_goal(struct
+@@ -1806,6 +1809,29 @@ static unsigned ext4_dentry_goal(struct
-+struct inode * ext4_create_inode(handle_t *handle, struct inode * dir, int mode)
++ /* Return locked inode, then the caller can modify the inode's states/flags
++ * before others finding it. The caller should unlock the inode by itself. */
++struct inode *ext4_create_inode(handle_t *handle, struct inode *dir, int mode)
+{
+ struct inode *inode;
+
+{
+ struct inode *inode;
+
-+ inode = ext4_new_inode(handle, dir, mode, 0, EXT4_SB(dir->i_sb)->s_inode_goal);
++ inode = ext4_new_inode(handle, dir, mode, 0,
++ EXT4_SB(dir->i_sb)->s_inode_goal);
+ if (!IS_ERR(inode)) {
+ if (S_ISCHR(mode) || S_ISBLK(mode) || S_ISFIFO(mode)) {
+ if (!IS_ERR(inode)) {
+ if (S_ISCHR(mode) || S_ISBLK(mode) || S_ISFIFO(mode)) {
-+#ifdef CONFIG_LDISKFS_FS_XATTR
++#ifdef CONFIG_EXT4_FS_XATTR
+ inode->i_op = &ext4_special_inode_operations;
+#endif
+ } else {
+ inode->i_op = &ext4_special_inode_operations;
+#endif
+ } else {
+ inode->i_fop = &ext4_file_operations;
+ ext4_set_aops(inode);
+ }
+ inode->i_fop = &ext4_file_operations;
+ ext4_set_aops(inode);
+ }
-+ unlock_new_inode(inode);
/*
* By the time this is called, we already have created
* the directory cache entry for the new file, but it
/*
* By the time this is called, we already have created
* the directory cache entry for the new file, but it
-@@ -1882,44 +1906,32 @@ retry:
+@@ -1882,44 +1908,32 @@ retry:
de = (struct ext4_dir_entry_2 *) dir_block->b_data;
de->inode = cpu_to_le32(inode->i_ino);
de->name_len = 1;
de = (struct ext4_dir_entry_2 *) dir_block->b_data;
de->inode = cpu_to_le32(inode->i_ino);
de->name_len = 1;
-@@ -1938,18 +1950,47 @@ retry:
+@@ -1938,18 +1952,47 @@ retry:
BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata");
err = ext4_handle_dirty_metadata(handle, inode, dir_block);
if (err)
BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata");
err = ext4_handle_dirty_metadata(handle, inode, dir_block);
if (err)
ext4_inc_count(handle, dir);
ext4_update_dx_flag(dir);
err = ext4_mark_inode_dirty(handle, dir);
ext4_inc_count(handle, dir);
ext4_update_dx_flag(dir);
err = ext4_mark_inode_dirty(handle, dir);
-@@ -1958,11 +1999,16 @@ out_clear_inode:
+@@ -1958,11 +2001,16 @@ out_clear_inode:
d_instantiate(dentry, inode);
unlock_new_inode(inode);
out_stop:
d_instantiate(dentry, inode);
unlock_new_inode(inode);
out_stop:
/*
* DIR_NLINK feature is set if 1) nlinks > EXT4_LINK_MAX or 2) nlinks == 2,
* since this indicates that nlinks count was previously 1.
/*
* DIR_NLINK feature is set if 1) nlinks > EXT4_LINK_MAX or 2) nlinks == 2,
* since this indicates that nlinks count was previously 1.
-@@ -1808,6 +1811,28 @@ static unsigned ext4_dentry_goal(struct
+@@ -1808,6 +1811,29 @@ static unsigned ext4_dentry_goal(struct
-+struct inode *ext4_create_inode(handle_t *handle, struct inode * dir, int mode)
++ /* Return locked inode, then the caller can modify the inode's states/flags
++ * before others finding it. The caller should unlock the inode by itself. */
++struct inode *ext4_create_inode(handle_t *handle, struct inode *dir, int mode)
+{
+ struct inode *inode;
+
+{
+ struct inode *inode;
+
+ EXT4_SB(dir->i_sb)->s_inode_goal);
+ if (!IS_ERR(inode)) {
+ if (S_ISCHR(mode) || S_ISBLK(mode) || S_ISFIFO(mode)) {
+ EXT4_SB(dir->i_sb)->s_inode_goal);
+ if (!IS_ERR(inode)) {
+ if (S_ISCHR(mode) || S_ISBLK(mode) || S_ISFIFO(mode)) {
-+#ifdef CONFIG_LDISKFS_FS_XATTR
++#ifdef CONFIG_EXT4_FS_XATTR
+ inode->i_op = &ext4_special_inode_operations;
+#endif
+ } else {
+ inode->i_op = &ext4_special_inode_operations;
+#endif
+ } else {
+ inode->i_fop = &ext4_file_operations;
+ ext4_set_aops(inode);
+ }
+ inode->i_fop = &ext4_file_operations;
+ ext4_set_aops(inode);
+ }
-+ unlock_new_inode(inode);
/*
* By the time this is called, we already have created
* the directory cache entry for the new file, but it
/*
* By the time this is called, we already have created
* the directory cache entry for the new file, but it
-@@ -1886,46 +1911,32 @@ retry:
+@@ -1886,46 +1912,32 @@ retry:
de = (struct ext4_dir_entry_2 *) dir_block->b_data;
de->inode = cpu_to_le32(inode->i_ino);
de->name_len = 1;
de = (struct ext4_dir_entry_2 *) dir_block->b_data;
de->inode = cpu_to_le32(inode->i_ino);
de->name_len = 1;
-@@ -1944,18 +1955,47 @@ retry:
+@@ -1944,18 +1956,47 @@ retry:
BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata");
err = ext4_handle_dirty_metadata(handle, inode, dir_block);
if (err)
BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata");
err = ext4_handle_dirty_metadata(handle, inode, dir_block);
if (err)
ext4_inc_count(handle, dir);
ext4_update_dx_flag(dir);
err = ext4_mark_inode_dirty(handle, dir);
ext4_inc_count(handle, dir);
ext4_update_dx_flag(dir);
err = ext4_mark_inode_dirty(handle, dir);
-@@ -1964,11 +2004,16 @@ out_clear_inode:
+@@ -1964,11 +2005,16 @@ out_clear_inode:
d_instantiate(dentry, inode);
unlock_new_inode(inode);
out_stop:
d_instantiate(dentry, inode);
unlock_new_inode(inode);
out_stop:
osd_sb(osd)->s_root->d_inode,
mode);
if (!IS_ERR(inode)) {
osd_sb(osd)->s_root->d_inode,
mode);
if (!IS_ERR(inode)) {
- /* Do not update file c/mtime in ldiskfs.
- * NB: don't need any lock because no contention at this
- * early stage */
- inode->i_flags |= S_NOCMTIME;
+ /* Do not update file c/mtime in ldiskfs. */
+ inode->i_flags |= S_NOCMTIME;
/* For new created object, it must be consistent,
* and it is unnecessary to scrub against it. */
ldiskfs_set_inode_state(inode, LDISKFS_STATE_LUSTRE_NOSCRUB);
/* For new created object, it must be consistent,
* and it is unnecessary to scrub against it. */
ldiskfs_set_inode_state(inode, LDISKFS_STATE_LUSTRE_NOSCRUB);
obj->oo_inode = inode;
result = 0;
} else {
obj->oo_inode = inode;
result = 0;
} else {
result = osd_create_type_f(dof->dof_type)(info, obj, attr, hint, dof,
th);
result = osd_create_type_f(dof->dof_type)(info, obj, attr, hint, dof,
th);
osd_attr_init(info, obj, attr, dof);
osd_object_init0(obj);
osd_attr_init(info, obj, attr, dof);
osd_object_init0(obj);
- /* bz 24037 */
- if (obj->oo_inode && (obj->oo_inode->i_state & I_NEW))
- unlock_new_inode(obj->oo_inode);
- }
+ }
+
+ if (obj->oo_inode != NULL) {
+ LASSERT(obj->oo_inode->i_state & I_NEW);
+
+ unlock_new_inode(obj->oo_inode);
+ }
/* restore previous umask value */
current->fs->umask = umask;
/* restore previous umask value */
current->fs->umask = umask;
+ ldiskfs_set_inode_state(local, LDISKFS_STATE_LUSTRE_NOSCRUB);
+ unlock_new_inode(local);
+
/* Set special LMA flag for local agent inode */
rc = osd_ea_fid_set(info, local, fid, 0, LMAI_AGENT);
if (rc != 0) {
/* Set special LMA flag for local agent inode */
rc = osd_ea_fid_set(info, local, fid, 0, LMAI_AGENT);
if (rc != 0) {
+ ldiskfs_set_inode_state(inode, LDISKFS_STATE_LUSTRE_NOSCRUB);
+ unlock_new_inode(inode);
+
if (feat->dif_flags & DT_IND_VARKEY)
rc = iam_lvar_create(inode, feat->dif_keysize_max,
feat->dif_ptrsize, feat->dif_recsize_max,
if (feat->dif_flags & DT_IND_VARKEY)
rc = iam_lvar_create(inode, feat->dif_keysize_max,
feat->dif_ptrsize, feat->dif_recsize_max,