On Mon, 5 May 2008, Nick Piggin wrote:
>
> Index: linux-2.6/include/asm-x86/pgtable_32.h
> ===================================================================
> --- linux-2.6.orig/include/asm-x86/pgtable_32.h
> +++ linux-2.6/include/asm-x86/pgtable_32.h
> @@ -133,7 +133,12 @@ extern int pmd_bad(pmd_t pmd);
> * pgd_offset() returns a (pgd_t *)
> * pgd_index() is used get the offset into the pgd page's array of pgd_t's;
> */
> -#define pgd_offset(mm, address) ((mm)->pgd + pgd_index((address)))
> +#define pgd_offset(mm, address) \
> +({ \
> + pgd_t *ret = ((mm)->pgd + pgd_index((address))); \
> + smp_read_barrier_depends(); /* see mm/memory.c:__pte_alloc */ \
> + ret; \
> +})
Is there some fundamental reason this needs to be a macro?
It is really ugly, and it would be much nicer to make this an inline
function if at all possible.
Yeah, maybe it requires some more #include's, but ..
(Especially since it apparently gets worse, and the pgd load needs a
ACCESS_ONCE() too - the code generated is the same, but the source gets
more and more involved)
That said, I *also* think that it's sad that you do this at all, since
smp_read_barrier_depends() is a no-op on x86, so why should we have it in
an x86-specific header file?
In short, I think the fixes are real, but the patch itself is really just
confusing things for no apparent good reason.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
