LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 09/10] mm, cma: move output param to the end of param list
From: Joonsoo Kim @ 2014-06-12  7:43 UTC (permalink / raw)
  To: Minchan Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, linux-mm, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <20140612071930.GL12415@bbox>

On Thu, Jun 12, 2014 at 04:19:31PM +0900, Minchan Kim wrote:
> On Thu, Jun 12, 2014 at 12:21:46PM +0900, Joonsoo Kim wrote:
> > Conventionally, we put output param to the end of param list.
> > cma_declare_contiguous() doesn't look like that, so change it.
> 
> If you says "Conventionally", I'd like to suggest one more thing.
> Conventionally, we put 'base' ahead 'size' but dma_contiguous_reserve_area
> is opposite.

Okay. I will do it. :)

Thanks.

^ permalink raw reply

* Re: [PATCH v2 10/10] mm, cma: use spinlock instead of mutex
From: Minchan Kim @ 2014-06-12  7:40 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, linux-mm, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <1402543307-29800-11-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12, 2014 at 12:21:47PM +0900, Joonsoo Kim wrote:
> Currently, we should take the mutex for manipulating bitmap.
> This job may be really simple and short so we don't need to sleep
> if contended. So I change it to spinlock.

I'm not sure it would be good always.
Maybe you remember we discussed about similar stuff about bitmap
searching in vmap friend internally, which was really painful
when it was fragmented. So, at least we need number if you really want
and I hope the number from ARM machine most popular platform for CMA
at the moment.

> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> diff --git a/mm/cma.c b/mm/cma.c
> index 22a5b23..3085e8c 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -27,6 +27,7 @@
>  #include <linux/memblock.h>
>  #include <linux/err.h>
>  #include <linux/mm.h>
> +#include <linux/spinlock.h>
>  #include <linux/mutex.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> @@ -36,7 +37,7 @@ struct cma {
>  	unsigned long	count;
>  	unsigned long	*bitmap;
>  	int order_per_bit; /* Order of pages represented by one bit */
> -	struct mutex	lock;
> +	spinlock_t	lock;
>  };
>  
>  /*
> @@ -72,9 +73,9 @@ static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
>  	bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
>  	nr_bits = cma_bitmap_pages_to_bits(cma, count);
>  
> -	mutex_lock(&cma->lock);
> +	spin_lock(&cma->lock);
>  	bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> -	mutex_unlock(&cma->lock);
> +	spin_unlock(&cma->lock);
>  }
>  
>  static int __init cma_activate_area(struct cma *cma)
> @@ -112,7 +113,7 @@ static int __init cma_activate_area(struct cma *cma)
>  		init_cma_reserved_pageblock(pfn_to_page(base_pfn));
>  	} while (--i);
>  
> -	mutex_init(&cma->lock);
> +	spin_lock_init(&cma->lock);
>  	return 0;
>  
>  err:
> @@ -261,11 +262,11 @@ struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
>  	nr_bits = cma_bitmap_pages_to_bits(cma, count);
>  
>  	for (;;) {
> -		mutex_lock(&cma->lock);
> +		spin_lock(&cma->lock);
>  		bitmapno = bitmap_find_next_zero_area(cma->bitmap,
>  					bitmap_maxno, start, nr_bits, mask);
>  		if (bitmapno >= bitmap_maxno) {
> -			mutex_unlock(&cma->lock);
> +			spin_unlock(&cma->lock);
>  			break;
>  		}
>  		bitmap_set(cma->bitmap, bitmapno, nr_bits);
> @@ -274,7 +275,7 @@ struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
>  		 * our exclusive use. If the migration fails we will take the
>  		 * lock again and unmark it.
>  		 */
> -		mutex_unlock(&cma->lock);
> +		spin_unlock(&cma->lock);
>  
>  		pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
>  		mutex_lock(&cma_mutex);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply

* Re: [PATCH v2 10/10] mm, cma: use spinlock instead of mutex
From: Joonsoo Kim @ 2014-06-12  7:56 UTC (permalink / raw)
  To: Minchan Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, linux-mm, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <20140612074029.GB12663@bbox>

On Thu, Jun 12, 2014 at 04:40:29PM +0900, Minchan Kim wrote:
> On Thu, Jun 12, 2014 at 12:21:47PM +0900, Joonsoo Kim wrote:
> > Currently, we should take the mutex for manipulating bitmap.
> > This job may be really simple and short so we don't need to sleep
> > if contended. So I change it to spinlock.
> 
> I'm not sure it would be good always.
> Maybe you remember we discussed about similar stuff about bitmap
> searching in vmap friend internally, which was really painful
> when it was fragmented. So, at least we need number if you really want
> and I hope the number from ARM machine most popular platform for CMA
> at the moment.

Good Point!! Agreed. I will drop this one in next spin and re-submit
in separate patchset after some testing.

Thanks.

^ permalink raw reply

* Re: [PATCH v2] powerpc: Avoid circular dependency with zImage.%
From: Michal Marek @ 2014-06-12  8:09 UTC (permalink / raw)
  To: Mike Qiu; +Cc: Sam Ravnborg, linuxppc-dev, linux-kernel, linux-kbuild
In-Reply-To: <53990B48.8010902@linux.vnet.ibm.com>

Dne 12.6.2014 04:07, Mike Qiu napsal(a):
> This v2 patch is good,
> 
> Tested-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>

Thanks. In kbuild.git it goes.

Michal

^ permalink raw reply

* [PATCH 1/4] KVM: PPC: Book3S PR: Fix ABIv2 issues
From: Anton Blanchard @ 2014-06-12  8:14 UTC (permalink / raw)
  To: benh, paulus, agraf, clg; +Cc: linuxppc-dev, kvm, kvm-ppc

binutils knows that a branch to a function descriptor is actually
a branch to the function text. By removing the explicit branch to
dot symbols, we maintain both ABIv1 and ABIv2 compatibility.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: b/arch/powerpc/kvm/book3s_interrupts.S
===================================================================
--- a/arch/powerpc/kvm/book3s_interrupts.S
+++ b/arch/powerpc/kvm/book3s_interrupts.S
@@ -25,11 +25,9 @@
 #include <asm/exception-64s.h>
 
 #if defined(CONFIG_PPC_BOOK3S_64)
-#define FUNC(name) 		GLUE(.,name)
 #define GET_SHADOW_VCPU(reg)    addi	reg, r13, PACA_SVCPU
 
 #elif defined(CONFIG_PPC_BOOK3S_32)
-#define FUNC(name)		name
 #define GET_SHADOW_VCPU(reg)	lwz     reg, (THREAD + THREAD_KVM_SVCPU)(r2)
 
 #endif /* CONFIG_PPC_BOOK3S_XX */
@@ -93,7 +91,7 @@ kvm_start_entry:
 kvm_start_lightweight:
 	/* Copy registers into shadow vcpu so we can access them in real mode */
 	GET_SHADOW_VCPU(r3)
-	bl	FUNC(kvmppc_copy_to_svcpu)
+	bl	kvmppc_copy_to_svcpu
 	nop
 	REST_GPR(4, r1)
 
@@ -131,7 +129,7 @@ after_sprg3_load:
 	PPC_LL	r4, VCPU_SHADOW_MSR(r4)	/* get shadow_msr */
 
 	/* Jump to segment patching handler and into our guest */
-	bl	FUNC(kvmppc_entry_trampoline)
+	bl	kvmppc_entry_trampoline
 	nop
 
 /*
@@ -164,7 +162,7 @@ after_sprg3_load:
 	/* On 64-bit, interrupts are still off at this point */
 
 	GET_SHADOW_VCPU(r4)
-	bl	FUNC(kvmppc_copy_from_svcpu)
+	bl	kvmppc_copy_from_svcpu
 	nop
 
 #ifdef CONFIG_PPC_BOOK3S_64
@@ -203,7 +201,7 @@ after_sprg3_load:
 
 	/* Restore r3 (kvm_run) and r4 (vcpu) */
 	REST_2GPRS(3, r1)
-	bl	FUNC(kvmppc_handle_exit_pr)
+	bl	kvmppc_handle_exit_pr
 
 	/* If RESUME_GUEST, get back in the loop */
 	cmpwi	r3, RESUME_GUEST
Index: b/arch/powerpc/kvm/book3s_rmhandlers.S
===================================================================
--- a/arch/powerpc/kvm/book3s_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_rmhandlers.S
@@ -34,13 +34,7 @@
  *                                                                           *
  ****************************************************************************/
 
-#if defined(CONFIG_PPC_BOOK3S_64)
-
-#define FUNC(name) 		GLUE(.,name)
-
-#elif defined(CONFIG_PPC_BOOK3S_32)
-
-#define FUNC(name)		name
+#ifdef CONFIG_PPC_BOOK3S_32
 
 .macro INTERRUPT_TRAMPOLINE intno
 

^ permalink raw reply

* Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
From: Zhang Yanfei @ 2014-06-12  8:15 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <1402543307-29800-2-git-send-email-iamjoonsoo.kim@lge.com>

On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
> We don't need explicit 'CMA:' prefix, since we already define prefix
> 'cma:' in pr_fmt. So remove it.
> 
> And, some logs print function name and others doesn't. This looks
> bad to me, so I unify log format to print function name consistently.
> 
> Lastly, I add one more debug log on cma_activate_area().
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Reviewed-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

> 
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 83969f8..bd0bb81 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>  	}
>  
>  	if (selected_size && !dma_contiguous_default_area) {
> -		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> +		pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>  			 (unsigned long)selected_size / SZ_1M);
>  
>  		dma_contiguous_reserve_area(selected_size, selected_base,
> @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
>  	unsigned i = cma->count >> pageblock_order;
>  	struct zone *zone;
>  
> -	cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> +	pr_debug("%s()\n", __func__);
>  
> +	cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
>  	if (!cma->bitmap)
>  		return -ENOMEM;
>  
> @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
>  
>  	/* Sanity checks */
>  	if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> -		pr_err("Not enough slots for CMA reserved regions!\n");
> +		pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> +			__func__);
>  		return -ENOSPC;
>  	}
>  
> @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
>  	*res_cma = cma;
>  	cma_area_count++;
>  
> -	pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
> -		(unsigned long)base);
> +	pr_info("%s(): reserved %ld MiB at %08lx\n",
> +		__func__, (unsigned long)size / SZ_1M, (unsigned long)base);
>  
>  	/* Architecture specific contiguous memory fixup. */
>  	dma_contiguous_early_fixup(base, size);
>  	return 0;
>  err:
> -	pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
> +	pr_err("%s(): failed to reserve %ld MiB\n",
> +		__func__, (unsigned long)size / SZ_1M);
>  	return ret;
>  }
>  
> 


-- 
Thanks.
Zhang Yanfei

^ permalink raw reply

* [PATCH 2/4] KVM: PPC: Book3S HV: Fix ABIv2 issues
From: Anton Blanchard @ 2014-06-12  8:15 UTC (permalink / raw)
  To: benh, paulus, agraf, clg; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20140612181420.65b05675@kryten>

binutils knows that a branch to a function descriptor is actually
a branch to the function text. By removing the explicit branch to
dot symbols, we maintain both ABIv1 and ABIv2 compatibility.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
===================================================================
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -675,9 +675,9 @@ END_FTR_SECTION_IFCLR(CPU_FTR_TM)
 
 	mr	r31, r4
 	addi	r3, r31, VCPU_FPRS_TM
-	bl	.load_fp_state
+	bl	load_fp_state
 	addi	r3, r31, VCPU_VRS_TM
-	bl	.load_vr_state
+	bl	load_vr_state
 	mr	r4, r31
 	lwz	r7, VCPU_VRSAVE_TM(r4)
 	mtspr	SPRN_VRSAVE, r7
@@ -1421,9 +1421,9 @@ END_FTR_SECTION_IFCLR(CPU_FTR_TM)
 
 	/* Save FP/VSX. */
 	addi	r3, r9, VCPU_FPRS_TM
-	bl	.store_fp_state
+	bl	store_fp_state
 	addi	r3, r9, VCPU_VRS_TM
-	bl	.store_vr_state
+	bl	store_vr_state
 	mfspr	r6, SPRN_VRSAVE
 	stw	r6, VCPU_VRSAVE_TM(r9)
 1:
@@ -2421,11 +2421,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
 	mtmsrd	r8
 	isync
 	addi	r3,r3,VCPU_FPRS
-	bl	.store_fp_state
+	bl	store_fp_state
 #ifdef CONFIG_ALTIVEC
 BEGIN_FTR_SECTION
 	addi	r3,r31,VCPU_VRS
-	bl	.store_vr_state
+	bl	store_vr_state
 END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 #endif
 	mfspr	r6,SPRN_VRSAVE
@@ -2457,11 +2457,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
 	mtmsrd	r8
 	isync
 	addi	r3,r4,VCPU_FPRS
-	bl	.load_fp_state
+	bl	load_fp_state
 #ifdef CONFIG_ALTIVEC
 BEGIN_FTR_SECTION
 	addi	r3,r31,VCPU_VRS
-	bl	.load_vr_state
+	bl	load_vr_state
 END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 #endif
 	lwz	r7,VCPU_VRSAVE(r31)

^ permalink raw reply

* [PATCH 3/4] KVM: PPC: Book3S HV: Fix ABIv2 indirect branch issue
From: Anton Blanchard @ 2014-06-12  8:16 UTC (permalink / raw)
  To: benh, paulus, agraf, clg; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20140612181420.65b05675@kryten>

To establish addressability quickly, ABIv2 requires the target
address of the function being called to be in r12.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
===================================================================
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1920,8 +1920,8 @@ hcall_try_real_mode:
 	lwax	r3,r3,r4
 	cmpwi	r3,0
 	beq	guest_exit_cont
-	add	r3,r3,r4
-	mtctr	r3
+	add	r12,r3,r4
+	mtctr	r12
 	mr	r3,r9		/* get vcpu pointer */
 	ld	r4,VCPU_GPR(R4)(r9)
 	bctrl

^ permalink raw reply

* [PATCH 4/4] KVM: PPC: Assembly functions exported to modules need _GLOBAL_TOC()
From: Anton Blanchard @ 2014-06-12  8:16 UTC (permalink / raw)
  To: benh, paulus, agraf, clg; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20140612181420.65b05675@kryten>

Both kvmppc_hv_entry_trampoline and kvmppc_entry_trampoline are
assembly functions that are exported to modules and also require
a valid r2.

As such we need to use _GLOBAL_TOC so we provide a global entry
point that establishes the TOC (r2).

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
===================================================================
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -51,7 +51,7 @@
  *
  * LR = return address to continue at after eventually re-enabling MMU
  */
-_GLOBAL(kvmppc_hv_entry_trampoline)
+_GLOBAL_TOC(kvmppc_hv_entry_trampoline)
 	mflr	r0
 	std	r0, PPC_LR_STKOFF(r1)
 	stdu	r1, -112(r1)
Index: b/arch/powerpc/kvm/book3s_rmhandlers.S
===================================================================
--- a/arch/powerpc/kvm/book3s_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_rmhandlers.S
@@ -140,7 +140,7 @@ kvmppc_handler_skip_ins:
  * On entry, r4 contains the guest shadow MSR
  * MSR.EE has to be 0 when calling this function
  */
-_GLOBAL(kvmppc_entry_trampoline)
+_GLOBAL_TOC(kvmppc_entry_trampoline)
 	mfmsr	r5
 	LOAD_REG_ADDR(r7, kvmppc_handler_trampoline_enter)
 	toreal(r7)

^ permalink raw reply

* Re: [PATCH] powerpc, kexec: Fix "Processor X is stuck" issue during kexec from ST mode
From: Srivatsa S. Bhat @ 2014-06-12  8:17 UTC (permalink / raw)
  To: Joel Stanley
  Cc: ego, matt, Mahesh Salgaonkar, kexec, linux-kernel, suzuki,
	ebiederm, Paul Mackerras, linuxppc-dev, Vivek Goyal
In-Reply-To: <CACPK8Xea+7rPHSCp51fVkX=GTmEwbV0T=JHDZ-XYt6RU96uyyg@mail.gmail.com>

Hi Joel,

On 06/12/2014 12:09 PM, Joel Stanley wrote:
> Hi Srivatsa,
> 
> On Sat, Jun 7, 2014 at 7:16 AM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> And with the following hunk added (which I had forgotten earlier), it worked just
>> fine on powernv :-)
> 
> How are the patches coming along?
> 

I'm still waiting to test this patch series on a PowerVM box, and unfortunately
there are some machine issues to debug first :-( So that's why this is taking
time... :-(

> I just hung a machine here while attempting to kexec. It appears to
> have onlined all of the secondary threads, and then hung here:
> 
> kexec: Waking offline cpu 1.
> kvm: enabling virtualization on CPU1
> kexec: Waking offline cpu 2.
> kvm: enabling virtualization on CPU2
> kexec: Waking offline cpu 3.
> kvm: enabling virtualization on CPU3
> kexec: Waking offline cpu 5.
> kvm: enabling virtualization on CPU5
> [...]
> kvm: enabling virtualization on CPU63
> kexec: waiting for cpu 1 (physical 1) to enter OPAL
> kexec: waiting for cpu 2 (physical 2) to enter OPAL
> kexec: waiting for cpu 3 (physical 3) to enter OPAL
> 
> I'm running benh's next branch as of thismorning, and SMT was off.
> 

Oh! This looks like a different hang than the one I tried to fix. My patch
("powerpc, kexec: Fix "Processor X is stuck" issue during kexec from ST mode")
which is already in benh's next branch was aimed at fixing the "CPU is stuck"
issue which was observed during the second kernel boot. If the first kernel
itself is hanging in the down-path, then it looks like a different problem
altogether.

> Could you please post your latest patches a series? I will test them here.
> 

The 4 patches that I proposed in this thread are aimed at making the above
solution more elegant, by not having to actually online the secondary threads
while doing kexec. I don't think it will solve the hang that you are seeing.
In any case, I'll provide the consolidated patch below if you want to give it
a try.

By the way, I have a few questions regarding the hang you observed: is it
always reproducible with SMT=off? And if SMT was 8 (i.e, all CPUs in the system
were online) and then you did a kexec, do you still see the hang?

Regards,
Srivatsa S. Bhat

---------------------------------------------------------------------------

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 16d7e33..2a31b52 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -68,6 +68,7 @@ static inline void crash_setup_regs(struct pt_regs *newregs,
 		ppc_save_regs(newregs);
 }
 
+extern bool kexec_cpu_wake(void);
 extern void kexec_smp_wait(void);	/* get and clear naca physid, wait for
 					  master to copy new code to 0 */
 extern int crashing_cpu;
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index f92b0b5..39f721d 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -255,6 +255,16 @@ struct machdep_calls {
 	void (*machine_shutdown)(void);
 
 #ifdef CONFIG_KEXEC
+#if (defined CONFIG_PPC64) && (defined CONFIG_PPC_BOOK3S)
+
+	/*
+	 * The pseries and powernv book3s platforms have a special requirement
+	 * that soft-offline CPUs have to be woken up before kexec, to avoid
+	 * CPUs getting stuck. This callback prepares the system for the
+	 * impending wakeup of the offline CPUs.
+	 */
+	void (*kexec_wake_prepare)(void);
+#endif
 	void (*kexec_cpu_down)(int crash_shutdown, int secondary);
 
 	/* Called to do what every setup is needed on image and the
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 879b3aa..84e91293 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -182,6 +182,14 @@ static void kexec_smp_down(void *arg)
 	/* NOTREACHED */
 }
 
+bool kexec_cpu_wake(void)
+{
+	kexec_smp_down(NULL);
+
+	/* NOTREACHED */
+	return true;
+}
+
 static void kexec_prepare_cpus_wait(int wait_state)
 {
 	int my_cpu, i, notified=-1;
@@ -202,7 +210,7 @@ static void kexec_prepare_cpus_wait(int wait_state)
 	 * these possible-but-not-online-but-should-be CPUs and chaperone them
 	 * into kexec_smp_wait().
 	 */
-	for_each_online_cpu(i) {
+	for_each_present_cpu(i) {
 		if (i == my_cpu)
 			continue;
 
@@ -228,16 +236,22 @@ static void kexec_prepare_cpus_wait(int wait_state)
  * threads as offline -- and again, these CPUs will be stuck.
  *
  * So, we online all CPUs that should be running, including secondary threads.
+ *
+ * TODO: Update this comment
  */
 static void wake_offline_cpus(void)
 {
 	int cpu = 0;
 
+	if (ppc_md.kexec_wake_prepare)
+		ppc_md.kexec_wake_prepare();
+
 	for_each_present_cpu(cpu) {
 		if (!cpu_online(cpu)) {
 			printk(KERN_INFO "kexec: Waking offline cpu %d.\n",
 			       cpu);
-			WARN_ON(cpu_up(cpu));
+			/* This should work even though the cpu is offline */
+			smp_send_reschedule(cpu);
 		}
 	}
 }
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h
index 75501bf..910081c 100644
--- a/arch/powerpc/platforms/powernv/powernv.h
+++ b/arch/powerpc/platforms/powernv/powernv.h
@@ -27,4 +27,8 @@ extern void pnv_lpc_init(void);
 
 bool cpu_core_split_required(void);
 
+#ifdef CONFIG_KEXEC
+extern void pnv_kexec_wake_prepare(void);
+#endif
+
 #endif /* _POWERNV_H */
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 8c16a5f..8dbccb7 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -331,6 +331,7 @@ define_machine(powernv) {
 	.calibrate_decr		= generic_calibrate_decr,
 	.dma_set_mask		= pnv_dma_set_mask,
 #ifdef CONFIG_KEXEC
+	.kexec_wake_prepare	= pnv_kexec_wake_prepare,
 	.kexec_cpu_down		= pnv_kexec_cpu_down,
 #endif
 #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index 0062a43..0b017b0 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -32,6 +32,7 @@
 #include <asm/opal.h>
 #include <asm/runlatch.h>
 #include <asm/code-patching.h>
+#include <asm/kexec.h>
 
 #include "powernv.h"
 
@@ -140,6 +141,15 @@ static int pnv_smp_cpu_disable(void)
 	return 0;
 }
 
+#ifdef CONFIG_KEXEC
+static bool kexec_wake_offline_cpus;
+
+void pnv_kexec_wake_prepare(void)
+{
+	kexec_wake_offline_cpus = true;
+}
+#endif
+
 static void pnv_smp_cpu_kill_self(void)
 {
 	unsigned int cpu;
@@ -170,6 +180,11 @@ static void pnv_smp_cpu_kill_self(void)
 		if (cpu_core_split_required())
 			continue;
 
+#ifdef CONFIG_KEXEC
+		if (kexec_wake_offline_cpus)
+			kexec_cpu_wake(); /* This function won't return! */
+#endif
+
 		if (!generic_check_cpu_restart(cpu))
 			DBG("CPU%d Unexpected exit while offline !\n", cpu);
 	}
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 20d6297..d026028 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -31,6 +31,7 @@
 #include <asm/vdso_datapage.h>
 #include <asm/xics.h>
 #include <asm/plpar_wrappers.h>
+#include <asm/kexec.h>
 
 #include "offline_states.h"
 
@@ -143,6 +144,13 @@ static void pseries_mach_cpu_die(void)
 			get_lppaca()->donate_dedicated_cpu = 0;
 		get_lppaca()->idle = 0;
 
+#if CONFIG_KEXEC
+		if (get_preferred_offline_state(cpu) == CPU_STATE_KEXEC_WAKE) {
+			/* This function won't return! */
+			kexec_cpu_wake();
+		}
+#endif
+
 		if (get_preferred_offline_state(cpu) == CPU_STATE_ONLINE) {
 			unregister_slb_shadow(hwcpu);
 
diff --git a/arch/powerpc/platforms/pseries/kexec.c b/arch/powerpc/platforms/pseries/kexec.c
index 13fa95b3..fc135e6 100644
--- a/arch/powerpc/platforms/pseries/kexec.c
+++ b/arch/powerpc/platforms/pseries/kexec.c
@@ -20,6 +20,17 @@
 #include <asm/plpar_wrappers.h>
 
 #include "pseries.h"
+#include "offline_states.h"
+
+void pseries_kexec_wake_prepare(void)
+{
+	unsigned int cpu;
+
+	for_each_present_cpu(cpu) {
+		if (!cpu_online(cpu))
+			set_preferred_offline_state(cpu, CPU_STATE_KEXEC_WAKE);
+	}
+}
 
 static void pseries_kexec_cpu_down(int crash_shutdown, int secondary)
 {
diff --git a/arch/powerpc/platforms/pseries/offline_states.h b/arch/powerpc/platforms/pseries/offline_states.h
index 08672d9..32fe5e8 100644
--- a/arch/powerpc/platforms/pseries/offline_states.h
+++ b/arch/powerpc/platforms/pseries/offline_states.h
@@ -5,6 +5,9 @@
 enum cpu_state_vals {
 	CPU_STATE_OFFLINE,
 	CPU_STATE_INACTIVE,
+#ifdef CONFIG_KEXEC
+	CPU_STATE_KEXEC_WAKE,
+#endif
 	CPU_STATE_ONLINE,
 	CPU_MAX_OFFLINE_STATES
 };
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 361add6..35ecb99 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -38,6 +38,8 @@ static inline void smp_init_pseries_xics(void) { };
 #endif
 
 #ifdef CONFIG_KEXEC
+extern void pseries_kexec_wake_prepare(void);
+
 extern void setup_kexec_cpu_down_xics(void);
 extern void setup_kexec_cpu_down_mpic(void);
 #else
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index adc21a0..c1a0722 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -808,6 +808,7 @@ define_machine(pseries) {
 	.system_reset_exception = pSeries_system_reset_exception,
 	.machine_check_exception = pSeries_machine_check_exception,
 #ifdef CONFIG_KEXEC
+	.kexec_wake_prepare	= pseries_kexec_wake_prepare,
 	.machine_kexec          = pSeries_machine_kexec,
 #endif
 #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 28c5706..55a6350 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1684,13 +1684,6 @@ int kernel_kexec(void)
 		kernel_restart_prepare(NULL);
 		migrate_to_reboot_cpu();
 
-		/*
-		 * migrate_to_reboot_cpu() disables CPU hotplug assuming that
-		 * no further code needs to use CPU hotplug (which is true in
-		 * the reboot case). However, the kexec path depends on using
-		 * CPU hotplug again; so re-enable it here.
-		 */
-		cpu_hotplug_enable();
 		printk(KERN_EMERG "Starting new kernel\n");
 		machine_shutdown();
 	}

^ permalink raw reply related

* Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
From: Zhang Yanfei @ 2014-06-12  8:19 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <20140612060211.GC30128@js1304-P5Q-DELUXE>

On 06/12/2014 02:02 PM, Joonsoo Kim wrote:
> On Thu, Jun 12, 2014 at 02:25:43PM +0900, Minchan Kim wrote:
>> On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote:
>>> We should free memory for bitmap when we find zone mis-match,
>>> otherwise this memory will leak.
>>
>> Then, -stable stuff?
> 
> I don't think so. This is just possible leak candidate, so we don't
> need to push this to stable tree.
> 
>>
>>>
>>> Additionally, I copy code comment from ppc kvm's cma code to notify
>>> why we need to check zone mis-match.
>>>
>>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>>>
>>> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
>>> index bd0bb81..fb0cdce 100644
>>> --- a/drivers/base/dma-contiguous.c
>>> +++ b/drivers/base/dma-contiguous.c
>>> @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma)
>>>  		base_pfn = pfn;
>>>  		for (j = pageblock_nr_pages; j; --j, pfn++) {
>>>  			WARN_ON_ONCE(!pfn_valid(pfn));
>>> +			/*
>>> +			 * alloc_contig_range requires the pfn range
>>> +			 * specified to be in the same zone. Make this
>>> +			 * simple by forcing the entire CMA resv range
>>> +			 * to be in the same zone.
>>> +			 */
>>>  			if (page_zone(pfn_to_page(pfn)) != zone)
>>> -				return -EINVAL;
>>> +				goto err;
>>
>> At a first glance, I thought it would be better to handle such error
>> before activating.
>> So when I see the registration code(ie, dma_contiguous_revere_area),
>> I realized it is impossible because we didn't set up zone yet. :(
>>
>> If so, when we detect to fail here, it would be better to report more
>> meaningful error message like what was successful zone and what is
>> new zone and failed pfn number?
> 
> What I want to do in early phase of this patchset is to make cma code
> on DMA APIs similar to ppc kvm's cma code. ppc kvm's cma code already
> has this error handling logic, so I make this patch.
> 
> If we think that we need more things, we can do that on general cma code
> after merging this patchset.
> 

Yeah, I also like the idea. After all, this patchset aims to a general CMA
management, we could improve more after this patchset. So

Acked-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

-- 
Thanks.
Zhang Yanfei

^ permalink raw reply

* Re: [PATCH] powerpc, xmon: Enable hardware instruction breakpoint support on POWER8
From: Anshuman Khandual @ 2014-06-12  8:19 UTC (permalink / raw)
  To: Anshuman Khandual; +Cc: Michael Ellerman, linuxppc-dev, mikey, Aneesh Kumar K.V
In-Reply-To: <538EDD61.60801@linux.vnet.ibm.com>

On 06/04/2014 02:18 PM, Anshuman Khandual wrote:
> On 06/03/2014 11:33 AM, Anshuman Khandual wrote:
>> On 05/30/2014 07:12 PM, Aneesh Kumar K.V wrote:
>>>> Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
>>>>
>>>>>> This patch enables support for hardware instruction breakpoints on POWER8 with
>>>>>> the help of a new register called CIABR (Completed Instruction Address Breakpoint
>>>>>> Register). With this patch, single hardware instruction breakpoint can be added
>>>>>> and cleared during any active xmon debug session. This hardware based instruction
>>>>>> breakpoint mechanism works correctly along with the existing TRAP based instruction
>>>>>> breakpoints available on xmon. Example usage as follows.
>>>>>>
>>>>>> (A) Start xmon:
>>>>>> $echo x > /proc/sysrq-trigger
>>>>>> SysRq : Entering xmon
>>>>>> cpu 0x0: Vector: 0  at [c000001f6c67f960]
>>>>>>     pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>>>>>     lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>>>>>     sp: c000001f6c67fac0
>>>>>>    msr: 9000000000009032
>>>>>>   current = 0xc000001f6e709ac0
>>>>>>   paca    = 0xc00000000fffa000   softe: 0        irq_happened: 0x00
>>>>>>     pid   = 3250, comm = bash
>>>>>> enter ? for help
>>>>>> 0:mon> b
>>>>>>    type            address
>>>>>>
>>>>>> (B) Set the breakpoint:
>>>>>> 0:mon> ls .power_pmu_add
>>>>>> .power_pmu_add: c000000000078f50
>>>>>> 0:mon> bi c000000000078f50
>>>>>> 0:mon> b
>>>>>>    type            address
>>>>>>  1 inst   c000000000078f50  .power_pmu_add+0x0/0x2e0
>>>>>> 0:mon> ls .perf_event_interrupt
>>>>>> .perf_event_interrupt: c00000000007aee0
>>>>>> 0:mon> bi c00000000007aee0
>>>>>> One instruction breakpoint possible with CIABR
>>>>>> 0:mon> x
>>>>>>
>>>>>> (C) Run the workload (with the breakpoint):
>>>>>> $./perf record ls
>>>>>> cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
>>>>>>     pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
>>>>>>     lr: c000000000155be0: .event_sched_in+0x90/0x1d0
>>>>>>     sp: c000001f71813620
>>>>>>    msr: 9000000040109032
>>>>>>   current = 0xc000001f6ce30000
>>>>>>   paca    = 0xc00000000fffa600   softe: 0        irq_happened: 0x01
>>>>>>     pid   = 3270, comm = ls
>>>>>>         std     r22,-80(r1)
>>>>>> enter ? for help
>>>>>>
>>>>>> (D) Clear the breakpoint:
>>>>>> 2:mon> bc
>>>>>> All breakpoints cleared
>>>>>> 2:mon> x
>>>>>> [ perf record: Woken up 1 times to write data ]
>>>>>> [ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]
>>>>>>
>>>>>> (E) Run the workload again (without any breakpoints):
>>>>>> $./perf record ls
>>>>>> [ perf record: Woken up 1 times to write data ]
>>>>>> [ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]
>>>>>>
>>>>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>>>>> ---
>>>>>>  arch/powerpc/xmon/xmon.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
>>>>>>  1 file changed, 58 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>>>>> index 3fd1d9a..f74ec83 100644
>>>>>> --- a/arch/powerpc/xmon/xmon.c
>>>>>> +++ b/arch/powerpc/xmon/xmon.c
>>>>>> @@ -48,6 +48,7 @@
>>>>>>  #ifdef CONFIG_PPC64
>>>>>>  #include <asm/hvcall.h>
>>>>>>  #include <asm/paca.h>
>>>>>> +#include <asm/plpar_wrappers.h>
>>>>>>  #endif
>>>>>>  
>>>>>>  #include "nonstdio.h"
>>>>>> @@ -89,6 +90,7 @@ struct bpt {
>>>>>>  /* Bits in bpt.enabled */
>>>>>>  #define BP_IABR_TE	1		/* IABR translation enabled */
>>>>>>  #define BP_IABR		2
>>>>>> +#define BP_CIABR	4
>>>>>>  #define BP_TRAP		8
>>>>>>  #define BP_DABR		0x10
>>>>>>  
>>>>>> @@ -97,6 +99,7 @@ static struct bpt bpts[NBPTS];
>>>>>>  static struct bpt dabr;
>>>>>>  static struct bpt *iabr;
>>>>>>  static unsigned bpinstr = 0x7fe00008;	/* trap */
>>>>>> +static bool ciabr_used = false;		/* CIABR instruction breakpoint */
>>>>>>  
>>>>>>  #define BP_NUM(bp)	((bp) - bpts + 1)
>>>>>>  
>>>>>> @@ -269,6 +272,34 @@ static inline void cinval(void *p)
>>>>>>  	asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
>>>>>>  }
>>>>>>  
>>>>>> +static void write_ciabr(unsigned long ciabr)
>>>>>> +{
>>>>>> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>>>>>> +		mtspr(SPRN_CIABR, ciabr);
>>>>>> +		return;
>>>>>> +	}
>>>>>> +
>>>>>> +#ifdef CONFIG_PPC64
>>>>>> +	plapr_set_ciabr(ciabr);
>>>>>> +#endif
>>>>>> +}
>>>>>> +
>>>>>> +static void set_ciabr(unsigned long addr)
>>>>>> +{
>>>>>> +	addr &= ~CIABR_PRIV;
>>>>>> +	if (cpu_has_feature(CPU_FTR_HVMODE))
>>>>>> +		addr |= CIABR_PRIV_HYPER;
>>>>>> +	else
>>>>>> +		addr |= CIABR_PRIV_SUPER;
>>>>>> +	write_ciabr(addr);
>>>>>> +}
>>>>>> +
>>>>>> +static void clear_ciabr(void)
>>>>>> +{
>>>>>> +	if (cpu_has_feature(CPU_FTR_ARCH_207S))
>>>>>> +		write_ciabr(0);
>>>>>> +}
>>>>>> +
>>>>>>  /*
>>>>>>   * Disable surveillance (the service processor watchdog function)
>>>>>>   * while we are in xmon.
>>>>>> @@ -764,6 +795,9 @@ static void insert_cpu_bpts(void)
>>>>>>  	if (iabr && cpu_has_feature(CPU_FTR_IABR))
>>>>>>  		mtspr(SPRN_IABR, iabr->address
>>>>>>  			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
>>>>>> +
>>>>>> +	if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
>>>>>> +		set_ciabr(iabr->address);
>>>>>>  }
>>>>>>  
>>>>>>  static void remove_bpts(void)
>>>>>> @@ -791,6 +825,7 @@ static void remove_cpu_bpts(void)
>>>>>>  	hw_breakpoint_disable();
>>>>>>  	if (cpu_has_feature(CPU_FTR_IABR))
>>>>>>  		mtspr(SPRN_IABR, 0);
>>>>>> +	clear_ciabr();
>>>>>>  }
>>>>>>  
>>>>>>  /* Command interpreting routine */
>>>>>> @@ -1124,7 +1159,7 @@ static char *breakpoint_help_string =
>>>>>>      "b <addr> [cnt]   set breakpoint at given instr addr\n"
>>>>>>      "bc               clear all breakpoints\n"
>>>>>>      "bc <n/addr>      clear breakpoint number n or at addr\n"
>>>>>> -    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64 only)\n"
>>>>>> +    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
>>>>>>      "bd <addr> [cnt]  set hardware data breakpoint\n"
>>>>>>      "";
>>>>>>  
>>>>>> @@ -1163,11 +1198,20 @@ bpt_cmds(void)
>>>>>>  		break;
>>>>>>  
>>>>>>  	case 'i':	/* bi - hardware instr breakpoint */
>>>>>> -		if (!cpu_has_feature(CPU_FTR_IABR)) {
>>>>>> +		if (!cpu_has_feature(CPU_FTR_IABR) && !cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>>>>>  			printf("Hardware instruction breakpoint "
>>>>>>  			       "not supported on this cpu\n");
>>>>>>  			break;
>>>>>>  		}
>>>>>> +
>>>>>> +		if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>>>>> +			if (ciabr_used) {
>>>>>> +				printf("One instruction breakpoint "
>>>>>> +					"possible with CIABR\n");
>>>>>> +				break;
>>>>>> +			}
>>>>
>>>> We don't seem to do that with iabr ? Why keep ciabr different 
> 
>> Right now with the IABR implementation if we try to set hardware instruction
>> breakpoint while one is already there, it just get overridden with the new
>> address without complaining. I thought with this at least for CIABR cases,
>> it will complain about it and require the user to clear the breakpoint
>> explicitly before allowing a new breakpoint. Okay will remove this.
>>  
> 
> I tried removing the "ciabr_used" variable and all related checks/assignments on it.
> Then I was able to add all these three address as hardware instruction breakpoint
> and xmon never complained that it might override the existing actual checkpoint
> implemented with CIABR. It accepted all the three addresses which can later be
> listed as below.
> 
> 0:mon> b
>    type            address
>  1 inst   c0000000000830d0  .power_pmu_add+0x0/0x2e0                                                
>  2 inst   c000000000084690  .power_pmu_del+0x0/0x3a0                                   
>  3 inst   c000000000085060  .perf_event_interrupt+0x0/0x480
> 
> But in reality, only perf_event_interrupt function's address got written into CIABR
> register and got triggered with the workload. I dont have a system which has IABR
> support to test it's behaviour for this situation. But this does not sound okay,
> we should explicitly inform the user that the hardware instruction breakpoint has
> been overridden with the latest command or reject the attempt. Looking for some
> suggestions in this regard. Thank you.

Hey Ben/MPE,

Any suggestions on this ^^^^^^^^^^^^ ?

^ permalink raw reply

* Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity
From: Zhang Yanfei @ 2014-06-12  8:28 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <1402543307-29800-6-git-send-email-iamjoonsoo.kim@lge.com>

On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
> ppc kvm's cma region management requires arbitrary bitmap granularity,
> since they want to reserve very large memory and manage this region
> with bitmap that one bit for several pages to reduce management overheads.
> So support arbitrary bitmap granularity for following generalization.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

> 
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index bc4c171..9bc9340 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -38,6 +38,7 @@ struct cma {
>  	unsigned long	base_pfn;
>  	unsigned long	count;
>  	unsigned long	*bitmap;
> +	int order_per_bit; /* Order of pages represented by one bit */
>  	struct mutex	lock;
>  };
>  
> @@ -157,9 +158,38 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>  
>  static DEFINE_MUTEX(cma_mutex);
>  
> +static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int align_order)
> +{
> +	return (1 << (align_order >> cma->order_per_bit)) - 1;
> +}
> +
> +static unsigned long cma_bitmap_maxno(struct cma *cma)
> +{
> +	return cma->count >> cma->order_per_bit;
> +}
> +
> +static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
> +						unsigned long pages)
> +{
> +	return ALIGN(pages, 1 << cma->order_per_bit) >> cma->order_per_bit;
> +}
> +
> +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> +{
> +	unsigned long bitmapno, nr_bits;
> +
> +	bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
> +	nr_bits = cma_bitmap_pages_to_bits(cma, count);
> +
> +	mutex_lock(&cma->lock);
> +	bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> +	mutex_unlock(&cma->lock);
> +}
> +
>  static int __init cma_activate_area(struct cma *cma)
>  {
> -	int bitmap_size = BITS_TO_LONGS(cma->count) * sizeof(long);
> +	int bitmap_maxno = cma_bitmap_maxno(cma);
> +	int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
>  	unsigned long base_pfn = cma->base_pfn, pfn = base_pfn;
>  	unsigned i = cma->count >> pageblock_order;
>  	struct zone *zone;
> @@ -221,6 +251,7 @@ core_initcall(cma_init_reserved_areas);
>   * @base: Base address of the reserved area optional, use 0 for any
>   * @limit: End address of the reserved memory (optional, 0 for any).
>   * @alignment: Alignment for the contiguous memory area, should be power of 2
> + * @order_per_bit: Order of pages represented by one bit on bitmap.
>   * @res_cma: Pointer to store the created cma region.
>   * @fixed: hint about where to place the reserved area
>   *
> @@ -235,7 +266,7 @@ core_initcall(cma_init_reserved_areas);
>   */
>  static int __init __dma_contiguous_reserve_area(phys_addr_t size,
>  				phys_addr_t base, phys_addr_t limit,
> -				phys_addr_t alignment,
> +				phys_addr_t alignment, int order_per_bit,
>  				struct cma **res_cma, bool fixed)
>  {
>  	struct cma *cma = &cma_areas[cma_area_count];
> @@ -269,6 +300,8 @@ static int __init __dma_contiguous_reserve_area(phys_addr_t size,
>  	base = ALIGN(base, alignment);
>  	size = ALIGN(size, alignment);
>  	limit &= ~(alignment - 1);
> +	/* size should be aligned with order_per_bit */
> +	BUG_ON(!IS_ALIGNED(size >> PAGE_SHIFT, 1 << order_per_bit));
>  
>  	/* Reserve memory */
>  	if (base && fixed) {
> @@ -294,6 +327,7 @@ static int __init __dma_contiguous_reserve_area(phys_addr_t size,
>  	 */
>  	cma->base_pfn = PFN_DOWN(base);
>  	cma->count = size >> PAGE_SHIFT;
> +	cma->order_per_bit = order_per_bit;
>  	*res_cma = cma;
>  	cma_area_count++;
>  
> @@ -313,7 +347,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
>  {
>  	int ret;
>  
> -	ret = __dma_contiguous_reserve_area(size, base, limit, 0,
> +	ret = __dma_contiguous_reserve_area(size, base, limit, 0, 0,
>  						res_cma, fixed);
>  	if (ret)
>  		return ret;
> @@ -324,13 +358,6 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
>  	return 0;
>  }
>  
> -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> -{
> -	mutex_lock(&cma->lock);
> -	bitmap_clear(cma->bitmap, pfn - cma->base_pfn, count);
> -	mutex_unlock(&cma->lock);
> -}
> -
>  /**
>   * dma_alloc_from_contiguous() - allocate pages from contiguous area
>   * @dev:   Pointer to device for which the allocation is performed.
> @@ -345,7 +372,8 @@ static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
>  static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
>  				       unsigned int align)
>  {
> -	unsigned long mask, pfn, pageno, start = 0;
> +	unsigned long mask, pfn, start = 0;
> +	unsigned long bitmap_maxno, bitmapno, nr_bits;
>  	struct page *page = NULL;
>  	int ret;
>  
> @@ -358,18 +386,19 @@ static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
>  	if (!count)
>  		return NULL;
>  
> -	mask = (1 << align) - 1;
> -
> +	mask = cma_bitmap_aligned_mask(cma, align);
> +	bitmap_maxno = cma_bitmap_maxno(cma);
> +	nr_bits = cma_bitmap_pages_to_bits(cma, count);
>  
>  	for (;;) {
>  		mutex_lock(&cma->lock);
> -		pageno = bitmap_find_next_zero_area(cma->bitmap, cma->count,
> -						    start, count, mask);
> -		if (pageno >= cma->count) {
> +		bitmapno = bitmap_find_next_zero_area(cma->bitmap,
> +					bitmap_maxno, start, nr_bits, mask);
> +		if (bitmapno >= bitmap_maxno) {
>  			mutex_unlock(&cma->lock);
>  			break;
>  		}
> -		bitmap_set(cma->bitmap, pageno, count);
> +		bitmap_set(cma->bitmap, bitmapno, nr_bits);
>  		/*
>  		 * It's safe to drop the lock here. We've marked this region for
>  		 * our exclusive use. If the migration fails we will take the
> @@ -377,7 +406,7 @@ static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
>  		 */
>  		mutex_unlock(&cma->lock);
>  
> -		pfn = cma->base_pfn + pageno;
> +		pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
>  		mutex_lock(&cma_mutex);
>  		ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA);
>  		mutex_unlock(&cma_mutex);
> @@ -392,7 +421,7 @@ static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
>  		pr_debug("%s(): memory range at %p is busy, retrying\n",
>  			 __func__, pfn_to_page(pfn));
>  		/* try again with a bit different memory target */
> -		start = pageno + mask + 1;
> +		start = bitmapno + mask + 1;
>  	}
>  
>  	pr_debug("%s(): returned %p\n", __func__, page);
> 


-- 
Thanks.
Zhang Yanfei

^ permalink raw reply

* Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
From: Zhang Yanfei @ 2014-06-12  8:29 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <1402543307-29800-7-git-send-email-iamjoonsoo.kim@lge.com>

On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
> Currently, there are two users on CMA functionality, one is the DMA
> subsystem and the other is the kvm on powerpc. They have their own code
> to manage CMA reserved area even if they looks really similar.
>>From my guess, it is caused by some needs on bitmap management. Kvm side
> wants to maintain bitmap not for 1 page, but for more size. Eventually it
> use bitmap where one bit represents 64 pages.
> 
> When I implement CMA related patches, I should change those two places
> to apply my change and it seem to be painful to me. I want to change
> this situation and reduce future code management overhead through
> this patch.
> 
> This change could also help developer who want to use CMA in their
> new feature development, since they can use CMA easily without
> copying & pasting this reserved area management code.
> 
> In previous patches, we have prepared some features to generalize
> CMA reserved area management and now it's time to do it. This patch
> moves core functions to mm/cma.c and change DMA APIs to use
> these functions.
> 
> There is no functional change in DMA APIs.
> 
> v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
> 
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

> 
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 00e13ce..4eac559 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
>  
>  	  If unsure, leave the default value "8".
>  
> -config CMA_AREAS
> -	int "Maximum count of the CMA device-private areas"
> -	default 7
> -	help
> -	  CMA allows to create CMA areas for particular devices. This parameter
> -	  sets the maximum number of such device private CMA areas in the
> -	  system.
> -
> -	  If unsure, leave the default value "7".
> -
>  endif
>  
>  endmenu
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 9bc9340..f177f73 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -24,25 +24,10 @@
>  
>  #include <linux/memblock.h>
>  #include <linux/err.h>
> -#include <linux/mm.h>
> -#include <linux/mutex.h>
> -#include <linux/page-isolation.h>
>  #include <linux/sizes.h>
> -#include <linux/slab.h>
> -#include <linux/swap.h>
> -#include <linux/mm_types.h>
>  #include <linux/dma-contiguous.h>
>  #include <linux/log2.h>
> -
> -struct cma {
> -	unsigned long	base_pfn;
> -	unsigned long	count;
> -	unsigned long	*bitmap;
> -	int order_per_bit; /* Order of pages represented by one bit */
> -	struct mutex	lock;
> -};
> -
> -struct cma *dma_contiguous_default_area;
> +#include <linux/cma.h>
>  
>  #ifdef CONFIG_CMA_SIZE_MBYTES
>  #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
> @@ -50,6 +35,8 @@ struct cma *dma_contiguous_default_area;
>  #define CMA_SIZE_MBYTES 0
>  #endif
>  
> +struct cma *dma_contiguous_default_area;
> +
>  /*
>   * Default global CMA area size can be defined in kernel's .config.
>   * This is useful mainly for distro maintainers to create a kernel
> @@ -156,199 +143,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>  	}
>  }
>  
> -static DEFINE_MUTEX(cma_mutex);
> -
> -static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int align_order)
> -{
> -	return (1 << (align_order >> cma->order_per_bit)) - 1;
> -}
> -
> -static unsigned long cma_bitmap_maxno(struct cma *cma)
> -{
> -	return cma->count >> cma->order_per_bit;
> -}
> -
> -static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
> -						unsigned long pages)
> -{
> -	return ALIGN(pages, 1 << cma->order_per_bit) >> cma->order_per_bit;
> -}
> -
> -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> -{
> -	unsigned long bitmapno, nr_bits;
> -
> -	bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
> -	nr_bits = cma_bitmap_pages_to_bits(cma, count);
> -
> -	mutex_lock(&cma->lock);
> -	bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> -	mutex_unlock(&cma->lock);
> -}
> -
> -static int __init cma_activate_area(struct cma *cma)
> -{
> -	int bitmap_maxno = cma_bitmap_maxno(cma);
> -	int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
> -	unsigned long base_pfn = cma->base_pfn, pfn = base_pfn;
> -	unsigned i = cma->count >> pageblock_order;
> -	struct zone *zone;
> -
> -	pr_debug("%s()\n", __func__);
> -
> -	cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> -	if (!cma->bitmap)
> -		return -ENOMEM;
> -
> -	WARN_ON_ONCE(!pfn_valid(pfn));
> -	zone = page_zone(pfn_to_page(pfn));
> -
> -	do {
> -		unsigned j;
> -		base_pfn = pfn;
> -		for (j = pageblock_nr_pages; j; --j, pfn++) {
> -			WARN_ON_ONCE(!pfn_valid(pfn));
> -			/*
> -			 * alloc_contig_range requires the pfn range
> -			 * specified to be in the same zone. Make this
> -			 * simple by forcing the entire CMA resv range
> -			 * to be in the same zone.
> -			 */
> -			if (page_zone(pfn_to_page(pfn)) != zone)
> -				goto err;
> -		}
> -		init_cma_reserved_pageblock(pfn_to_page(base_pfn));
> -	} while (--i);
> -
> -	mutex_init(&cma->lock);
> -	return 0;
> -
> -err:
> -	kfree(cma->bitmap);
> -	return -EINVAL;
> -}
> -
> -static struct cma cma_areas[MAX_CMA_AREAS];
> -static unsigned cma_area_count;
> -
> -static int __init cma_init_reserved_areas(void)
> -{
> -	int i;
> -
> -	for (i = 0; i < cma_area_count; i++) {
> -		int ret = cma_activate_area(&cma_areas[i]);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	return 0;
> -}
> -core_initcall(cma_init_reserved_areas);
> -
> -/**
> - * dma_contiguous_reserve_area() - reserve custom contiguous area
> - * @size: Size of the reserved area (in bytes),
> - * @base: Base address of the reserved area optional, use 0 for any
> - * @limit: End address of the reserved memory (optional, 0 for any).
> - * @alignment: Alignment for the contiguous memory area, should be power of 2
> - * @order_per_bit: Order of pages represented by one bit on bitmap.
> - * @res_cma: Pointer to store the created cma region.
> - * @fixed: hint about where to place the reserved area
> - *
> - * This function reserves memory from early allocator. It should be
> - * called by arch specific code once the early allocator (memblock or bootmem)
> - * has been activated and all other subsystems have already allocated/reserved
> - * memory. This function allows to create custom reserved areas for specific
> - * devices.
> - *
> - * If @fixed is true, reserve contiguous area at exactly @base.  If false,
> - * reserve in range from @base to @limit.
> - */
> -static int __init __dma_contiguous_reserve_area(phys_addr_t size,
> -				phys_addr_t base, phys_addr_t limit,
> -				phys_addr_t alignment, int order_per_bit,
> -				struct cma **res_cma, bool fixed)
> -{
> -	struct cma *cma = &cma_areas[cma_area_count];
> -	int ret = 0;
> -
> -	pr_debug("%s(size %lx, base %08lx, limit %08lx align_order %08lx)\n",
> -		__func__, (unsigned long)size, (unsigned long)base,
> -		(unsigned long)limit, (unsigned long)alignment);
> -
> -	/* Sanity checks */
> -	if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> -		pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> -			__func__);
> -		return -ENOSPC;
> -	}
> -
> -	if (!size)
> -		return -EINVAL;
> -
> -	if (alignment && !is_power_of_2(alignment))
> -		return -EINVAL;
> -
> -	/*
> -	 * Sanitise input arguments.
> -	 * CMA area should be at least MAX_ORDER - 1 aligned. Otherwise,
> -	 * CMA area could be merged into other MIGRATE_TYPE by buddy mechanism
> -	 * and CMA property will be broken.
> -	 */
> -	alignment = max(alignment,
> -		(phys_addr_t)PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));
> -	base = ALIGN(base, alignment);
> -	size = ALIGN(size, alignment);
> -	limit &= ~(alignment - 1);
> -	/* size should be aligned with order_per_bit */
> -	BUG_ON(!IS_ALIGNED(size >> PAGE_SHIFT, 1 << order_per_bit));
> -
> -	/* Reserve memory */
> -	if (base && fixed) {
> -		if (memblock_is_region_reserved(base, size) ||
> -		    memblock_reserve(base, size) < 0) {
> -			ret = -EBUSY;
> -			goto err;
> -		}
> -	} else {
> -		phys_addr_t addr = memblock_alloc_range(size, alignment, base,
> -							limit);
> -		if (!addr) {
> -			ret = -ENOMEM;
> -			goto err;
> -		} else {
> -			base = addr;
> -		}
> -	}
> -
> -	/*
> -	 * Each reserved area must be initialised later, when more kernel
> -	 * subsystems (like slab allocator) are available.
> -	 */
> -	cma->base_pfn = PFN_DOWN(base);
> -	cma->count = size >> PAGE_SHIFT;
> -	cma->order_per_bit = order_per_bit;
> -	*res_cma = cma;
> -	cma_area_count++;
> -
> -	pr_info("%s(): reserved %ld MiB at %08lx\n",
> -		__func__, (unsigned long)size / SZ_1M, (unsigned long)base);
> -	return 0;
> -
> -err:
> -	pr_err("%s(): failed to reserve %ld MiB\n",
> -		__func__, (unsigned long)size / SZ_1M);
> -	return ret;
> -}
> -
>  int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
>  				       phys_addr_t limit, struct cma **res_cma,
>  				       bool fixed)
>  {
>  	int ret;
>  
> -	ret = __dma_contiguous_reserve_area(size, base, limit, 0, 0,
> -						res_cma, fixed);
> +	ret = cma_declare_contiguous(size, base, limit, 0, 0, res_cma, fixed);
>  	if (ret)
>  		return ret;
>  
> @@ -358,124 +159,17 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
>  	return 0;
>  }
>  
> -/**
> - * dma_alloc_from_contiguous() - allocate pages from contiguous area
> - * @dev:   Pointer to device for which the allocation is performed.
> - * @count: Requested number of pages.
> - * @align: Requested alignment of pages (in PAGE_SIZE order).
> - *
> - * This function allocates memory buffer for specified device. It uses
> - * device specific contiguous memory area if available or the default
> - * global one. Requires architecture specific dev_get_cma_area() helper
> - * function.
> - */
> -static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
> -				       unsigned int align)
> -{
> -	unsigned long mask, pfn, start = 0;
> -	unsigned long bitmap_maxno, bitmapno, nr_bits;
> -	struct page *page = NULL;
> -	int ret;
> -
> -	if (!cma || !cma->count)
> -		return NULL;
> -
> -	pr_debug("%s(cma %p, count %d, align %d)\n", __func__, (void *)cma,
> -		 count, align);
> -
> -	if (!count)
> -		return NULL;
> -
> -	mask = cma_bitmap_aligned_mask(cma, align);
> -	bitmap_maxno = cma_bitmap_maxno(cma);
> -	nr_bits = cma_bitmap_pages_to_bits(cma, count);
> -
> -	for (;;) {
> -		mutex_lock(&cma->lock);
> -		bitmapno = bitmap_find_next_zero_area(cma->bitmap,
> -					bitmap_maxno, start, nr_bits, mask);
> -		if (bitmapno >= bitmap_maxno) {
> -			mutex_unlock(&cma->lock);
> -			break;
> -		}
> -		bitmap_set(cma->bitmap, bitmapno, nr_bits);
> -		/*
> -		 * It's safe to drop the lock here. We've marked this region for
> -		 * our exclusive use. If the migration fails we will take the
> -		 * lock again and unmark it.
> -		 */
> -		mutex_unlock(&cma->lock);
> -
> -		pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
> -		mutex_lock(&cma_mutex);
> -		ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA);
> -		mutex_unlock(&cma_mutex);
> -		if (ret == 0) {
> -			page = pfn_to_page(pfn);
> -			break;
> -		} else if (ret != -EBUSY) {
> -			clear_cma_bitmap(cma, pfn, count);
> -			break;
> -		}
> -		clear_cma_bitmap(cma, pfn, count);
> -		pr_debug("%s(): memory range at %p is busy, retrying\n",
> -			 __func__, pfn_to_page(pfn));
> -		/* try again with a bit different memory target */
> -		start = bitmapno + mask + 1;
> -	}
> -
> -	pr_debug("%s(): returned %p\n", __func__, page);
> -	return page;
> -}
> -
>  struct page *dma_alloc_from_contiguous(struct device *dev, int count,
>  				       unsigned int align)
>  {
> -	struct cma *cma = dev_get_cma_area(dev);
> -
>  	if (align > CONFIG_CMA_ALIGNMENT)
>  		align = CONFIG_CMA_ALIGNMENT;
>  
> -	return __dma_alloc_from_contiguous(cma, count, align);
> -}
> -
> -/**
> - * dma_release_from_contiguous() - release allocated pages
> - * @dev:   Pointer to device for which the pages were allocated.
> - * @pages: Allocated pages.
> - * @count: Number of allocated pages.
> - *
> - * This function releases memory allocated by dma_alloc_from_contiguous().
> - * It returns false when provided pages do not belong to contiguous area and
> - * true otherwise.
> - */
> -static bool __dma_release_from_contiguous(struct cma *cma, struct page *pages,
> -				 int count)
> -{
> -	unsigned long pfn;
> -
> -	if (!cma || !pages)
> -		return false;
> -
> -	pr_debug("%s(page %p)\n", __func__, (void *)pages);
> -
> -	pfn = page_to_pfn(pages);
> -
> -	if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count)
> -		return false;
> -
> -	VM_BUG_ON(pfn + count > cma->base_pfn + cma->count);
> -
> -	free_contig_range(pfn, count);
> -	clear_cma_bitmap(cma, pfn, count);
> -
> -	return true;
> +	return cma_alloc(dev_get_cma_area(dev), count, align);
>  }
>  
>  bool dma_release_from_contiguous(struct device *dev, struct page *pages,
>  				 int count)
>  {
> -	struct cma *cma = dev_get_cma_area(dev);
> -
> -	return __dma_release_from_contiguous(cma, pages, count);
> +	return cma_release(dev_get_cma_area(dev), pages, count);
>  }
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> new file mode 100644
> index 0000000..e38efe9
> --- /dev/null
> +++ b/include/linux/cma.h
> @@ -0,0 +1,12 @@
> +#ifndef __CMA_H__
> +#define __CMA_H__
> +
> +struct cma;
> +
> +extern int __init cma_declare_contiguous(phys_addr_t size,
> +				phys_addr_t base, phys_addr_t limit,
> +				phys_addr_t alignment, int order_per_bit,
> +				struct cma **res_cma, bool fixed);
> +extern struct page *cma_alloc(struct cma *cma, int count, unsigned int align);
> +extern bool cma_release(struct cma *cma, struct page *pages, int count);
> +#endif
> diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index 772eab5..a40c1f3 100644
> --- a/include/linux/dma-contiguous.h
> +++ b/include/linux/dma-contiguous.h
> @@ -53,9 +53,10 @@
>  
>  #ifdef __KERNEL__
>  
> +#include <linux/device.h>
> +
>  struct cma;
>  struct page;
> -struct device;
>  
>  #ifdef CONFIG_DMA_CMA
>  
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 3e9977a..f4899ec 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -508,6 +508,17 @@ config CMA_DEBUG
>  	  processing calls such as dma_alloc_from_contiguous().
>  	  This option does not affect warning and error messages.
>  
> +config CMA_AREAS
> +	int "Maximum count of the CMA areas"
> +	depends on CMA
> +	default 7
> +	help
> +	  CMA allows to create CMA areas for particular purpose, mainly,
> +	  used as device private area. This parameter sets the maximum
> +	  number of CMA area in the system.
> +
> +	  If unsure, leave the default value "7".
> +
>  config ZBUD
>  	tristate
>  	default n
> diff --git a/mm/Makefile b/mm/Makefile
> index 1eaa70b..bc0422b 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -62,3 +62,4 @@ obj-$(CONFIG_MEMORY_ISOLATION) += page_isolation.o
>  obj-$(CONFIG_ZBUD)	+= zbud.o
>  obj-$(CONFIG_ZSMALLOC)	+= zsmalloc.o
>  obj-$(CONFIG_GENERIC_EARLY_IOREMAP) += early_ioremap.o
> +obj-$(CONFIG_CMA)	+= cma.o
> diff --git a/mm/cma.c b/mm/cma.c
> new file mode 100644
> index 0000000..1e1b017
> --- /dev/null
> +++ b/mm/cma.c
> @@ -0,0 +1,330 @@
> +/*
> + * Contiguous Memory Allocator
> + *
> + * Copyright (c) 2010-2011 by Samsung Electronics.
> + * Copyright IBM Corporation, 2013
> + * Copyright LG Electronics Inc., 2014
> + * Written by:
> + *	Marek Szyprowski <m.szyprowski@samsung.com>
> + *	Michal Nazarewicz <mina86@mina86.com>
> + *	Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> + *	Joonsoo Kim <iamjoonsoo.kim@lge.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License or (at your optional) any later version of the license.
> + */
> +
> +#define pr_fmt(fmt) "cma: " fmt
> +
> +#ifdef CONFIG_CMA_DEBUG
> +#ifndef DEBUG
> +#  define DEBUG
> +#endif
> +#endif
> +
> +#include <linux/memblock.h>
> +#include <linux/err.h>
> +#include <linux/mm.h>
> +#include <linux/mutex.h>
> +#include <linux/sizes.h>
> +#include <linux/slab.h>
> +
> +struct cma {
> +	unsigned long	base_pfn;
> +	unsigned long	count;
> +	unsigned long	*bitmap;
> +	int order_per_bit; /* Order of pages represented by one bit */
> +	struct mutex	lock;
> +};
> +
> +/*
> + * There is always at least global CMA area and a few optional
> + * areas configured in kernel .config.
> + */
> +#define MAX_CMA_AREAS	(1 + CONFIG_CMA_AREAS)
> +
> +static struct cma cma_areas[MAX_CMA_AREAS];
> +static unsigned cma_area_count;
> +static DEFINE_MUTEX(cma_mutex);
> +
> +static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int align_order)
> +{
> +	return (1 << (align_order >> cma->order_per_bit)) - 1;
> +}
> +
> +static unsigned long cma_bitmap_maxno(struct cma *cma)
> +{
> +	return cma->count >> cma->order_per_bit;
> +}
> +
> +static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
> +						unsigned long pages)
> +{
> +	return ALIGN(pages, 1 << cma->order_per_bit) >> cma->order_per_bit;
> +}
> +
> +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> +{
> +	unsigned long bitmapno, nr_bits;
> +
> +	bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
> +	nr_bits = cma_bitmap_pages_to_bits(cma, count);
> +
> +	mutex_lock(&cma->lock);
> +	bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> +	mutex_unlock(&cma->lock);
> +}
> +
> +static int __init cma_activate_area(struct cma *cma)
> +{
> +	int bitmap_maxno = cma_bitmap_maxno(cma);
> +	int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
> +	unsigned long base_pfn = cma->base_pfn, pfn = base_pfn;
> +	unsigned i = cma->count >> pageblock_order;
> +	struct zone *zone;
> +
> +	pr_debug("%s()\n", __func__);
> +
> +	cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> +	if (!cma->bitmap)
> +		return -ENOMEM;
> +
> +	WARN_ON_ONCE(!pfn_valid(pfn));
> +	zone = page_zone(pfn_to_page(pfn));
> +
> +	do {
> +		unsigned j;
> +
> +		base_pfn = pfn;
> +		for (j = pageblock_nr_pages; j; --j, pfn++) {
> +			WARN_ON_ONCE(!pfn_valid(pfn));
> +			/*
> +			 * alloc_contig_range requires the pfn range
> +			 * specified to be in the same zone. Make this
> +			 * simple by forcing the entire CMA resv range
> +			 * to be in the same zone.
> +			 */
> +			if (page_zone(pfn_to_page(pfn)) != zone)
> +				goto err;
> +		}
> +		init_cma_reserved_pageblock(pfn_to_page(base_pfn));
> +	} while (--i);
> +
> +	mutex_init(&cma->lock);
> +	return 0;
> +
> +err:
> +	kfree(cma->bitmap);
> +	return -EINVAL;
> +}
> +
> +static int __init cma_init_reserved_areas(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < cma_area_count; i++) {
> +		int ret = cma_activate_area(&cma_areas[i]);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +core_initcall(cma_init_reserved_areas);
> +
> +/**
> + * cma_declare_contiguous() - reserve custom contiguous area
> + * @size: Size of the reserved area (in bytes),
> + * @base: Base address of the reserved area optional, use 0 for any
> + * @limit: End address of the reserved memory (optional, 0 for any).
> + * @alignment: Alignment for the contiguous memory area, should be power of 2
> + * @order_per_bit: Order of pages represented by one bit on bitmap.
> + * @res_cma: Pointer to store the created cma region.
> + * @fixed: hint about where to place the reserved area
> + *
> + * This function reserves memory from early allocator. It should be
> + * called by arch specific code once the early allocator (memblock or bootmem)
> + * has been activated and all other subsystems have already allocated/reserved
> + * memory. This function allows to create custom reserved areas.
> + *
> + * If @fixed is true, reserve contiguous area at exactly @base.  If false,
> + * reserve in range from @base to @limit.
> + */
> +int __init cma_declare_contiguous(phys_addr_t size,
> +				phys_addr_t base, phys_addr_t limit,
> +				phys_addr_t alignment, int order_per_bit,
> +				struct cma **res_cma, bool fixed)
> +{
> +	struct cma *cma = &cma_areas[cma_area_count];
> +	int ret = 0;
> +
> +	pr_debug("%s(size %lx, base %08lx, limit %08lx alignment %08lx)\n",
> +		__func__, (unsigned long)size, (unsigned long)base,
> +		(unsigned long)limit, (unsigned long)alignment);
> +
> +	/* Sanity checks */
> +	if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> +		pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> +			__func__);
> +		return -ENOSPC;
> +	}
> +
> +	if (!size)
> +		return -EINVAL;
> +
> +	if (alignment && !is_power_of_2(alignment))
> +		return -EINVAL;
> +
> +	/*
> +	 * Sanitise input arguments.
> +	 * CMA area should be at least MAX_ORDER - 1 aligned. Otherwise,
> +	 * CMA area could be merged into other MIGRATE_TYPE by buddy mechanism
> +	 * and CMA property will be broken.
> +	 */
> +	alignment = max(alignment,
> +		(phys_addr_t)PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));
> +	base = ALIGN(base, alignment);
> +	size = ALIGN(size, alignment);
> +	limit &= ~(alignment - 1);
> +	/* size should be aligned with order_per_bit */
> +	BUG_ON(!IS_ALIGNED(size >> PAGE_SHIFT, 1 << order_per_bit));
> +
> +	/* Reserve memory */
> +	if (base && fixed) {
> +		if (memblock_is_region_reserved(base, size) ||
> +		    memblock_reserve(base, size) < 0) {
> +			ret = -EBUSY;
> +			goto err;
> +		}
> +	} else {
> +		phys_addr_t addr = memblock_alloc_range(size, alignment, base,
> +							limit);
> +		if (!addr) {
> +			ret = -ENOMEM;
> +			goto err;
> +		} else {
> +			base = addr;
> +		}
> +	}
> +
> +	/*
> +	 * Each reserved area must be initialised later, when more kernel
> +	 * subsystems (like slab allocator) are available.
> +	 */
> +	cma->base_pfn = PFN_DOWN(base);
> +	cma->count = size >> PAGE_SHIFT;
> +	cma->order_per_bit = order_per_bit;
> +	*res_cma = cma;
> +	cma_area_count++;
> +
> +	pr_info("%s(): reserved %ld MiB at %08lx\n",
> +		__func__, (unsigned long)size / SZ_1M, (unsigned long)base);
> +	return 0;
> +
> +err:
> +	pr_err("%s(): failed to reserve %ld MiB\n",
> +		__func__, (unsigned long)size / SZ_1M);
> +	return ret;
> +}
> +
> +/**
> + * cma_alloc() - allocate pages from contiguous area
> + * @cma:   Contiguous memory region for which the allocation is performed.
> + * @count: Requested number of pages.
> + * @align: Requested alignment of pages (in PAGE_SIZE order).
> + *
> + * This function allocates part of contiguous memory on specific
> + * contiguous memory area.
> + */
> +struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
> +{
> +	unsigned long mask, pfn, start = 0;
> +	unsigned long bitmap_maxno, bitmapno, nr_bits;
> +	struct page *page = NULL;
> +	int ret;
> +
> +	if (!cma || !cma->count)
> +		return NULL;
> +
> +	pr_debug("%s(cma %p, count %d, align %d)\n", __func__, (void *)cma,
> +		 count, align);
> +
> +	if (!count)
> +		return NULL;
> +
> +	mask = cma_bitmap_aligned_mask(cma, align);
> +	bitmap_maxno = cma_bitmap_maxno(cma);
> +	nr_bits = cma_bitmap_pages_to_bits(cma, count);
> +
> +	for (;;) {
> +		mutex_lock(&cma->lock);
> +		bitmapno = bitmap_find_next_zero_area(cma->bitmap,
> +					bitmap_maxno, start, nr_bits, mask);
> +		if (bitmapno >= bitmap_maxno) {
> +			mutex_unlock(&cma->lock);
> +			break;
> +		}
> +		bitmap_set(cma->bitmap, bitmapno, nr_bits);
> +		/*
> +		 * It's safe to drop the lock here. We've marked this region for
> +		 * our exclusive use. If the migration fails we will take the
> +		 * lock again and unmark it.
> +		 */
> +		mutex_unlock(&cma->lock);
> +
> +		pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
> +		mutex_lock(&cma_mutex);
> +		ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA);
> +		mutex_unlock(&cma_mutex);
> +		if (ret == 0) {
> +			page = pfn_to_page(pfn);
> +			break;
> +		} else if (ret != -EBUSY) {
> +			clear_cma_bitmap(cma, pfn, count);
> +			break;
> +		}
> +		clear_cma_bitmap(cma, pfn, count);
> +		pr_debug("%s(): memory range at %p is busy, retrying\n",
> +			 __func__, pfn_to_page(pfn));
> +		/* try again with a bit different memory target */
> +		start = bitmapno + mask + 1;
> +	}
> +
> +	pr_debug("%s(): returned %p\n", __func__, page);
> +	return page;
> +}
> +
> +/**
> + * cma_release() - release allocated pages
> + * @cma:   Contiguous memory region for which the allocation is performed.
> + * @pages: Allocated pages.
> + * @count: Number of allocated pages.
> + *
> + * This function releases memory allocated by alloc_cma().
> + * It returns false when provided pages do not belong to contiguous area and
> + * true otherwise.
> + */
> +bool cma_release(struct cma *cma, struct page *pages, int count)
> +{
> +	unsigned long pfn;
> +
> +	if (!cma || !pages)
> +		return false;
> +
> +	pr_debug("%s(page %p)\n", __func__, (void *)pages);
> +
> +	pfn = page_to_pfn(pages);
> +
> +	if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count)
> +		return false;
> +
> +	VM_BUG_ON(pfn + count > cma->base_pfn + cma->count);
> +
> +	free_contig_range(pfn, count);
> +	clear_cma_bitmap(cma, pfn, count);
> +
> +	return true;
> +}
> 


-- 
Thanks.
Zhang Yanfei

^ permalink raw reply

* Re: [PATCH v2 08/10] mm, cma: clean-up cma allocation error path
From: Zhang Yanfei @ 2014-06-12  8:31 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
	Marek Szyprowski
In-Reply-To: <1402543307-29800-9-git-send-email-iamjoonsoo.kim@lge.com>

On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
> We can remove one call sites for clear_cma_bitmap() if we first
> call it before checking error number.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Reviewed-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

> 
> diff --git a/mm/cma.c b/mm/cma.c
> index 1e1b017..01a0713 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -282,11 +282,12 @@ struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
>  		if (ret == 0) {
>  			page = pfn_to_page(pfn);
>  			break;
> -		} else if (ret != -EBUSY) {
> -			clear_cma_bitmap(cma, pfn, count);
> -			break;
>  		}
> +
>  		clear_cma_bitmap(cma, pfn, count);
> +		if (ret != -EBUSY)
> +			break;
> +
>  		pr_debug("%s(): memory range at %p is busy, retrying\n",
>  			 __func__, pfn_to_page(pfn));
>  		/* try again with a bit different memory target */
> 


-- 
Thanks.
Zhang Yanfei

^ permalink raw reply

* Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
From: Michal Nazarewicz @ 2014-06-12  8:55 UTC (permalink / raw)
  To: Joonsoo Kim, Aneesh Kumar K.V
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Andrew Morton,
	linuxppc-dev, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20140612055358.GA30128@js1304-P5Q-DELUXE>

>> Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:
>>=20
>> > We don't need explicit 'CMA:' prefix, since we already define prefix
>> > 'cma:' in pr_fmt. So remove it.
>> >
>> > And, some logs print function name and others doesn't. This looks
>> > bad to me, so I unify log format to print function name consistently.
>> >
>> > Lastly, I add one more debug log on cma_activate_area().
>> >
>> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>> >
>> > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguo=
us.c
>> > index 83969f8..bd0bb81 100644
>> > --- a/drivers/base/dma-contiguous.c
>> > +++ b/drivers/base/dma-contiguous.c
>> > @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t lim=
it)
>> >  	}
>> >
>> >  	if (selected_size && !dma_contiguous_default_area) {
>> > -		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
>> > +		pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>> >  			 (unsigned long)selected_size / SZ_1M);

> On Thu, Jun 12, 2014 at 10:11:19AM +0530, Aneesh Kumar K.V wrote:
>> Do we need to do function(), or just function:. I have seen the later
>> usage in other parts of the kernel.

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> I also haven't seen this format in other kernel code, but, in cma, they u=
se
> this format as following.
>
> function(arg1, arg2, ...): some message
>
> If we all dislike this format, we can change it after merging this
> patchset. Until then, it seems better to me to leave it as is.

I used =E2=80=9Cfunction(arg1, arg2, =E2=80=A6)=E2=80=9D at the *beginning*=
 of functions when
the arguments passed to the function were included in the message.  In
all other cases I left it at just =E2=80=9Cfunction:=E2=80=9D (or just no a=
dditional
prefix).  IMO that's a reasonable strategy.

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
From: Michal Nazarewicz @ 2014-06-12  8:56 UTC (permalink / raw)
  To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-2-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> We don't need explicit 'CMA:' prefix, since we already define prefix
> 'cma:' in pr_fmt. So remove it.
>
> And, some logs print function name and others doesn't. This looks
> bad to me, so I unify log format to print function name consistently.
>
> Lastly, I add one more debug log on cma_activate_area().

I don't particularly care what format of logs you choose, so:

Acked-by: Michal Nazarewicz <mina86@mina86.com>

even though I'd go without empty =E2=80=9C()=E2=80=9D.

> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 83969f8..bd0bb81 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>  	}
>=20=20
>  	if (selected_size && !dma_contiguous_default_area) {
> -		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> +		pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>  			 (unsigned long)selected_size / SZ_1M);
>=20=20
>  		dma_contiguous_reserve_area(selected_size, selected_base,
> @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
>  	unsigned i =3D cma->count >> pageblock_order;
>  	struct zone *zone;
>=20=20
> -	cma->bitmap =3D kzalloc(bitmap_size, GFP_KERNEL);
> +	pr_debug("%s()\n", __func__);
>=20=20
> +	cma->bitmap =3D kzalloc(bitmap_size, GFP_KERNEL);
>  	if (!cma->bitmap)
>  		return -ENOMEM;
>=20=20
> @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t si=
ze, phys_addr_t base,
>=20=20
>  	/* Sanity checks */
>  	if (cma_area_count =3D=3D ARRAY_SIZE(cma_areas)) {
> -		pr_err("Not enough slots for CMA reserved regions!\n");
> +		pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> +			__func__);
>  		return -ENOSPC;
>  	}
>=20=20
> @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t =
size, phys_addr_t base,
>  	*res_cma =3D cma;
>  	cma_area_count++;
>=20=20
> -	pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
> -		(unsigned long)base);
> +	pr_info("%s(): reserved %ld MiB at %08lx\n",
> +		__func__, (unsigned long)size / SZ_1M, (unsigned long)base);
>=20=20
>  	/* Architecture specific contiguous memory fixup. */
>  	dma_contiguous_early_fixup(base, size);
>  	return 0;
>  err:
> -	pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
> +	pr_err("%s(): failed to reserve %ld MiB\n",
> +		__func__, (unsigned long)size / SZ_1M);
>  	return ret;
>  }

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH V3 0/3] Add new PowerPC specific ELF core notes
From: Anshuman Khandual @ 2014-06-12  9:09 UTC (permalink / raw)
  To: Anshuman Khandual, Benjamin Herrenschmidt
  Cc: Michael Neuling, james.hogan, avagin, Paul.Clothier, davem,
	peterz, palves, linux-kernel, oleg, dhowells, linuxppc-dev, davej,
	akpm, tglx
In-Reply-To: <1400858138-3939-1-git-send-email-khandual@linux.vnet.ibm.com>

On 05/23/2014 08:45 PM, Anshuman Khandual wrote:
> 	This patch series adds five new ELF core note sections which can be
> used with existing ptrace request PTRACE_GETREGSET/SETREGSET for accessing
> various transactional memory and miscellaneous register sets on PowerPC
> platform. Please find a test program exploiting these new ELF core note
> types on a POWER8 system.
> 
> RFC: https://lkml.org/lkml/2014/4/1/292
> V1:  https://lkml.org/lkml/2014/4/2/43
> V2:  https://lkml.org/lkml/2014/5/5/88
> 
> Changes in V3
> =============
> (1) Added two new error paths in every TM related get/set functions when regset
>     support is not present on the system (ENODEV) or when the process does not
>     have any transaction active (ENODATA) in the context
> 
> (2) Installed the active hooks for all the newly added regset core note types
> 
> Changes in V2
> =============
> (1) Removed all the power specific ptrace requests corresponding to new NT_PPC_*
>     elf core note types. Now all the register sets can be accessed from ptrace
>     through PTRACE_GETREGSET/PTRACE_SETREGSET using the individual NT_PPC* core
>     note type instead
> (2) Fixed couple of attribute values for REGSET_TM_CGPR register set
> (3) Renamed flush_tmreg_to_thread as flush_tmregs_to_thread
> (4) Fixed 32 bit checkpointed GPR support
> (5) Changed commit messages accordingly
> 
> Outstanding Issues
> ==================
> (1) Running DSCR register value inside a transaction does not seem to be saved
>     at thread.dscr when the process stops for ptrace examination.

Hey Ben,

Any updates on this patch series ?

^ permalink raw reply

* Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
From: Michal Nazarewicz @ 2014-06-12  9:47 UTC (permalink / raw)
  To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-3-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> We should free memory for bitmap when we find zone mis-match,
> otherwise this memory will leak.
>
> Additionally, I copy code comment from ppc kvm's cma code to notify
> why we need to check zone mis-match.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Reviewed-by: Michal Nazarewicz <mina86@mina86.com>

> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index bd0bb81..fb0cdce 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma)
>  		base_pfn =3D pfn;
>  		for (j =3D pageblock_nr_pages; j; --j, pfn++) {
>  			WARN_ON_ONCE(!pfn_valid(pfn));
> +			/*
> +			 * alloc_contig_range requires the pfn range
> +			 * specified to be in the same zone. Make this
> +			 * simple by forcing the entire CMA resv range
> +			 * to be in the same zone.
> +			 */
>  			if (page_zone(pfn_to_page(pfn)) !=3D zone)
> -				return -EINVAL;
> +				goto err;
>  		}
>  		init_cma_reserved_pageblock(pfn_to_page(base_pfn));
>  	} while (--i);
>=20=20
>  	mutex_init(&cma->lock);
>  	return 0;
> +
> +err:
> +	kfree(cma->bitmap);
> +	return -EINVAL;
>  }

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
From: Michal Nazarewicz @ 2014-06-12  9:53 UTC (permalink / raw)
  To: Joonsoo Kim, Aneesh Kumar K.V
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Andrew Morton,
	linuxppc-dev, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <xa1toaxyjym3.fsf@mina86.com>

On Thu, Jun 12 2014, Michal Nazarewicz <mina86@mina86.com> wrote:
> I used =E2=80=9Cfunction(arg1, arg2, =E2=80=A6)=E2=80=9D at the *beginnin=
g* of functions when
> the arguments passed to the function were included in the message.  In
> all other cases I left it at just =E2=80=9Cfunction:=E2=80=9D (or just no=
 additional
> prefix).  IMO that's a reasonable strategy.

At closer inspection, I realised drivers/base/dma-contiguous.c is
Marek's code, but the above I think is still reasonable thing to do, so
I'd rather standardise on having =E2=80=9Cfunction(=E2=80=A6)=E2=80=9D only=
 at the beginning of
a function.  Just my 0.02 CHF.

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH v2 03/10] DMA, CMA: separate core cma management codes from DMA APIs
From: Michal Nazarewicz @ 2014-06-12  9:55 UTC (permalink / raw)
  To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-4-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> To prepare future generalization work on cma area management code,
> we need to separate core cma management codes from DMA APIs.
> We will extend these core functions to cover requirements of
> ppc kvm's cma area management functionality in following patches.
> This separation helps us not to touch DMA APIs while extending
> core functions.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>


--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH v2 04/10] DMA, CMA: support alignment constraint on cma region
From: Michal Nazarewicz @ 2014-06-12 10:02 UTC (permalink / raw)
  To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-5-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> ppc kvm's cma area management needs alignment constraint on

I've noticed it earlier and cannot seem to get to terms with this.  It
should IMO be PPC, KVM and CMA since those are acronyms.  But if you
have strong feelings, it's not a big issue.

> cma region. So support it to prepare generalization of cma area
> management functionality.
>
> Additionally, add some comments which tell us why alignment
> constraint is needed on cma region.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 8a44c82..bc4c171 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -219,6 +220,7 @@ core_initcall(cma_init_reserved_areas);
>   * @size: Size of the reserved area (in bytes),
>   * @base: Base address of the reserved area optional, use 0 for any
>   * @limit: End address of the reserved memory (optional, 0 for any).
> + * @alignment: Alignment for the contiguous memory area, should be
>  	power of 2

=E2=80=9Cmust be power of 2 or zero=E2=80=9D.

>   * @res_cma: Pointer to store the created cma region.
>   * @fixed: hint about where to place the reserved area
>   *
> @@ -233,15 +235,15 @@ core_initcall(cma_init_reserved_areas);
>   */
>  static int __init __dma_contiguous_reserve_area(phys_addr_t size,
>  				phys_addr_t base, phys_addr_t limit,
> +				phys_addr_t alignment,
>  				struct cma **res_cma, bool fixed)
>  {
>  	struct cma *cma =3D &cma_areas[cma_area_count];
> -	phys_addr_t alignment;
>  	int ret =3D 0;
>=20=20
> -	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
> -		 (unsigned long)size, (unsigned long)base,
> -		 (unsigned long)limit);
> +	pr_debug("%s(size %lx, base %08lx, limit %08lx align_order %08lx)\n",
> +		__func__, (unsigned long)size, (unsigned long)base,
> +		(unsigned long)limit, (unsigned long)alignment);

Nit: Align with the rest of the arguments, i.e.:

+	pr_debug("%s(size %lx, base %08lx, limit %08lx align_order %08lx)\n",
+		 __func__, (unsigned long)size, (unsigned long)base,
+		 (unsigned long)limit, (unsigned long)alignment);

>=20=20
>  	/* Sanity checks */
>  	if (cma_area_count =3D=3D ARRAY_SIZE(cma_areas)) {

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity
From: Michal Nazarewicz @ 2014-06-12 10:19 UTC (permalink / raw)
  To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-6-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> ppc kvm's cma region management requires arbitrary bitmap granularity,
> since they want to reserve very large memory and manage this region
> with bitmap that one bit for several pages to reduce management overheads.
> So support arbitrary bitmap granularity for following generalization.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index bc4c171..9bc9340 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -38,6 +38,7 @@ struct cma {
>  	unsigned long	base_pfn;
>  	unsigned long	count;

Have you considered replacing count with maxno?

>  	unsigned long	*bitmap;
> +	int order_per_bit; /* Order of pages represented by one bit */

I'd make it unsigned.

>  	struct mutex	lock;
>  };
>=20=20
> +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int
> count)

For consistency cma_clear_bitmap would make more sense I think.  On the
other hand, you're just moving stuff around so perhaps renaming the
function at this point is not worth it any more.

> +{
> +	unsigned long bitmapno, nr_bits;
> +
> +	bitmapno =3D (pfn - cma->base_pfn) >> cma->order_per_bit;
> +	nr_bits =3D cma_bitmap_pages_to_bits(cma, count);
> +
> +	mutex_lock(&cma->lock);
> +	bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> +	mutex_unlock(&cma->lock);
> +}
> +
>  static int __init cma_activate_area(struct cma *cma)
>  {
> -	int bitmap_size =3D BITS_TO_LONGS(cma->count) * sizeof(long);
> +	int bitmap_maxno =3D cma_bitmap_maxno(cma);
> +	int bitmap_size =3D BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
>  	unsigned long base_pfn =3D cma->base_pfn, pfn =3D base_pfn;
>  	unsigned i =3D cma->count >> pageblock_order;
>  	struct zone *zone;

bitmap_maxno is never used again, perhaps:

+	int bitmap_size =3D BITS_TO_LONGS(cma_bitmap_maxno(cma)) * sizeof(long);

instead? Up to you.

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH v2 08/10] mm, cma: clean-up cma allocation error path
From: Michal Nazarewicz @ 2014-06-12 11:34 UTC (permalink / raw)
  To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-9-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> We can remove one call sites for clear_cma_bitmap() if we first
> call it before checking error number.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Reviewed-by: Michal Nazarewicz <mina86@mina86.com>

> diff --git a/mm/cma.c b/mm/cma.c
> index 1e1b017..01a0713 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -282,11 +282,12 @@ struct page *cma_alloc(struct cma *cma, int count, =
unsigned int align)
>  		if (ret =3D=3D 0) {
>  			page =3D pfn_to_page(pfn);
>  			break;
> -		} else if (ret !=3D -EBUSY) {
> -			clear_cma_bitmap(cma, pfn, count);
> -			break;
>  		}
> +
>  		clear_cma_bitmap(cma, pfn, count);
> +		if (ret !=3D -EBUSY)
> +			break;
> +
>  		pr_debug("%s(): memory range at %p is busy, retrying\n",
>  			 __func__, pfn_to_page(pfn));
>  		/* try again with a bit different memory target */

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply

* Re: [PATCH v2 09/10] mm, cma: move output param to the end of param list
From: Michal Nazarewicz @ 2014-06-12 11:38 UTC (permalink / raw)
  To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
  Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
	Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-10-git-send-email-iamjoonsoo.kim@lge.com>

On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> Conventionally, we put output param to the end of param list.
> cma_declare_contiguous() doesn't look like that, so change it.

Perhaps the function should be changed to return an error-pointer?

> Additionally, move down cma_areas reference code to the position
> where it is really needed.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

>
> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book=
3s_hv_builtin.c
> index 28ec226..97613ea 100644
> --- a/arch/powerpc/kvm/book3s_hv_builtin.c
> +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
> @@ -184,7 +184,7 @@ void __init kvm_cma_reserve(void)
>=20=20
>  		align_size =3D max(kvm_rma_pages << PAGE_SHIFT, align_size);
>  		cma_declare_contiguous(selected_size, 0, 0, align_size,
> -			KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, &kvm_cma, false);
> +			KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, false, &kvm_cma);
>  	}
>  }
>=20=20
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index f177f73..bfd4553 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -149,7 +149,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t si=
ze, phys_addr_t base,
>  {
>  	int ret;
>=20=20
> -	ret =3D cma_declare_contiguous(size, base, limit, 0, 0, res_cma, fixed);
> +	ret =3D cma_declare_contiguous(size, base, limit, 0, 0, fixed, res_cma);
>  	if (ret)
>  		return ret;
>=20=20
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> index e38efe9..e53eead 100644
> --- a/include/linux/cma.h
> +++ b/include/linux/cma.h
> @@ -6,7 +6,7 @@ struct cma;
>  extern int __init cma_declare_contiguous(phys_addr_t size,
>  				phys_addr_t base, phys_addr_t limit,
>  				phys_addr_t alignment, int order_per_bit,
> -				struct cma **res_cma, bool fixed);
> +				bool fixed, struct cma **res_cma);
>  extern struct page *cma_alloc(struct cma *cma, int count, unsigned int a=
lign);
>  extern bool cma_release(struct cma *cma, struct page *pages, int count);
>  #endif
> diff --git a/mm/cma.c b/mm/cma.c
> index 01a0713..22a5b23 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -142,8 +142,8 @@ core_initcall(cma_init_reserved_areas);
>   * @limit: End address of the reserved memory (optional, 0 for any).
>   * @alignment: Alignment for the contiguous memory area, should be power=
 of 2
>   * @order_per_bit: Order of pages represented by one bit on bitmap.
> - * @res_cma: Pointer to store the created cma region.
>   * @fixed: hint about where to place the reserved area
> + * @res_cma: Pointer to store the created cma region.
>   *
>   * This function reserves memory from early allocator. It should be
>   * called by arch specific code once the early allocator (memblock or bo=
otmem)
> @@ -156,9 +156,9 @@ core_initcall(cma_init_reserved_areas);
>  int __init cma_declare_contiguous(phys_addr_t size,
>  				phys_addr_t base, phys_addr_t limit,
>  				phys_addr_t alignment, int order_per_bit,
> -				struct cma **res_cma, bool fixed)
> +				bool fixed, struct cma **res_cma)
>  {
> -	struct cma *cma =3D &cma_areas[cma_area_count];
> +	struct cma *cma;
>  	int ret =3D 0;
>=20=20
>  	pr_debug("%s(size %lx, base %08lx, limit %08lx alignment %08lx)\n",
> @@ -214,6 +214,7 @@ int __init cma_declare_contiguous(phys_addr_t size,
>  	 * Each reserved area must be initialised later, when more kernel
>  	 * subsystems (like slab allocator) are available.
>  	 */
> +	cma =3D &cma_areas[cma_area_count];
>  	cma->base_pfn =3D PFN_DOWN(base);
>  	cma->count =3D size >> PAGE_SHIFT;
>  	cma->order_per_bit =3D order_per_bit;
> --=20
> 1.7.9.5
>

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox