Messages in this thread | ![/](/images/icornerl.gif) | | From | Ron Niles <> | Subject | RE: Question about free_one_pgd() changes in these 3.5G patches | Date | Wed, 16 Jul 2003 09:23:43 -0400 |
| |
>On Tue, 15 Jul 2003, Ron Niles wrote: >> >> /* >> * 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. > >That comment (and the rewritten loop) originally came from me. >I thought it was a champion comment, I'm saddened that you disagree! > >I've tried to cover the point by saying they terminated the loop with >"a _signed_ address comparison": the loop got optimized in such a way >that it wasn't testing int j as the C shows, but the address pmd+j. >
Thanks, Hugh, it _is_ a champion comment, and it makes perfect sense now that I understand the compiler-optimized comparison is on pmd+j, not j. - 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/
| ![\](/images/icornerr.gif) |