Re: [PATCH 1/3] riscv: Introduce huge page support for 32/64bit kernel

Christoph Hellwig
2019-01-15T16:11:04Z

> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
> +static inline bool gigantic_page_supported(void) { return true; }

Can you move each brace and the return statement to an line of its
own? Just to keep the code a little easier to read.

> +int pud_huge(pud_t pud)
> +{
> + return pud_present(pud)
> + && (pud_val(pud) & (_PAGE_READ | _PAGE_WRITE | _PAGE_EXEC));
> +}
> +
> +int pmd_huge(pmd_t pmd)
> +{
> + return pmd_present(pmd)
> + && (pmd_val(pmd) & (_PAGE_READ | _PAGE_WRITE | _PAGE_EXEC));
> +}

Kinda sad that we can't easily have these as inline functions, but not
really your fault. Note that the && should move to the first line
of the multi-line statement.

> +#if defined(CONFIG_64BIT)
> + } else if (ps == PUD_SIZE) {
> + hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> +#endif

I think this could be:

} else if (IS_ENABLED(CONFIG_64BIT) && ps == PUD_SIZE) {
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);