Whamcloud - gitweb
Fix three problems related to lock extent revocation:
authoradilger <adilger>
Thu, 4 Mar 2004 08:33:01 +0000 (08:33 +0000)
committeradilger <adilger>
Thu, 4 Mar 2004 08:33:01 +0000 (08:33 +0000)
- 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
lustre/llite/llite_internal.h
lustre/llite/rw.c
lustre/llite/rw24.c
lustre/llite/rw26.c

index 710c637..64579b4 100644 (file)
@@ -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)) {
index 84c910f..80cac34 100644 (file)
@@ -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,
index 08ae222..4d6df97 100644 (file)
@@ -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,
index 23be231..c645abd 100644 (file)
@@ -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;
         }
index 21e884f..640cf05 100644 (file)
@@ -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;
         }