On Mon, 23 May 2005, William Lee Irwin III wrote:
On Mon, May 23, 2005 at 05:14:06PM -0700, Andrew Morton wrote:
I must say that I continue to find this approach a bit queazifying.
After some reading of the code I'd agree that yes, it's not possible for us
to get here with `pte' pointing at the first slot of the pte page, but it's
not 100% obvious and it's possible that someone will come along later and
will change things in try_to_unmap_cluster() which cause this unmap to
suddenly do the wrong thing in rare circumstances.
IOW: I'd sleep better at night if we took a temporary and actually unmapped
the thing which we we got back from pte_offset_map().. Am I being silly?
There's a similar argument for queasiness in all the other (8 or more)
instances of the idiom. I think we originally adopted (and I furthered)
this pte_unmap(pte - 1) idiom because in the majority of architecture's
configurations pte_unmap does nothing at all, so we resented assigning
a pointless variable in some critical loops.
Not at all. I merely attempt to minimize diffsize by default. An
alternative implementation follows (changelog etc. to be taken
from the prior patch) in case it saves the time (however short) needed
to write it yourself.
Either of wli's patches is fine with me. There are several levels on
which try_to_unmap_cluster is harder to understand than the others,
and no good reason to resist the variable assignment.
We could rewrite pte_unmap to avoid the issue completely, since its
job is to unmap (or pretend to unmap) KM_PTE0's pte if the address
is in the fixmap area: but changing it to tolerate an off-by-one
address gives a queasy feeling too.