lkml.org 
[lkml]   [2001]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch] truncate_inode_pages
Daniel Phillips wrote:
>
> On Monday 11 June 2001 14:45, Andrew Morton wrote:
> > Daniel Phillips wrote:
> > > On Sunday 10 June 2001 03:31, Andrew Morton wrote:
> > > > Daniel Phillips wrote:
> > > > > This is easy, just set the list head to the page about to be
> > > > > truncated.
> > > >
> > > > Works for me.
> > >
> > > It looks good, but it's black magic
> >
> > No, it's wrong. I'm getting BUG()s in clear_inode():
> > [...]
> > The lists are mangled.
>
> curr is being advanced in the wrong place.

Yes.

> /me makes note to self: never resist the temptation to clean things up the
> rest of the way
>
> I'll actually apply the patch and try it this time ;-)

The bug is surprisingly hard to trigger.


Take three:


--- linux-2.4.5/mm/filemap.c Mon May 28 13:31:49 2001
+++ linux-akpm/mm/filemap.c Mon Jun 11 23:31:08 2001
@@ -230,17 +230,17 @@
unsigned long offset;

page = list_entry(curr, struct page, list);
- curr = curr->next;
offset = page->index;

/* Is one of the pages to truncate? */
if ((offset >= start) || (*partial && (offset + 1) == start)) {
+ list_del(head);
+ list_add(head, curr);
if (TryLockPage(page)) {
page_cache_get(page);
spin_unlock(&pagecache_lock);
wait_on_page(page);
- page_cache_release(page);
- return 1;
+ goto out_restart;
}
page_cache_get(page);
spin_unlock(&pagecache_lock);
@@ -252,11 +252,15 @@
truncate_complete_page(page);

UnlockPage(page);
- page_cache_release(page);
- return 1;
+ goto out_restart;
}
+ curr = curr->next;
}
return 0;
+out_restart:
+ page_cache_release(page);
+ spin_lock(&pagecache_lock);
+ return 1;
}


@@ -273,15 +277,19 @@
{
unsigned long start = (lstart + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
unsigned partial = lstart & (PAGE_CACHE_SIZE - 1);
+ int complete;

-repeat:
spin_lock(&pagecache_lock);
- if (truncate_list_pages(&mapping->clean_pages, start, &partial))
- goto repeat;
- if (truncate_list_pages(&mapping->dirty_pages, start, &partial))
- goto repeat;
- if (truncate_list_pages(&mapping->locked_pages, start, &partial))
- goto repeat;
+ do {
+ complete = 1;
+ while (truncate_list_pages(&mapping->clean_pages, start, &partial))
+ complete = 0;
+ while (truncate_list_pages(&mapping->dirty_pages, start, &partial))
+ complete = 0;
+ while (truncate_list_pages(&mapping->locked_pages, start, &partial))
+ complete = 0;
+ } while (!complete);
+ /* Traversed all three lists without dropping the lock */
spin_unlock(&pagecache_lock);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-03-22 12:55    [W:0.021 / U:15.864 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site