From: Andrew Morton <andrewm@uow.edu.au>
To: Daniel Phillips <phillips@bonn-fries.net>
Cc: Alexander Viro <viro@math.psu.edu>, lkml <linux-kernel@vger.kernel.org>
Subject: Re: [patch] truncate_inode_pages
Date: Mon, 11 Jun 2001 23:28:03 +1000 [thread overview]
Message-ID: <3B24C763.16E8E360@uow.edu.au> (raw)
In-Reply-To: <Pine.GSO.4.21.0106091331120.19361-100000@weyl.math.psu.edu> <01061018402300.05248@starship> <3B24BD57.E1D6D1D0@uow.edu.au>, <3B24BD57.E1D6D1D0@uow.edu.au> <01061115131301.05248@starship>
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);
}
next prev parent reply other threads:[~2001-06-11 13:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2001-06-09 15:51 [patch] truncate_inode_pages Andrew Morton
2001-06-09 17:40 ` Alexander Viro
2001-06-09 22:53 ` Daniel Phillips
2001-06-10 1:31 ` Andrew Morton
2001-06-10 16:40 ` Daniel Phillips
2001-06-11 12:45 ` Andrew Morton
2001-06-11 13:13 ` Daniel Phillips
2001-06-11 13:28 ` Andrew Morton [this message]
2001-06-11 2:22 Dieter Nützel
[not found] ` <200106112252.AAA19615@mail.bonn-fries.net>
[not found] ` <01061214324600.00879@starship>
2001-06-12 16:32 ` Dieter Nützel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3B24C763.16E8E360@uow.edu.au \
--to=andrewm@uow.edu.au \
--cc=linux-kernel@vger.kernel.org \
--cc=phillips@bonn-fries.net \
--cc=viro@math.psu.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).