bug-hurd
[Top][All Lists]
Advanced

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

Re: Ognyan's libpager changes


From: Neal H. Walfield
Subject: Re: Ognyan's libpager changes
Date: Mon, 02 Aug 2004 12:35:23 -0400
User-agent: Wanderlust/2.8.1 (Something) SEMI/1.14.3 (Ushinoya) FLIM/1.14.3 (Unebigoryƍmae) APEL/10.6 Emacs/21.2 (i386-debian-linux-gnu) MULE/5.0 (SAKAKI)

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

This is indeed what I am working with.

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

I find it hard to imagine a situation where this would be terribly
useful.  And should we encounter that situation, then we can consider
fully and make the most appropriate changes to that API at that time.
In the mean time, I think the pager wide approach is better as it
saves some cycles and as a bonus is less invasive.

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

Sorry.  You are absolutely right: as implemented, there certainly is a
race.  However, I think the race can be avoided using a different
approach.  Namely, by not calling the pager_notify_evict.

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

Right, as will any other state.  I.e. for completeness, we should
reset the page map entry when a eviction notification is sent.

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

This is perfectly reasonable.

> I'm close to releasing ext3fs 0.1, and after that I'll make these 
> changes and future ones that will eventually come up.

Great, thanks.

Neal




reply via email to

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