[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Fwd: [patch #2508] ext2fs support for large store (> 1.5G)]

From: Ognyan Kulev
Subject: Re: [Fwd: [patch #2508] ext2fs support for large store (> 1.5G)]
Date: Wed, 25 Feb 2004 10:35:00 +0200
User-agent: Mozilla Thunderbird 0.5 (X11/20040221)

Marco Gerards wrote:
disk_pager_read_page removes the RC_MAPPING bit from the flags and
notices the disk_cache_wait_remapping that it is safe to continue.
But you broadcast the condition before removing the bit.  I think this
can cause some problems, what do you think?

You're right, but this has no practical sequences, because condition_wait will continue after unlocking of disk_cache_lock in disk_pager_read_page. See below. Anyway, I'll change this (swap broadcast and clearing the flag) in RC2.

How about the loop that touches all pages?  I read touch as writing,
does it make pages dirty?  If that happens it is not acceptable.  when
it reads in a page that was evicted it is just an unfortunate waste of
resources but ok for me (but I do not think everyone will agree with
me).  Better fix it in Mach.

Here "touch" is used as "peek".  Il'l change that too.

And, really, Mach should be fixed. But I don't have time resource to do this, and Mach code is famous for being unpleasent to read.

When there are no pages free you just throw the entire cache away (if
I understand it correctly).  I do not like this because this can be
one of the main factors that will slow down someone's computer.  Isn't
it possible to ask Mach to evict 10 pages (for example)?  Mach usually
knows better which pages should be evicted.  If that is not possible
you can just evict 10 pages (10 was randomly chosen) and evict the
non-dirty pages first.

In the text, there is some rationale behind flushing all pages with reference count zero. Dirty pages are not flushed, because they have refcount > 0. (Dirty pages are always in some pokel or are synced before deref.)

There is another way for fine-grained flushing of pages. disk_cache_hint cycles through whole cache and is used for finding unreferenced block which is not in core (i.e. evicted). So next 5-10 unreferenced blocks after disk_cache_hint can be flushed (pager_return_some) and another try can be made. Synchronization between pager_notify_pageout and disk_cache_map is not clear to me right now, though. I'll try to implement it for RC2.

There is some possible locking problem:
The call disk_cache_wait_remapping is called with disk_cache_lock
locked.  The condition in disk_pager_read_page is broadcasted while
disk_cache_lock is locked too.

This is by design. condition_wait unlocks the lock and waits. When condition is broadcasted, it locks the lock again, potentially blocking the thread. In our case, this will make disk_cache_wait_remapping to wait until disk_pager_read_page unlocks disk_cache_lock.

Please be really careful with a global lock like disk_cache_lock.  It
cannot make it hard to prevent deadlocks, it can slow ext2fs down
too.  I do not know if there is another way though.

Yes, but in our case I see no way to avoid it too.

Have you fixed the bug I reported BTW?

No, unfortunately I still haven't found time for debugging :-( But this is something that must be fixed before RC2.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]