Branch b1_4

Don't get ll_inode_size_lock() in ll_update_inode() as this can be called
with inode_lock (spinlock) held and deadlock.  This was protecting the
setting of lli_smd to prevent ll_inode_size_unlock() from inconsistently
calling lov_stripe_unlock() when it was never locked because lli_smd changed
since ll_inode_size_lock() was called.

We now avoid this race by only ever calling ll_inode_size_lock() with
lli_smd already set, or with "lock_lsm = 0" so we don't care if it changes
between lock and unlock.  This makes sense in any case, because if there is
no lli_smd we shouldn't be doing glimpse/enqueue on the OSTs anyways.
b=9547
r=nikita

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 959ab94..fc64688 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -673,17 +673,19 @@ static int ll_glimpse_callback(struct ldlm_lock *lock, void *reqp)
 
 /* NB: lov_merge_size will prefer locally cached writes if they extend the
  * file (because it prefers KMS over RSS when larger) */
-int ll_glimpse_size(struct inode *inode)
+int ll_glimpse_size(struct inode *inode, int ast_flags)
 {
         struct ll_inode_info *lli = ll_i2info(inode);
         struct ll_sb_info *sbi = ll_i2sbi(inode);
         ldlm_policy_data_t policy = { .l_extent = { 0, OBD_OBJECT_EOF } };
         struct lustre_handle lockh = { 0 };
-        int rc, flags = LDLM_FL_HAS_INTENT;
+        int rc;
         ENTRY;
 
         CDEBUG(D_DLMTRACE, "Glimpsing inode %lu\n", inode->i_ino);
 
+        ast_flags |= LDLM_FL_HAS_INTENT;
+
         /* NOTE: this looks like DLM lock request, but it may not be one. Due
          *       to LDLM_FL_HAS_INTENT flag, this is glimpse request, that
          *       won't revoke any conflicting DLM locks held. Instead,
@@ -692,7 +694,7 @@ int ll_glimpse_size(struct inode *inode)
          *       will be returned for each stripe. DLM lock on [0, EOF] is
          *       acquired only if there were no conflicting locks. */
         rc = obd_enqueue(sbi->ll_osc_exp, lli->lli_smd, LDLM_EXTENT, &policy,
-                         LCK_PR, &flags, ll_extent_lock_callback,
+                         LCK_PR, &ast_flags, ll_extent_lock_callback,
                          ldlm_completion_ast, ll_glimpse_callback, inode,
                          sizeof(struct ost_lvb), lustre_swab_ost_lvb, &lockh);
         if (rc == -ENOENT)
@@ -727,6 +729,7 @@ int ll_extent_lock(struct ll_file_data *fd, struct inode *inode,
         ENTRY;
 
         LASSERT(lockh->cookie == 0);
+        LASSERT(lsm != NULL);
 
         /* don't drop the mmapped file to LRU */
         if (mapping_mapped(inode->i_mapping))
@@ -870,7 +873,7 @@ static ssize_t ll_file_read(struct file *file, char *buf, size_t count,
                 /* A glimpse is necessary to determine whether we return a
                  * short read (B) or some zeroes at the end of the buffer (C) */
                 ll_inode_size_unlock(inode, 1);
-                retval = ll_glimpse_size(inode);
+                retval = ll_glimpse_size(inode, 0);
                 if (retval)
                         goto out;
         } else {
@@ -1286,10 +1289,8 @@ int ll_file_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 loff_t ll_file_seek(struct file *file, loff_t offset, int origin)
 {
         struct inode *inode = file->f_dentry->d_inode;
-        struct ll_file_data *fd = LUSTRE_FPRIVATE(file);
         struct ll_inode_info *lli = ll_i2info(inode);
         struct lov_stripe_md *lsm = lli->lli_smd;
-        struct lustre_handle lockh = {0};
         loff_t retval;
         ENTRY;
         retval = offset + ((origin == 2) ? inode->i_size :
@@ -1300,16 +1301,16 @@ loff_t ll_file_seek(struct file *file, loff_t offset, int origin)
 
         lprocfs_counter_incr(ll_i2sbi(inode)->ll_stats, LPROC_LL_LLSEEK);
         if (origin == 2) { /* SEEK_END */
-                ldlm_policy_data_t policy = { .l_extent = {0, OBD_OBJECT_EOF }};
                 int nonblock = 0, rc;
 
                 if (file->f_flags & O_NONBLOCK)
                         nonblock = LDLM_FL_BLOCK_NOWAIT;
 
-                rc = ll_extent_lock(fd, inode, lsm, LCK_PR, &policy, &lockh,
-                                    nonblock);
-                if (rc != 0)
-                        RETURN(rc);
+                if (lsm != NULL) {
+                        rc = ll_glimpse_size(inode, nonblock);
+                        if (rc != 0)
+                                RETURN(rc);
+                }
 
                 ll_inode_size_lock(inode, 0);
                 offset += inode->i_size;
@@ -1330,8 +1331,6 @@ loff_t ll_file_seek(struct file *file, loff_t offset, int origin)
                 retval = offset;
         }
 
-        if (origin == 2)
-                ll_extent_unlock(fd, inode, lsm, LCK_PR, &lockh);
         RETURN(retval);
 }
 
@@ -1560,7 +1559,7 @@ int ll_inode_revalidate_it(struct dentry *dentry, struct lookup_intent *it)
 
         /* ll_glimpse_size will prefer locally cached writes if they extend
          * the file */
-        rc = ll_glimpse_size(inode);
+        rc = ll_glimpse_size(inode, 0);
         RETURN(rc);
 }
 

diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h
index 0c51732..3ddf759 100644 (file)
--- a/lustre/llite/llite_internal.h
+++ b/lustre/llite/llite_internal.h
@@ -361,7 +361,7 @@ int ll_extent_unlock(struct ll_file_data *, struct inode *,
 int ll_file_open(struct inode *inode, struct file *file);
 int ll_file_release(struct inode *inode, struct file *file);
 int ll_lsm_getattr(struct obd_export *, struct lov_stripe_md *, struct obdo *);
-int ll_glimpse_size(struct inode *inode);
+int ll_glimpse_size(struct inode *inode, int ast_flags);
 int ll_local_open(struct file *file,
                   struct lookup_intent *it, struct ll_file_data *fd);
 int ll_mdc_close(struct obd_export *mdc_exp, struct inode *inode,

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 554e229..acccc1e 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -871,11 +871,10 @@ void ll_clear_inode(struct inode *inode)
         clear_bit(LLI_F_HAVE_MDS_SIZE_LOCK, &(ll_i2info(inode)->lli_flags));
         mdc_change_cbdata(sbi->ll_mdc_exp, &fid, null_if_equal, inode);
 
-        if (lli->lli_smd)
+        if (lli->lli_smd) {
                 obd_change_cbdata(sbi->ll_osc_exp, lli->lli_smd,
                                   null_if_equal, inode);
 
-        if (lli->lli_smd) {
                 obd_free_memmd(sbi->ll_osc_exp, &lli->lli_smd);
                 lli->lli_smd = NULL;
         }
@@ -1180,7 +1179,9 @@ void ll_inode_size_lock(struct inode *inode, int lock_lsm)
         LASSERT(lli->lli_size_sem_owner == NULL);
         lli->lli_size_sem_owner = current;
         lsm = lli->lli_smd;
-        if (lsm != NULL && lock_lsm)
+        LASSERTF(lsm != NULL || lock_lsm == 0, "lsm %p, lock_lsm %d\n",
+                 lsm, lock_lsm);
+        if (lock_lsm)
                 lov_stripe_lock(lsm);
 }
 
@@ -1191,7 +1192,9 @@ void ll_inode_size_unlock(struct inode *inode, int unlock_lsm)
 
         lli = ll_i2info(inode);
         lsm = lli->lli_smd;
-        if (lsm != NULL && unlock_lsm)
+        LASSERTF(lsm != NULL || unlock_lsm == 0, "lsm %p, lock_lsm %d\n",
+                 lsm, unlock_lsm);
+        if (unlock_lsm)
                 lov_stripe_unlock(lsm);
         LASSERT(lli->lli_size_sem_owner == current);
         lli->lli_size_sem_owner = NULL;
@@ -1212,9 +1215,10 @@ void ll_update_inode(struct inode *inode, struct mds_body *body,
                         }
                         CDEBUG(D_INODE, "adding lsm %p to inode %lu/%u(%p)\n",
                                lsm, inode->i_ino, inode->i_generation, inode);
-                        ll_inode_size_lock(inode, 0);
+                        /* ll_inode_size_lock() requires it is only called
+                         * with lli_smd != NULL or lock_lsm == 0 or we can
+                         * race between lock/unlock.  bug 9547 */
                         lli->lli_smd = lsm;
-                        ll_inode_size_unlock(inode, 0);
                         lli->lli_maxbytes = lsm->lsm_maxbytes;
                         if (lli->lli_maxbytes > PAGE_CACHE_MAXBYTES)
                                 lli->lli_maxbytes = PAGE_CACHE_MAXBYTES;

diff --git a/lustre/llite/llite_mmap.c b/lustre/llite/llite_mmap.c
index 7ad228e..18c122d 100644 (file)
--- a/lustre/llite/llite_mmap.c
+++ b/lustre/llite/llite_mmap.c
@@ -404,7 +404,7 @@ struct page *ll_nopage(struct vm_area_struct *vma, unsigned long address,
 
         if (pgoff >= size) {
                 lov_stripe_unlock(lsm);
-                ll_glimpse_size(inode);
+                ll_glimpse_size(inode, 0);
         } else {
                 /* XXX change inode size without ll_inode_size_lock() held!
                  *     there is a race condition with truncate path. (see
@@ -600,7 +600,7 @@ int ll_file_mmap(struct file * file, struct vm_area_struct * vma)
                 vma->vm_ops = &ll_file_vm_ops;
                 vma->vm_ops->open(vma);
                 /* update the inode's size and mtime */
-                rc = ll_glimpse_size(file->f_dentry->d_inode);
+                rc = ll_glimpse_size(file->f_dentry->d_inode, 0);
         }
 
         RETURN(rc);