You're certainly not the only one, and certainly not the worst offender.
I do agree: I recommended that same split on a previous occasion.
Actually, it's three patches: you seem to prefer one style over
another in cow_user_page, I don't care either way, go ahead and
make the change, but it's nothing to do with the rest of it.
Like would be a little strong, but I certainly don't dislike:
you're right that the current old-established way is somewhat
contingent, do go ahead and make these Uptodates more consistent.
So far as I could tell, it's correct on top of -rc8-mm1 despite the
fuzz - unless we've added more places where a mod is needed, but I
don't think so.
I'm testing with that now, mainly because an earlier version hit a
BUG or WARNing with shmem: I wanted to check that no longer happens.
It doesn't, but then I got curious why not: I believe the problem
line was shmem_getpage's
(!PageUptodate(filepage) || TestSetPageLocked(filepage)))
which gave trouble when you had a PageLocked check inside PageUptodate.
You don't have that now because you're not at this point trying to push
the PageLocked shortcuts; but worth noting if you resurrect those later.
I do like this version _so_ much more than your earlier attempts to
avoid the overhead wherever you could argue it. Those might become
more acceptable once we've grown accustomed to this initial set.
Thanks for doing the FAQs: I think in the meantime I'd already
persuaded myself that you're right that PageUptodate is the
proper place for this stuff, even if I regret the complication.
Four little points. In the comments, "preceding" not "preceeding"
and I presume "wmb" not "smb". You've carried over several "(page)"s
from the macros which would better be "page"s in inline functions.
And do we really need that smp_wmb in __SetPageUptodate? It seems
to me that when you're in a position to use __SetPageUptodate, the
page cannot yet be visible, and the necessary barrier will be
provided later when the page is made visible.
I suspect the answer is that I'm confusing two different kinds of
visibility; and that although it's true that we don't actually need
that smp_wmb in the places where __SetPageUptodate is being used,
it'd be hard to write and remember the rules to justify its removal.
But mention it because you may well see this more clearly.
Hugh
--
