Question about free_one_pgd() changes in these 3.5G patches

From: Ron Niles (Ron.Niles@falconstor.com)
Date: Tue Jul 15 2003 - 19:52:36 EST


I'm trying to expand my kernel memory space to 3GB and am checking out these
patches. I'm kind of mystified by the changes to free_one_pgd in these
patches, both the one that Chuck Luciano recently posted and the one in AA
(00_3.5G-address-space-4). Both of these seem to change the loop from:

        int j;
        ...

        for (j = 0; j < PTRS_PER_PMD ; j++) {
                 prefetchw(pmd+j+(PREFETCH_STRIDE/16));
                free_one_pmd(pmd+j);
        }
to:

        pmd_t * pmd, * md, * emd;
        ...

        /*
         * Beware if changing the loop below. It once used int j,
         * for (j = 0; j < PTRS_PER_PMD; j++)
         * free_one_pmd(pmd+j);
         * but some older i386 compilers (e.g. egcs-2.91.66, gcc-2.95.3)
         * terminated the loop with a _signed_ address comparison
         * using "jle", when configured for HIGHMEM64GB (X86_PAE).
         * If also configured for 3GB of kernel virtual address space,
         * if page at physical 0x3ffff000 virtual 0x7ffff000 is used as
         * a pmd, when that mm exits the loop goes on to free "entries"
         * found at 0x80000000 onwards. The loop below compiles instead
         * to be terminated by unsigned address comparison using "jb".

        for (md = pmd, emd = pmd + PTRS_PER_PMD; md < emd; md++) {
                prefetchw(md+(PREFETCH_STRIDE/16));
                free_one_pmd(md);
         }

The comment (found in the AA patch) makes no sense to me. Since j is an int,
you would expect the loop to exit with jle. If you want it to exit on jb,
just change j to unsigned, right? Also PTRS_PER_PMD is never very large,
around 512 I think, so it really doesn't matter unless PTRS_PER_PMD exceeds
0x7fffffff, which is really far from reality.

Secondly, the new code is dangerous if the pmd happens to be on the page at
the top of memory. In this case pmd is something like 0xfffff000, emd = pmd
+ PTRS_PER_PMD rolls over to zero, and the loop never gets executed since md
is never less than zero.

It seems to me the change is unnecessary, but if it is needed, it should
protect against rollover on the top of memory page, assuming PTRS_PER_PMD is
never zero:

        for (md = pmd, emd = pmd + PTRS_PER_PMD - 1; md <= emd; md++)

Do we guarantee that the top of memory page is never used and the rollover
is impossible? Even so, am I missing something as to why this change is
necessary?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Jul 15 2003 - 22:01:01 EST