Re: [PATCH v2 11/11] powerpc/mm: drop slice DEBUG

[Christophe Leroy <christophe.leroy@c-s.fr>]

Le 26/04/2019 à 08:44, Aneesh Kumar K.V a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> 
>> slice is now an improved functionnality. Drop the DEBUG stuff.
>>
> 
> I would like to keep that. I helped a lot when moving address ranges and
> it should not have any runtime impact.

Ok for me.

Christophe

> 
> 
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/mm/slice.c | 62 ++++---------------------------------------------
>>   1 file changed, 4 insertions(+), 58 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
>> index 97fbf7b54422..a9d803738b65 100644
>> --- a/arch/powerpc/mm/slice.c
>> +++ b/arch/powerpc/mm/slice.c
>> @@ -41,28 +41,6 @@
>>   
>>   static DEFINE_SPINLOCK(slice_convert_lock);
>>   
>> -#ifdef DEBUG
>> -int _slice_debug = 1;
>> -
>> -static void slice_print_mask(const char *label, const struct slice_mask *mask)
>> -{
>> -	if (!_slice_debug)
>> -		return;
>> -	pr_devel("%s low_slice: %*pbl\n", label,
>> -			(int)SLICE_NUM_LOW, &mask->low_slices);
>> -	pr_devel("%s high_slice: %*pbl\n", label,
>> -			(int)SLICE_NUM_HIGH, mask->high_slices);
>> -}
>> -
>> -#define slice_dbg(fmt...) do { if (_slice_debug) pr_devel(fmt); } while (0)
>> -
>> -#else
>> -
>> -static void slice_print_mask(const char *label, const struct slice_mask *mask) {}
>> -#define slice_dbg(fmt...)
>> -
>> -#endif
>> -
>>   static inline bool slice_addr_is_low(unsigned long addr)
>>   {
>>   	u64 tmp = (u64)addr;
>> @@ -207,9 +185,6 @@ static void slice_convert(struct mm_struct *mm,
>>   	unsigned long i, flags;
>>   	int old_psize;
>>   
>> -	slice_dbg("slice_convert(mm=%p, psize=%d)\n", mm, psize);
>> -	slice_print_mask(" mask", mask);
>> -
>>   	psize_mask = slice_mask_for_size(&mm->context, psize);
>>   
>>   	/* We need to use a spinlock here to protect against
>> @@ -255,10 +230,6 @@ static void slice_convert(struct mm_struct *mm,
>>   				(((unsigned long)psize) << (mask_index * 4));
>>   	}
>>   
>> -	slice_dbg(" lsps=%lx, hsps=%lx\n",
>> -		  (unsigned long)mm_ctx_low_slices(&mm->context),
>> -		  (unsigned long)mm_ctx_high_slices(&mm->context));
>> -
>>   	spin_unlock_irqrestore(&slice_convert_lock, flags);
>>   
>>   	copro_flush_all_slbs(mm);
>> @@ -485,14 +456,9 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>   	BUG_ON(mm_ctx_slb_addr_limit(&mm->context) == 0);
>>   	VM_BUG_ON(radix_enabled());
>>   
>> -	slice_dbg("slice_get_unmapped_area(mm=%p, psize=%d...\n", mm, psize);
>> -	slice_dbg(" addr=%lx, len=%lx, flags=%lx, topdown=%d\n",
>> -		  addr, len, flags, topdown);
>> -
>>   	/* If hint, make sure it matches our alignment restrictions */
>>   	if (!fixed && addr) {
>>   		addr = _ALIGN_UP(addr, page_size);
>> -		slice_dbg(" aligned addr=%lx\n", addr);
>>   		/* Ignore hint if it's too large or overlaps a VMA */
>>   		if (addr > high_limit - len || addr < mmap_min_addr ||
>>   		    !slice_area_is_free(mm, addr, len))
>> @@ -538,17 +504,12 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>   		slice_copy_mask(&good_mask, maskp);
>>   	}
>>   
>> -	slice_print_mask(" good_mask", &good_mask);
>> -	if (compat_maskp)
>> -		slice_print_mask(" compat_mask", compat_maskp);
>> -
>>   	/* First check hint if it's valid or if we have MAP_FIXED */
>>   	if (addr != 0 || fixed) {
>>   		/* Check if we fit in the good mask. If we do, we just return,
>>   		 * nothing else to do
>>   		 */
>>   		if (slice_check_range_fits(mm, &good_mask, addr, len)) {
>> -			slice_dbg(" fits good !\n");
>>   			newaddr = addr;
>>   			goto return_addr;
>>   		}
>> @@ -558,13 +519,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>   		 */
>>   		newaddr = slice_find_area(mm, len, &good_mask,
>>   					  psize, topdown, high_limit);
>> -		if (newaddr != -ENOMEM) {
>> -			/* Found within the good mask, we don't have to setup,
>> -			 * we thus return directly
>> -			 */
>> -			slice_dbg(" found area at 0x%lx\n", newaddr);
>> +
>> +		/* Found within good mask, don't have to setup, thus return directly */
>> +		if (newaddr != -ENOMEM)
>>   			goto return_addr;
>> -		}
>>   	}
>>   	/*
>>   	 * We don't fit in the good mask, check what other slices are
>> @@ -572,11 +530,9 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>   	 */
>>   	slice_mask_for_free(mm, &potential_mask, high_limit);
>>   	slice_or_mask(&potential_mask, &potential_mask, &good_mask);
>> -	slice_print_mask(" potential", &potential_mask);
>>   
>>   	if (addr != 0 || fixed) {
>>   		if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
>> -			slice_dbg(" fits potential !\n");
>>   			newaddr = addr;
>>   			goto convert;
>>   		}
>> @@ -586,18 +542,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>   	if (fixed)
>>   		return -EBUSY;
>>   
>> -	slice_dbg(" search...\n");
>> -
>>   	/* If we had a hint that didn't work out, see if we can fit
>>   	 * anywhere in the good area.
>>   	 */
>>   	if (addr) {
>>   		newaddr = slice_find_area(mm, len, &good_mask,
>>   					  psize, topdown, high_limit);
>> -		if (newaddr != -ENOMEM) {
>> -			slice_dbg(" found area at 0x%lx\n", newaddr);
>> +		if (newaddr != -ENOMEM)
>>   			goto return_addr;
>> -		}
>>   	}
>>   
>>   	/* Now let's see if we can find something in the existing slices
>> @@ -618,8 +570,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>   		return -ENOMEM;
>>   
>>   	slice_range_to_mask(newaddr, len, &potential_mask);
>> -	slice_dbg(" found potential area at 0x%lx\n", newaddr);
>> -	slice_print_mask(" mask", &potential_mask);
>>   
>>    convert:
>>   	/*
>> @@ -697,8 +647,6 @@ void slice_init_new_context_exec(struct mm_struct *mm)
>>   	struct slice_mask *mask;
>>   	unsigned int psize = mmu_virtual_psize;
>>   
>> -	slice_dbg("slice_init_new_context_exec(mm=%p)\n", mm);
>> -
>>   	/*
>>   	 * In the case of exec, use the default limit. In the
>>   	 * case of fork it is just inherited from the mm being
>> @@ -730,8 +678,6 @@ void slice_setup_new_exec(void)
>>   {
>>   	struct mm_struct *mm = current->mm;
>>   
>> -	slice_dbg("slice_setup_new_exec(mm=%p)\n", mm);
>> -
>>   	if (!is_32bit_task())
>>   		return;
>>   
>> -- 
>> 2.13.3