From 67786af2fa8faa488fc85606a42b7066c0dbc32d Mon Sep 17 00:00:00 2001 From: adilger Date: Thu, 4 Mar 2004 08:33:01 +0000 Subject: [PATCH] Fix three problems related to lock extent revocation: - we were incorrectly calculating the "end" index value to be at the beginning of the next stripe instead of at the end of the current stripe (extra +1 for end, instead of using <= end for page indices) - when comparing end << PAGE_CACHE_SHIFT for overflow against extent->end we used an unsigned long (which overflowed during the test ;-) so we always evicted pages to EOF (may have been a culprit in bug 2106 also) - when matching against other locks (to see if page is under 2 locks) we were doing so in the file offset space (from page->index) instead of in the osc/extent space so we were matching against the wrong locks, if any Change LL_CDEBUG_PAGE() to take a debug mask, so we can use it for error cases. b=2765 --- lustre/llite/file.c | 62 ++++++++++++++++++++++++------------------- lustre/llite/llite_internal.h | 6 ++--- lustre/llite/rw.c | 21 ++++++++------- lustre/llite/rw24.c | 8 +++--- lustre/llite/rw26.c | 8 +++--- 5 files changed, 57 insertions(+), 48 deletions(-) diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 710c637..64579b4 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -324,7 +324,6 @@ static int ll_lock_to_stripe_offset(struct inode *inode, struct ldlm_lock *lock) void ll_pgcache_remove_extent(struct inode *inode, struct lov_stripe_md *lsm, struct ldlm_lock *lock, __u32 stripe) { - struct ldlm_extent *extent = &lock->l_policy_data.l_extent; ldlm_policy_data_t tmpex; unsigned long start, end, count, skip, i, j; struct page *page; @@ -332,48 +331,49 @@ void ll_pgcache_remove_extent(struct inode *inode, struct lov_stripe_md *lsm, struct lustre_handle lockh; ENTRY; - CDEBUG(D_INODE, "obdo %lu inode %p ["LPU64"->"LPU64"] size: %llu\n", - inode->i_ino, inode, extent->start, extent->end, inode->i_size); + memcpy(&tmpex, &lock->l_policy_data.l_extent, sizeof(tmpex)); + CDEBUG(D_INODE|D_PAGE, "inode %lu(%p) ["LPU64"->"LPU64"] size: %llu\n", + inode->i_ino, inode, tmpex.l_extent.start, tmpex.l_extent.end, + inode->i_size); /* our locks are page granular thanks to osc_enqueue, we invalidate the * whole page. */ - LASSERT((extent->start & ~PAGE_CACHE_MASK) == 0); - LASSERT(((extent->end+1) & ~PAGE_CACHE_MASK) == 0); + LASSERT((tmpex.l_extent.start & ~PAGE_CACHE_MASK) == 0); + LASSERT(((tmpex.l_extent.end + 1) & ~PAGE_CACHE_MASK) == 0); - start = extent->start >> PAGE_CACHE_SHIFT; count = ~0; skip = 0; - end = (extent->end >> PAGE_CACHE_SHIFT) + 1; - if ((end << PAGE_CACHE_SHIFT) < extent->end) - end = ~0; + start = tmpex.l_extent.start >> PAGE_CACHE_SHIFT; + end = tmpex.l_extent.end >> PAGE_CACHE_SHIFT; if (lsm->lsm_stripe_count > 1) { count = lsm->lsm_stripe_size >> PAGE_CACHE_SHIFT; skip = (lsm->lsm_stripe_count - 1) * count; - start += (start/count * skip) + (stripe * count); + start += start/count * skip + stripe * count; if (end != ~0) - end += (end/count * skip) + (stripe * count); + end += end/count * skip + stripe * count; } + if (end < tmpex.l_extent.end >> PAGE_CACHE_SHIFT) + end = ~0; i = (inode->i_size + PAGE_CACHE_SIZE-1) >> PAGE_CACHE_SHIFT; if (i < end) end = i; - CDEBUG(D_INODE, "walking page indices start: %lu j: %lu count: %lu " - "skip: %lu end: %lu%s\n", start, start % count, count, skip, end, - discard ? " (DISCARDING)" : ""); + CDEBUG(D_INODE|D_PAGE, "walking page indices start: %lu j: %lu " + "count: %lu skip: %lu end: %lu%s\n", start, start % count, + count, skip, end, discard ? " (DISCARDING)" : ""); /* this is the simplistic implementation of page eviction at * cancelation. It is careful to get races with other page * lockers handled correctly. fixes from bug 20 will make it * more efficient by associating locks with pages and with * batching writeback under the lock explicitly. */ - for (i = start, j = start % count ; ; j++, i++) { - if (j == count) { - i += skip; - j = 0; - } - if (i >= end) - break; + for (i = start, j = start % count ; i <= end; + tmpex.l_extent.start += PAGE_CACHE_SIZE, j++, i++) { + LASSERTF(tmpex.l_extent.start< lock->l_policy_data.l_extent.end, + LPU64" >= "LPU64" start %lu i %lu end %lu\n", + tmpex.l_extent.start, lock->l_policy_data.l_extent.end, + start, i, end); ll_pgcache_lock(inode->i_mapping); if (list_empty(&inode->i_mapping->dirty_pages) && @@ -389,14 +389,14 @@ void ll_pgcache_remove_extent(struct inode *inode, struct lov_stripe_md *lsm, page = find_get_page(inode->i_mapping, i); if (page == NULL) - continue; - LL_CDEBUG_PAGE(page, "locking page\n"); + goto next_index; + LL_CDEBUG_PAGE(D_PAGE, page, "locking page\n"); lock_page(page); /* page->mapping to check with racing against teardown */ if (page->mapping && PageDirty(page) && !discard) { ClearPageDirty(page); - LL_CDEBUG_PAGE(page, "found dirty\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "found dirty\n"); ll_pgcache_lock(inode->i_mapping); list_del(&page->list); list_add(&page->list, &inode->i_mapping->locked_pages); @@ -411,7 +411,6 @@ void ll_pgcache_remove_extent(struct inode *inode, struct lov_stripe_md *lsm, lock_page(page); } - tmpex.l_extent.start = (__u64)page->index << PAGE_CACHE_SHIFT; tmpex.l_extent.end = tmpex.l_extent.start + PAGE_CACHE_SIZE - 1; /* check to see if another DLM lock covers this page */ rc2 = ldlm_lock_match(lock->l_resource->lr_namespace, @@ -421,11 +420,17 @@ void ll_pgcache_remove_extent(struct inode *inode, struct lov_stripe_md *lsm, &tmpex, LCK_PR | LCK_PW, &lockh); if (rc2 == 0 && page->mapping != NULL) { // checking again to account for writeback's lock_page() - LL_CDEBUG_PAGE(page, "truncating\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "truncating\n"); ll_truncate_complete_page(page); } unlock_page(page); page_cache_release(page); + next_index: + if (j == count) { + i += skip; + j = 0; + } + } EXIT; } @@ -624,8 +629,10 @@ int ll_glimpse_size(struct inode *inode, struct ost_lvb *lvb) LCK_PR, &flags, ll_extent_lock_callback, ldlm_completion_ast, ll_glimpse_callback, inode, sizeof(*lvb), lustre_swab_ost_lvb, &lockh); - if (rc > 0) + if (rc > 0) { + CERROR("obd_enqueue returned rc %d, returning -EIO\n", rc); RETURN(-EIO); + } lvb->lvb_size = lov_merge_size(lli->lli_smd, 0); //inode->i_mtime = lov_merge_mtime(lli->lli_smd, inode->i_mtime); @@ -1241,6 +1248,7 @@ static int ll_have_md_lock(struct dentry *de) CDEBUG(D_INFO, "trying to match res "LPU64"\n", res_id.name[0]); + /* FIXME use LDLM_FL_TEST_LOCK instead */ flags = LDLM_FL_BLOCK_GRANTED | LDLM_FL_CBPENDING; if (ldlm_lock_match(obddev->obd_namespace, flags, &res_id, LDLM_PLAIN, NULL, LCK_PR, &lockh)) { diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h index 84c910f..80cac34 100644 --- a/lustre/llite/llite_internal.h +++ b/lustre/llite/llite_internal.h @@ -115,9 +115,9 @@ struct ll_async_page { struct list_head llap_proc_item; }; -#define LL_CDEBUG_PAGE(page, STR) \ - CDEBUG(D_PAGE, "page %p map %p ind %lu priv %0lx: " STR, \ - page, page->mapping, page->index, page->private) +#define LL_CDEBUG_PAGE(mask, page, fmt, arg...) \ + CDEBUG(mask, "page %p map %p ind %lu priv %0lx: " fmt, \ + page, page->mapping, page->index, page->private, ## arg) /* llite/lproc_llite.c */ int lprocfs_register_mountpoint(struct proc_dir_entry *parent, diff --git a/lustre/llite/rw.c b/lustre/llite/rw.c index 08ae222..4d6df97 100644 --- a/lustre/llite/rw.c +++ b/lustre/llite/rw.c @@ -250,7 +250,7 @@ static int ll_ap_make_ready(void *data, int cmd) if (TryLockPage(page)) RETURN(-EAGAIN); - LL_CDEBUG_PAGE(page, "made ready\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "made ready\n"); page_cache_get(page); /* if we left PageDirty we might get another writepage call @@ -439,7 +439,7 @@ free_oig: oig_release(oig); GOTO(out, rc); } - LL_CDEBUG_PAGE(page, "write queued\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "write queued\n"); //llap_write_pending(inode, llap); } else { lprocfs_counter_incr(ll_i2sbi(inode)->ll_stats, @@ -483,7 +483,7 @@ void ll_removepage(struct page *page) return; } - LL_CDEBUG_PAGE(page, "being evicted\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "being evicted\n"); exp = ll_i2obdexp(inode); if (exp == NULL) { @@ -536,7 +536,8 @@ static int ll_page_matches(struct page *page) &page_extent, LCK_PR | LCK_PW, &flags, inode, &match_lockh); if (matches < 0) - LL_CDEBUG_PAGE(page, "lock match failed\n"); + LL_CDEBUG_PAGE(D_ERROR, page, "lock match failed: rc %d\n", + matches); RETURN(matches); } @@ -553,7 +554,7 @@ static int ll_issue_page_read(struct obd_export *exp, NULL, oig, llap->llap_cookie, OBD_BRW_READ, 0, PAGE_SIZE, 0, ASYNC_COUNT_STABLE); if (rc) { - LL_CDEBUG_PAGE(page, "read queueing failed\n"); + LL_CDEBUG_PAGE(D_ERROR, page, "read queue failed: rc %d\n", rc); page_cache_release(page); } RETURN(rc); @@ -620,10 +621,10 @@ static void ll_readahead(struct ll_readahead_state *ras, rc = ll_issue_page_read(exp, llap, oig, 1); if (rc == 0) - LL_CDEBUG_PAGE(page, "started read-ahead\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "started read-ahead\n"); if (rc) { next_page: - LL_CDEBUG_PAGE(page, "skipping read-ahead\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "skipping read-ahead\n"); unlock_page(page); } @@ -763,7 +764,7 @@ int ll_readpage(struct file *filp, struct page *page) ll_readahead(&fd->fd_ras, exp, page->mapping, oig); obd_trigger_group_io(exp, ll_i2info(inode)->lli_smd, NULL, oig); - LL_CDEBUG_PAGE(page, "marking uptodate from defer\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "marking uptodate from defer\n"); SetPageUptodate(page); unlock_page(page); GOTO(out_oig, rc = 0); @@ -798,7 +799,7 @@ int ll_readpage(struct file *filp, struct page *page) if (rc) GOTO(out, rc); - LL_CDEBUG_PAGE(page, "queued readpage\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "queued readpage\n"); if ((ll_i2sbi(inode)->ll_flags & LL_SBI_READAHEAD)) ll_readahead(&fd->fd_ras, exp, page->mapping, oig); @@ -839,7 +840,7 @@ int ll_sync_page(struct page *page) if (IS_ERR(llap)) RETURN(PTR_ERR(llap)); - LL_CDEBUG_PAGE(page, "setting ready|urgent\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "setting ready|urgent\n"); rc = obd_set_async_flags(exp, ll_i2info(page->mapping->host)->lli_smd, NULL, llap->llap_cookie, diff --git a/lustre/llite/rw24.c b/lustre/llite/rw24.c index 23be231..c645abd 100644 --- a/lustre/llite/rw24.c +++ b/lustre/llite/rw24.c @@ -71,11 +71,11 @@ void ll_ap_completion_24(void *data, int cmd, int rc) } else { llap->llap_write_queued = 0; } - } else { + } else { SetPageError(page); } - LL_CDEBUG_PAGE(page, "io complete, unlocking\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "io complete, unlocking\n"); unlock_page(page); @@ -108,7 +108,7 @@ static int ll_writepage_24(struct page *page) page_cache_get(page); if (llap->llap_write_queued) { - LL_CDEBUG_PAGE(page, "marking urgent\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "marking urgent\n"); rc = obd_set_async_flags(exp, ll_i2info(inode)->lli_smd, NULL, llap->llap_cookie, ASYNC_READY | ASYNC_URGENT); @@ -118,7 +118,7 @@ static int ll_writepage_24(struct page *page) llap->llap_cookie, OBD_BRW_WRITE, 0, 0, 0, ASYNC_READY | ASYNC_URGENT); if (rc == 0) - LL_CDEBUG_PAGE(page, "mmap write queued\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "mmap write queued\n"); else llap->llap_write_queued = 0; } diff --git a/lustre/llite/rw26.c b/lustre/llite/rw26.c index 21e884f..640cf05 100644 --- a/lustre/llite/rw26.c +++ b/lustre/llite/rw26.c @@ -73,11 +73,11 @@ void ll_ap_completion_26(void *data, int cmd, int rc) } else { llap->llap_write_queued = 0; } - } else { + } else { SetPageError(page); } - LL_CDEBUG_PAGE(page, "io complete, unlocking\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "io complete, unlocking\n"); unlock_page(page); @@ -110,7 +110,7 @@ static int ll_writepage_26(struct page *page, struct writeback_control *wbc) page_cache_get(page); if (llap->llap_write_queued) { - LL_CDEBUG_PAGE(page, "marking urgent\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "marking urgent\n"); rc = obd_set_async_flags(exp, ll_i2info(inode)->lli_smd, NULL, llap->llap_cookie, ASYNC_READY | ASYNC_URGENT); @@ -120,7 +120,7 @@ static int ll_writepage_26(struct page *page, struct writeback_control *wbc) llap->llap_cookie, OBD_BRW_WRITE, 0, 0, 0, ASYNC_READY | ASYNC_URGENT); if (rc == 0) - LL_CDEBUG_PAGE(page, "mmap write queued\n"); + LL_CDEBUG_PAGE(D_PAGE, page, "mmap write queued\n"); else llap->llap_write_queued = 0; } -- 1.8.3.1