On Tue, 18 Apr 2000, Rik van Riel wrote:
>Think about eg. multiple truncates on the same inode...
Oh don't worry about that, that can't happen by design. That's what the
VFS should be good for.
>> This will make your machine sloowww and you also won't be able
>> to shrink the cache at a certain point and you'll swapout even
>> more at that point. Just try this:
>>
>> find / -type f -exec cp {} /dev/null \; >/dev/null
>
>I'm running it now (with my latest version of the patch,
>which is attached below) and performance of the rest of the
>machine is not impacted by the continuous copy...
Fun, you do another completly different patch and then you say that it
wasn't true the old patch was going to harm :). Did you ever tried your
previous patch in first place?
>I'm interested in real-world cases where anybody manages to
Try your previous patch (not the new one) and then you'll see what
happens in real world.
>--- linux-2.3.99-pre6-3/mm/filemap.c.orig Mon Apr 17 12:21:46 2000
>+++ linux-2.3.99-pre6-3/mm/filemap.c Tue Apr 18 15:08:08 2000
>@@ -149,11 +149,16 @@
>
> /* page wholly truncated - free it */
> if (offset >= start) {
>+ if (TryLockPage(page)) {
>+ spin_unlock(&pagecache_lock);
>+ get_page(page);
>+ wait_on_page(page);
>+ put_page(page);
>+ goto repeat;
>+ }
> get_page(page);
> spin_unlock(&pagecache_lock);
>
>- lock_page(page);
>-
> if (!page->buffers || block_flushpage(page, 0))
> lru_cache_del(page);
>
There's no need of this stuff. Also it's not the cleaner thing you can do
since you may end increasing the page count of a page outside the page
cache (since you release the lock first). It should work fine but again
it's not the cleaner thing we can do there. And it's not necessary in
first place.
>@@ -233,7 +243,18 @@
> page = list_entry(page_lru, struct page, lru);
> list_del(page_lru);
>
>+ /* What?! A page is in the LRU queue of another zone?! */
>+ if (!memclass(page->zone, zone))
>+ BUG();
>+
>+ count--;
>+
The lru should be global. It make no sense to keep in 16mbyte of cache
very obsolete stuff. The fix is to put the lru into the NUMA structure
instead of in the zone. That's another thing I want to fix.
It make perfect sense to take the lru into the numa struct because we may
want to shrink and then allocate from a certain node for performance
reasons (it's near) (we do need that stuff in the long term even if we
couldn't use it now) but it make no sense to take obsolete stuff into the
DMA memory, as worse we do the opposite, so shrinking first the DMA zone
(to try to keep DMA free) and then the regular zone!
> dispose = &zone->lru_cache;
>+ /* the page is in use, we can't free it now */
>+ if (!page->buffers && page_count(page) > 1)
>+ goto dispose_continue;
you put busy pages into the lru and you'll lockup as soon as only busy
pages will remains into the lru.
>+
>+ dispose = &old;
> if (test_and_clear_bit(PG_referenced, &page->flags))
> /* Roll the page at the top of the lru list,
> * we could also be more aggressive putting
You now enterely broke the lru cache aging.
Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sun Apr 23 2000 - 21:00:14 EST