[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Ognyan's libpager changes
From: |
Ognyan Kulev |
Subject: |
Re: Ognyan's libpager changes |
Date: |
Mon, 02 Aug 2004 19:29:36 +0300 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5 |
Neal H. Walfield wrote:
As a start to getting back into Hurd hacking mode, I have begun to
review your ext2fs patch for large file system support. At this
point, I have only reviewed the libpager changes in any detail. I
want to work with you to develop a well thought out interface and
implementation that not only does the job but is elegant and
represents something with which I feel comfortable in advocating
acceptance into the mainline.
Great! Please use version 20040421 from
http://debian.fmi.uni-sofia.bg/~ogi/hurd/ext3fs/ . It's not uploaded in
Savannah and contains some major changes in the core function that
remaps pages.
As such, I advise that we remove this extra flexibility and
have pager_notify_pageout (which I prefer to call pager_notify_evict
as data can be paged out without being evicted in the case of flushing
dirty pages) called on a _per pager_ basis and indicate whether or not
it should be called in the struct pager and set by pager_create. If
it is useful, we may also have a pair of get and set methods to toggle
it, however, the implementation of turning this feature on after a
pager has been executing for some time seems problematic: all pages
need to be marked as precious; I can't think of an easy way to do this
off hand.
Yes, in ext2fs, we need either all or none pages to be marked as
precious. I can't predict if there will be future pager users that will
want more fine-grained marking, so I've just taken this by-page
approach. In conclusion, I think it's OK this option to be pager-wide.
Second, Ognyan suggests that there is a race.
I wouldn't find it if I didn't experience it :-) BTW In complete hurd
build, this optimization turns 1-3 times, usually 1 time.
My sense is that this problem can be easily avoided by not sending a
pager_notify_pageout in data-return.c if PM_PAGEINWAIT is set thereby
eliminating all of the hubbub with PM_FORCEREAD and the inefficiencies
which is introduces. Or perhaps, I am missing some subtility.
This sounds reasonable. I need to think a bit more about it though.
Finally, when a pager_notify_evict is called on a page, the page is
potentially changed. Hence any state associated with the page must
also be changed. That is, its pagemap entry needs to be cleared
otherwise, a page which is marked PM_EIO is shows for the wrong page.
Right, I didn't catch this. The error will be lost, but libpager can do
nothing about it.
So what name should our pager-wide flag has? Following the current
patch and the pager_notify_pageout -> pager_notify_evict rename, I will
use notify_on_evict if noone has better proposal.
I'm close to releasing ext3fs 0.1, and after that I'll make these
changes and future ones that will eventually come up.
Regards,
ogi