[Top][All Lists]

[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.


reply via email to

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