From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3zNFG61YHDzF0bN for ; Fri, 19 Jan 2018 20:02:37 +1100 (AEDT) Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w0J900V3108205 for ; Fri, 19 Jan 2018 04:02:35 -0500 Received: from e38.co.us.ibm.com (e38.co.us.ibm.com [32.97.110.159]) by mx0a-001b2d01.pphosted.com with ESMTP id 2fkcfxk42p-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 19 Jan 2018 04:02:35 -0500 Received: from localhost by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 19 Jan 2018 02:02:34 -0700 Subject: Re: [PATCH v2 1/5] powerpc/mm: Enhance 'slice' for supporting PPC32 To: Christophe LEROY , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Scott Wood Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org References: <49148d07955d3e5f963cedf9adcfcc37c3e03ef4.1516179904.git.christophe.leroy@c-s.fr> <87vafyz265.fsf@linux.vnet.ibm.com> <84dc1df4-db2f-be11-c1f3-5dddd1e44983@c-s.fr> From: "Aneesh Kumar K.V" Date: Fri, 19 Jan 2018 14:32:15 +0530 MIME-Version: 1.0 In-Reply-To: <84dc1df4-db2f-be11-c1f3-5dddd1e44983@c-s.fr> Content-Type: text/plain; charset=utf-8; format=flowed Message-Id: List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 01/19/2018 02:14 PM, Christophe LEROY wrote: > > > Le 19/01/2018 à 09:24, Aneesh Kumar K.V a écrit : >> Christophe Leroy writes: >> >>> In preparation for the following patch which will fix an issue on >>> the 8xx by re-using the 'slices', this patch enhances the >>> 'slices' implementation to support 32 bits CPUs. >>> >>> On PPC32, the address space is limited to 4Gbytes, hence only the low >>> slices will be used. As of today, the code uses >>> SLICE_LOW_TOP (0x100000000ul) and compares it with addr to determine >>> if addr refers to low or high space. >>> On PPC32, such a (addr < SLICE_LOW_TOP) test is always false because >>> 0x100000000ul degrades to 0. Therefore, the patch modifies >>> SLICE_LOW_TOP to (0xfffffffful) and modifies the tests to >>> (addr <= SLICE_LOW_TOP) which will then always be true on PPC32 >>> as addr has type 'unsigned long' while not modifying the PPC64 >>> behaviour. >>> >>> This patch moves "slices" functions prototypes from page64.h to page.h >>> >>> The high slices use bitmaps. As bitmap functions are not prepared to >>> handling bitmaps of size 0, the bitmap_xxx() calls are wrapped into >>> slice_bitmap_xxx() macros which will take care of the 0 nbits case. >>> >>> Signed-off-by: Christophe Leroy >>> --- >>>   v2: First patch of v1 serie split in two parts ; added >>> slice_bitmap_xxx() macros. >>> >>>   arch/powerpc/include/asm/page.h      | 14 +++++++++ >>>   arch/powerpc/include/asm/page_32.h   | 19 ++++++++++++ >>>   arch/powerpc/include/asm/page_64.h   | 21 ++----------- >>>   arch/powerpc/mm/hash_utils_64.c      |  2 +- >>>   arch/powerpc/mm/mmu_context_nohash.c |  7 +++++ >>>   arch/powerpc/mm/slice.c              | 60 >>> ++++++++++++++++++++++++------------ >>>   6 files changed, 83 insertions(+), 40 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/page.h >>> b/arch/powerpc/include/asm/page.h >>> index 8da5d4c1cab2..d0384f9db9eb 100644 >>> --- a/arch/powerpc/include/asm/page.h >>> +++ b/arch/powerpc/include/asm/page.h >>> @@ -342,6 +342,20 @@ typedef struct page *pgtable_t; >>>   #endif >>>   #endif >>> +#ifdef CONFIG_PPC_MM_SLICES >>> +struct mm_struct; >>> + >>> +unsigned long slice_get_unmapped_area(unsigned long addr, unsigned >>> long len, >>> +                      unsigned long flags, unsigned int psize, >>> +                      int topdown); >>> + >>> +unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr); >>> + >>> +void slice_set_user_psize(struct mm_struct *mm, unsigned int psize); >>> +void slice_set_range_psize(struct mm_struct *mm, unsigned long start, >>> +               unsigned long len, unsigned int psize); >>> +#endif >>> + >> >> Should we do a slice.h ? the way we have other files? and then do > > Yes we could add a slice.h instead of using page.h for that, good idea. > >> >> arch/powerpc/include/asm/book3s/64/slice.h that will carry >> #define slice_bitmap_zero(dst, nbits) \ >>     do { if (nbits) bitmap_zero(dst, nbits); } while (0) >> #define slice_bitmap_set(dst, pos, nbits) \ >> do { if (nbits) bitmap_set(dst, pos, nbits); } while (0) >> #define slice_bitmap_copy(dst, src, nbits) \ >> do { if (nbits) bitmap_copy(dst, src, nbits); } while (0) >> #define slice_bitmap_and(dst, src1, src2, nbits) \ >>     ({ (nbits) ? bitmap_and(dst, src1, src2, nbits) : 0; }) >> #define slice_bitmap_or(dst, src1, src2, nbits) \ >>     do { if (nbits) bitmap_or(dst, src1, src2, nbits); } while (0) >> #define slice_bitmap_andnot(dst, src1, src2, nbits) \ >>     ({ (nbits) ? bitmap_andnot(dst, src1, src2, nbits) : 0; }) >> #define slice_bitmap_equal(src1, src2, nbits) \ >>     ({ (nbits) ? bitmap_equal(src1, src2, nbits) : 1; }) >> #define slice_bitmap_empty(src, nbits) \ >>     ({ (nbits) ? bitmap_empty(src, nbits) : 1; }) >> >> This without that if(nbits) check and a proper static inline so that we >> can do type checking. > > Is it really worth duplicating that just for eliminating the 'if > (nbits)' in one case ? > > Only in book3s/64 we will be able to eliminate that, for nohash/32 we > need to keep the test due to the difference between low and high slices. the other advantage is we move the SLICE_LOW_SHIFT to the right location. IMHO mm subystem is really complex with these really overloaded headers. If we can keep it seperate we should with minimal code duplication? > > In any case, as the nbits we use in slice.c is a constant, the test is > eliminated at compilation, so I can't see the benefit of making -aneesh