LU-6377 llite: clear LLIF_DATA_MODIFIED in atomic

This flag should be cleared atomically after the op_data flag
MDS_DATA_MODIFIED is packed. Otherwise, if there exists an
operation to dirty the file again, the state may be missed on
the MDT.

Stop using spin lock lli_lock to protect operations of changing
file flags; using bit operations instead.

Signed-off-by: Jinshan Xiong <jinshan.xiong@intel.com>
Signed-off-by: James Simmons <uja.ornl@yahoo.com>
Change-Id: I3a3129e24cea89b0ebaae1b7816d892b5623b3b4
Reviewed-on: http://review.whamcloud.com/14100
Tested-by: Maloo <hpdd-maloo@intel.com>
Tested-by: Jenkins
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 8b54068..20b8f95 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -86,46 +86,36 @@ static void ll_file_data_put(struct ll_file_data *fd)
                 OBD_SLAB_FREE_PTR(fd, ll_file_data_slab);
 }
 
-void ll_pack_inode2opdata(struct inode *inode, struct md_op_data *op_data,
-                          struct lustre_handle *fh)
-{
-        op_data->op_fid1 = ll_i2info(inode)->lli_fid;
-        op_data->op_attr.ia_mode = inode->i_mode;
-        op_data->op_attr.ia_atime = inode->i_atime;
-        op_data->op_attr.ia_mtime = inode->i_mtime;
-        op_data->op_attr.ia_ctime = inode->i_ctime;
-        op_data->op_attr.ia_size = i_size_read(inode);
-        op_data->op_attr_blocks = inode->i_blocks;
-       op_data->op_attr_flags = ll_inode_to_ext_flags(inode->i_flags);
-        if (fh)
-                op_data->op_handle = *fh;
-
-       if (LLIF_DATA_MODIFIED & ll_i2info(inode)->lli_flags)
-               op_data->op_bias |= MDS_DATA_MODIFIED;
-}
-
 /**
  * Packs all the attributes into @op_data for the CLOSE rpc.
  */
 static void ll_prepare_close(struct inode *inode, struct md_op_data *op_data,
                              struct obd_client_handle *och)
 {
-        ENTRY;
-
-       op_data->op_attr.ia_valid = ATTR_MODE | ATTR_ATIME | ATTR_ATIME_SET |
-                                       ATTR_MTIME | ATTR_MTIME_SET |
-                                       ATTR_CTIME | ATTR_CTIME_SET;
+       ENTRY;
 
-        if (!(och->och_flags & FMODE_WRITE))
-                goto out;
+       ll_prep_md_op_data(op_data, inode, NULL, NULL,
+                          0, 0, LUSTRE_OPC_ANY, NULL);
+
+       op_data->op_attr.ia_mode = inode->i_mode;
+       op_data->op_attr.ia_atime = inode->i_atime;
+       op_data->op_attr.ia_mtime = inode->i_mtime;
+       op_data->op_attr.ia_ctime = inode->i_ctime;
+       op_data->op_attr.ia_size = i_size_read(inode);
+       op_data->op_attr.ia_valid |= ATTR_MODE | ATTR_ATIME | ATTR_ATIME_SET |
+                                    ATTR_MTIME | ATTR_MTIME_SET |
+                                    ATTR_CTIME | ATTR_CTIME_SET;
+       op_data->op_attr_blocks = inode->i_blocks;
+       op_data->op_attr_flags = ll_inode_to_ext_flags(inode->i_flags);
+       op_data->op_handle = och->och_fh;
 
-       op_data->op_attr.ia_valid |= ATTR_SIZE | ATTR_BLOCKS;
+       if (och->och_flags & FMODE_WRITE &&
+           ll_file_test_and_clear_flag(ll_i2info(inode), LLIF_DATA_MODIFIED))
+               /* For HSM: if inode data has been modified, pack it so that
+                * MDT can set data dirty flag in the archive. */
+               op_data->op_bias |= MDS_DATA_MODIFIED;
 
-out:
-        ll_pack_inode2opdata(inode, op_data, &och->och_fh);
-        ll_prep_md_op_data(op_data, inode, NULL, NULL,
-                           0, 0, LUSTRE_OPC_ANY, NULL);
-        EXIT;
+       EXIT;
 }
 
 /**
@@ -194,16 +184,6 @@ static int ll_close_inode_openhandle(struct obd_export *md_exp,
                       PFID(ll_inode2fid(inode)), rc);
        }
 
-       /* DATA_MODIFIED flag was successfully sent on close, cancel data
-        * modification flag. */
-       if (rc == 0 && (op_data->op_bias & MDS_DATA_MODIFIED)) {
-               struct ll_inode_info *lli = ll_i2info(inode);
-
-               spin_lock(&lli->lli_lock);
-               lli->lli_flags &= ~LLIF_DATA_MODIFIED;
-               spin_unlock(&lli->lli_lock);
-       }
-
        if (rc == 0 &&
            op_data->op_bias & (MDS_HSM_RELEASE | MDS_CLOSE_LAYOUT_SWAP)) {
                struct mdt_body *body;
@@ -3202,7 +3182,7 @@ ll_inode_revalidate(struct dentry *dentry, __u64 ibits)
                 * restore the MDT holds the layout lock so the glimpse will
                 * block up to the end of restore (getattr will block)
                 */
-               if (!(ll_i2info(inode)->lli_flags & LLIF_FILE_RESTORING))
+               if (!ll_file_test_flag(ll_i2info(inode), LLIF_FILE_RESTORING))
                        rc = ll_glimpse_size(inode);
        }
        RETURN(rc);

diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h
index 030c468..a2b00ca 100644 (file)
--- a/lustre/llite/llite_internal.h
+++ b/lustre/llite/llite_internal.h
@@ -120,20 +120,11 @@ struct ll_grouplock {
        unsigned long    lg_gid;
 };
 
-enum lli_flags {
-       /* File data is modified. */
-       LLIF_DATA_MODIFIED      = 1 << 0,
-       /* File is being restored */
-       LLIF_FILE_RESTORING     = 1 << 1,
-       /* Xattr cache is attached to the file */
-       LLIF_XATTR_CACHE        = 1 << 2,
-};
-
 struct ll_inode_info {
        __u32                           lli_inode_magic;
-       __u32                           lli_flags;
-
        spinlock_t                      lli_lock;
+
+       volatile unsigned long          lli_flags;
        struct posix_acl                *lli_posix_acl;
 
        struct hlist_head               *lli_remote_perms;
@@ -281,6 +272,39 @@ static inline void ll_layout_version_set(struct ll_inode_info *lli, __u32 gen)
        spin_unlock(&lli->lli_layout_lock);
 }
 
+enum ll_file_flags {
+       /* File data is modified. */
+       LLIF_DATA_MODIFIED      = 0,
+       /* File is being restored */
+       LLIF_FILE_RESTORING     = 1,
+       /* Xattr cache is attached to the file */
+       LLIF_XATTR_CACHE        = 2,
+};
+
+static inline void ll_file_set_flag(struct ll_inode_info *lli,
+                                   enum ll_file_flags flag)
+{
+       set_bit(flag, &lli->lli_flags);
+}
+
+static inline void ll_file_clear_flag(struct ll_inode_info *lli,
+                                     enum ll_file_flags flag)
+{
+       clear_bit(flag, &lli->lli_flags);
+}
+
+static inline bool ll_file_test_flag(struct ll_inode_info *lli,
+                                    enum ll_file_flags flag)
+{
+       return test_bit(flag, &lli->lli_flags);
+}
+
+static inline bool ll_file_test_and_clear_flag(struct ll_inode_info *lli,
+                                              enum ll_file_flags flag)
+{
+       return test_and_clear_bit(flag, &lli->lli_flags);
+}
+
 int ll_xattr_cache_destroy(struct inode *inode);
 
 int ll_xattr_cache_get(struct inode *inode,
@@ -809,8 +833,6 @@ int ll_file_open(struct inode *inode, struct file *file);
 int ll_file_release(struct inode *inode, struct file *file);
 int ll_release_openhandle(struct dentry *, struct lookup_intent *);
 int ll_md_real_close(struct inode *inode, fmode_t fmode);
-void ll_pack_inode2opdata(struct inode *inode, struct md_op_data *op_data,
-                          struct lustre_handle *fh);
 extern void ll_rw_stats_tally(struct ll_sb_info *sbi, pid_t pid,
                               struct ll_file_data *file, loff_t pos,
                               size_t count, int rw);

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index ac62630..2b04fd5 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -1630,9 +1630,6 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr, bool hsm_import)
                        /* If we are changing file size, file content is
                         * modified, flag it. */
                        attr->ia_valid |= MDS_OPEN_OWNEROVERRIDE;
-                       spin_lock(&lli->lli_lock);
-                       lli->lli_flags |= LLIF_DATA_MODIFIED;
-                       spin_unlock(&lli->lli_lock);
                        op_data->op_bias |= MDS_DATA_MODIFIED;
                }
        }
@@ -1643,13 +1640,6 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr, bool hsm_import)
        if (rc)
                GOTO(out, rc);
 
-       /* RPC to MDT is sent, cancel data modification flag */
-       if (rc == 0 && (op_data->op_bias & MDS_DATA_MODIFIED)) {
-               spin_lock(&lli->lli_lock);
-               lli->lli_flags &= ~LLIF_DATA_MODIFIED;
-               spin_unlock(&lli->lli_lock);
-       }
-
        if (!S_ISREG(inode->i_mode) || file_is_released)
                GOTO(out, rc = 0);
 
@@ -1929,7 +1919,7 @@ int ll_update_inode(struct inode *inode, struct lustre_md *md)
 
        if (body->mbo_valid & OBD_MD_TSTATE) {
                if (body->mbo_t_state & MS_RESTORE)
-                       lli->lli_flags |= LLIF_FILE_RESTORING;
+                       ll_file_set_flag(lli, LLIF_FILE_RESTORING);
        }
 
        return 0;
@@ -2446,8 +2436,6 @@ struct md_op_data *ll_prep_md_op_data(struct md_op_data *op_data,
        op_data->op_fsuid = from_kuid(&init_user_ns, current_fsuid());
        op_data->op_fsgid = from_kgid(&init_user_ns, current_fsgid());
        op_data->op_cap = cfs_curproc_cap_pack();
-       op_data->op_bias = 0;
-       op_data->op_cli_flags = 0;
        if ((opc == LUSTRE_OPC_CREATE) && (name != NULL) &&
             filename_is_volatile(name, namelen, &op_data->op_mds)) {
                op_data->op_bias |= MDS_CREATE_VOLATILE;
@@ -2456,10 +2444,6 @@ struct md_op_data *ll_prep_md_op_data(struct md_op_data *op_data,
        }
        op_data->op_data = data;
 
-       /* When called by ll_setattr_raw, file is i1. */
-       if (LLIF_DATA_MODIFIED & ll_i2info(i1)->lli_flags)
-               op_data->op_bias |= MDS_DATA_MODIFIED;
-
        return op_data;
 }
 

diff --git a/lustre/llite/llite_mmap.c b/lustre/llite/llite_mmap.c
index bc41042..e7f57d6 100644 (file)
--- a/lustre/llite/llite_mmap.c
+++ b/lustre/llite/llite_mmap.c
@@ -243,11 +243,8 @@ static int ll_page_mkwrite0(struct vm_area_struct *vma, struct page *vmpage,
                         result = -EAGAIN;
                 }
 
-               if (result == 0) {
-                       spin_lock(&lli->lli_lock);
-                       lli->lli_flags |= LLIF_DATA_MODIFIED;
-                       spin_unlock(&lli->lli_lock);
-               }
+               if (result == 0)
+                       ll_file_set_flag(lli, LLIF_DATA_MODIFIED);
         }
         EXIT;
 

diff --git a/lustre/llite/vvp_io.c b/lustre/llite/vvp_io.c
index 1fae402..fb1cd65 100644 (file)
--- a/lustre/llite/vvp_io.c
+++ b/lustre/llite/vvp_io.c
@@ -351,8 +351,8 @@ static void vvp_io_fini(const struct lu_env *env, const struct cl_io_slice *ios)
                        /* today successful restore is the only possible
                         * case */
                        /* restore was done, clear restoring state */
-                       ll_i2info(vvp_object_inode(obj))->lli_flags &=
-                               ~LLIF_FILE_RESTORING;
+                       ll_file_clear_flag(ll_i2info(vvp_object_inode(obj)),
+                                          LLIF_FILE_RESTORING);
                }
        }
 }
@@ -1082,11 +1082,7 @@ static int vvp_io_write_start(const struct lu_env *env,
                }
        }
        if (result > 0) {
-               struct ll_inode_info *lli = ll_i2info(inode);
-
-               spin_lock(&lli->lli_lock);
-               lli->lli_flags |= LLIF_DATA_MODIFIED;
-               spin_unlock(&lli->lli_lock);
+               ll_file_set_flag(ll_i2info(inode), LLIF_DATA_MODIFIED);
 
                if (result < cnt)
                        io->ci_continue = 0;

diff --git a/lustre/llite/xattr_cache.c b/lustre/llite/xattr_cache.c
index 64bcbc1..1abd6ba 100644 (file)
--- a/lustre/llite/xattr_cache.c
+++ b/lustre/llite/xattr_cache.c
@@ -86,7 +86,7 @@ static void ll_xattr_cache_init(struct ll_inode_info *lli)
        LASSERT(lli != NULL);
 
        INIT_LIST_HEAD(&lli->lli_xattrs);
-       lli->lli_flags |= LLIF_XATTR_CACHE;
+       ll_file_set_flag(lli, LLIF_XATTR_CACHE);
 }
 
 /**
@@ -258,7 +258,7 @@ static int ll_xattr_cache_list(struct list_head *cache,
  */
 static int ll_xattr_cache_valid(struct ll_inode_info *lli)
 {
-       return !!(lli->lli_flags & LLIF_XATTR_CACHE);
+       return ll_file_test_flag(lli, LLIF_XATTR_CACHE);
 }
 
 /**
@@ -277,7 +277,8 @@ static int ll_xattr_cache_destroy_locked(struct ll_inode_info *lli)
 
        while (ll_xattr_cache_del(&lli->lli_xattrs, NULL) == 0)
                /* empty loop */ ;
-       lli->lli_flags &= ~LLIF_XATTR_CACHE;
+
+       ll_file_clear_flag(lli, LLIF_XATTR_CACHE);
 
        RETURN(0);
 }