Messages in this thread Patch in this message |  | | Date | Thu, 14 Sep 2000 12:12:17 -0500 | From | Bill Wendling <> | Subject | Re: [PATCH 2.4.0-test8] mm/filemap.c |
| |
Also sprach Juan J. Quintela: } >>>>> "bill" == Bill Wendling <wendling@ganymede.isdn.uiuc.edu> writes: } } Hi } } Linus, please don't apply. } } bill> - The `head = &mapping->pages;' statement is useless inside the } bill> repeat, since head isn't modified inside the loop. } } No, but we sleep inside the loop, and while we sleep, we don't have } locked the page cache :(((( } As David pointed out, it's only necessary if mapping->pages changes. But, then, shouldn't the head = &mapping->pages statement be within the spinlock?
} If you think that the for is nicer (I think that the while is easier } to read, but that is question of taste). } It's not really a question of taste. The for loop does the increment after the block of code. Doing it beforehand is a waste if the repeat is taken.
Attached is a new patch which just addresses this issue and leaves the head = &mapping->pages thingy alone...
-- || Bill Wendling wendling@ganymede.isdn.uiuc.edu --- linux-2.4.0-test8/mm/filemap.c Sat Sep 9 02:35:09 2000 +++ linux-2.4.0-test8-new/mm/filemap.c Thu Sep 14 12:09:21 2000 @@ -193,12 +193,10 @@ repeat: head = &mapping->pages; spin_lock(&pagecache_lock); - curr = head->next; - while (curr != head) { + list_for_each(curr, head) { unsigned long offset; page = list_entry(curr, struct page, list); - curr = curr->next; offset = page->index; /* Is one of the pages to truncate? */ |  |