On Mon, Jul 09, 2012 at 06:43:37PM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com>
> 
> This patch makes the high psizes mask as an unsigned char array
> so that we can have more than 16TB. Currently we support upto
> 64TB

Some comments inline...

> @@ -804,16 +804,19 @@ unsigned int hash_page_do_lazy_icache(unsigned int pp, 
> pte_t pte, int trap)
>  #ifdef CONFIG_PPC_MM_SLICES
>  unsigned int get_paca_psize(unsigned long addr)
>  {
> -     unsigned long index, slices;
> +     u64 lpsizes;
> +     unsigned char *hpsizes;
> +     unsigned long index, mask_index;
>  
>       if (addr < SLICE_LOW_TOP) {
> -             slices = get_paca()->context.low_slices_psize;
> +             lpsizes = get_paca()->context.low_slices_psize;
>               index = GET_LOW_SLICE_INDEX(addr);
> -     } else {
> -             slices = get_paca()->context.high_slices_psize;
> -             index = GET_HIGH_SLICE_INDEX(addr);
> +             return (lpsizes >> (index * 4)) & 0xF;
>       }
> -     return (slices >> (index * 4)) & 0xF;
> +     hpsizes = get_paca()->context.high_slices_psize;
> +     index = GET_HIGH_SLICE_INDEX(addr) >> 1;
> +     mask_index = GET_HIGH_SLICE_INDEX(addr) - (index << 1);
> +     return (hpsizes[index] >> (mask_index * 4)) & 0xF;

The last 3 lines here feel awkward.  How about:

> +     index = GET_HIGH_SLICE_INDEX(addr);
> +     mask_index = index & 1;
> +     return (hpsizes[index >> 1] >> (mask_index * 4)) & 0xF;

>  static struct slice_mask slice_mask_for_size(struct mm_struct *mm, int psize)
>  {
> +     unsigned char *hpsizes;
> +     int index, mask_index;
>       struct slice_mask ret = { 0, 0 };
>       unsigned long i;
> -     u64 psizes;
> +     u64 lpsizes;
>  
> -     psizes = mm->context.low_slices_psize;
> +     lpsizes = mm->context.low_slices_psize;
>       for (i = 0; i < SLICE_NUM_LOW; i++)
> -             if (((psizes >> (i * 4)) & 0xf) == psize)
> +             if (((lpsizes >> (i * 4)) & 0xf) == psize)
>                       ret.low_slices |= 1u << i;
>  
> -     psizes = mm->context.high_slices_psize;
> -     for (i = 0; i < SLICE_NUM_HIGH; i++)
> -             if (((psizes >> (i * 4)) & 0xf) == psize)
> +     hpsizes = mm->context.high_slices_psize;
> +     for (i = 0; i < SLICE_NUM_HIGH; i++) {
> +             index = i >> 1;
> +             mask_index = i - (index << 1);

Again, seems like a complicated way to do mask_index = i & 1 (or
even i % 2, if you prefer, but then make i an unsigned type).

Paul.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to