On Tue, 22 Mar 2005, David S. Miller wrote:
> On Tue, 22 Mar 2005 21:51:39 +0000 (GMT)
> Hugh Dickins <[EMAIL PROTECTED]> wrote:
> 
> > I still can't see what's wrong with the code that's already
> > there.  My brain is seizing up, I'm taking a break.
> 
> Ok, meanwhile I'll do a brain dump of what I think this
> code should be doing.
> 
> Let's take an example free_pgd_range() call.  Say the
> address parameters are:
> 
> addr  0x10000
> end   0xa4000
> floor 0x00000
> ceiling       0xb2000

This actual example helped to focus my mind a lot, thank you.

> (This example comes from my exit_mmap() VMA dump earlier
>  in this thread.  If you disable the VMA skipping optimization
>  the first call to free_pgd_range() has these parameters.)
> 
> What ought this free_pgd_range() call do?  This range of
> addresses, from floor to ceiling, is smaller than a PMD_SIZE
> (which on sparc64 is 1 << 23).  Therefore it should clear
> no PGD or PUD entries.

Yup, it ought to decide at the beginning of free_pgd_range
that it simply has no work to do.

> Yet, it does clear them, specifically:
> 
> free_pgd_range():
>       1) mask addr (0x10000) to PMD_MASK, addr is now 0
>       2) addr < floor (0x00000) test does not pass
>       3) mask ceiling (0xb2000) to PMD_MASK, ceiling is now 0 too
>       4) end - 1 > ceiling - 1 test does not pass
>       5) addr > end - 1 test does not pass either

And now we've gone wrong, yes.

> The source of the problems seems to be how ceiling began
> at the top of the call chain as 0xb2000, but when we
> masked it with PMD_MASK that set it to zero, which means
> "top of address space" in these functions.  That's not
> what we want.
> 
> I added a quick hack to the simulator I posted, where
> we mask ceiling in free_pgd_range(), I do it like this:
> 
>       if (ceiling) {
>               ceiling &= PMD_MASK;
>               if (!ceiling)
>                       return;
>       }

At first that just looked like a hack to me.  But on reflection,
no, you're doing exactly what I had to do with addr above: in the
case where we arrive at 0 from non-0 value, have to get out quick
to avoid confusion with the "other" 0.  These wrap issues are hard.

And in other mail I see you found more such checks were needed.
I believe you've got it, thank you so much!

Though frankly, by now, I'm sure of nothing:
will review in the morning.

> and things seem to behave.  I'll try to analyze things
> further and test this out on a real kernel, but all of
> these adjustments at the top of free_pgd_range() really
> start to look like pure spaghetti. :-)

Well, it's trying to decide in reasonably few steps that it's not
worth wasting time going down to the deeper levels.  Lots of
"return"s as it eliminates cases, yes.

Hugh
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to