On Tue, 2015-07-21 at 12:28 +0530, Anshuman Khandual wrote:
> From: "khand...@linux.vnet.ibm.com" <khand...@linux.vnet.ibm.com>
>
> This patch adds the following helper functions to improve modularization
> and readability of the code.
>
> (1) slb_invalid_all: Invalidates entire SLB
This reads badly. Although invalid can be a verb, the meaning of invalid as a
verb is not correct here. You want "invalidate".
> (2) slb_invalid_paca_slots: Invalidate SLB entries present in PACA
Ditto.
But, I think that's the wrong abstraction.
We should just have one routine, slb_invalidate(), which deals with all the
mess. ie. checking the MMU_FTR and the offset etc. So basically the whole
if/else.
> (3) kernel_linear_vsid_flags: VSID flags for kernel linear mapping
> (4) kernel_virtual_vsid_flags: VSID flags for kernel virtual mapping
I don't like these names either.
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index cbeaaa2..dcba4c2 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -94,18 +94,37 @@ static inline void new_shadowed_slbe(unsigned long ea,
> int ssize,
> : "memory" );
> }
>
> +static inline unsigned long kernel_linear_vsid_flags(void)
> +{
> + return SLB_VSID_KERNEL | mmu_psize_defs[mmu_linear_psize].sllp;
> +}
mmu_linear_vsid_flags() ?
> +
> +static inline unsigned long kernel_virtual_vsid_flags(void)
> +{
> + return SLB_VSID_KERNEL | mmu_psize_defs[mmu_vmalloc_psize].sllp;
> +}
mmu_vmalloc_vsid_flags() ?
etc.
ie. have the function names match the mmu psize names. I don't think we need
"kernel" in the name, I think that's implied.
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
