From: adilger Date: Tue, 25 Oct 2005 19:57:43 +0000 (+0000) Subject: Branch b1_4 X-Git-Tag: v1_7_100~1^103~4^2~260^2~8 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=18f2a63128eacce338efe61adb7ef69d3342791d;p=fs%2Flustre-release.git 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 --- 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 --- 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 --- 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 --- 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);