From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758603AbbA0PIv (ORCPT ); Tue, 27 Jan 2015 10:08:51 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:33898 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754801AbbA0PIt (ORCPT ); Tue, 27 Jan 2015 10:08:49 -0500 Message-ID: <54C7A9F6.1080500@oracle.com> Date: Tue, 27 Jan 2015 10:08:38 -0500 From: Sasha Levin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Joonsoo Kim CC: linux-kernel@vger.kernel.org, m.szyprowski@samsung.com, akpm@linux-foundation.org, lauraa@codeaurora.org Subject: Re: [PATCH v2 2/3] mm: cma: allocation trigger References: <1422282365-20015-1-git-send-email-sasha.levin@oracle.com> <1422282365-20015-3-git-send-email-sasha.levin@oracle.com> <20150127080603.GC11358@js1304-P5Q-DELUXE> In-Reply-To: <20150127080603.GC11358@js1304-P5Q-DELUXE> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: ucsinet22.oracle.com [156.151.31.94] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/27/2015 03:06 AM, Joonsoo Kim wrote: > On Mon, Jan 26, 2015 at 09:26:04AM -0500, Sasha Levin wrote: >> Provides a userspace interface to trigger a CMA allocation. >> >> Usage: >> >> echo [pages] > alloc >> >> This would provide testing/fuzzing access to the CMA allocation paths. >> >> Signed-off-by: Sasha Levin >> --- >> mm/cma_debug.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 58 insertions(+), 2 deletions(-) >> >> diff --git a/mm/cma_debug.c b/mm/cma_debug.c >> index 3a25413..39c7116 100644 >> --- a/mm/cma_debug.c >> +++ b/mm/cma_debug.c >> @@ -7,9 +7,22 @@ >> >> #include >> #include >> +#include >> +#include >> +#include > > dma-contiguous.h doesn't needed now. Right. >> +#include >> >> #include "cma.h" >> >> +struct cma_mem { >> + struct hlist_node node; >> + struct page *p; >> + unsigned long n; >> +}; >> + >> +static HLIST_HEAD(cma_mem_head); >> +static DEFINE_SPINLOCK(cma_mem_head_lock); >> + >> static struct dentry *cma_debugfs_root; > > How about keeping cma_mem_head on each cma area separately? Good point, we're mixing allocations here. >> static int cma_debugfs_get(void *data, u64 *val) >> @@ -23,8 +36,48 @@ static int cma_debugfs_get(void *data, u64 *val) >> >> DEFINE_SIMPLE_ATTRIBUTE(cma_debugfs_fops, cma_debugfs_get, NULL, "%llu\n"); >> >> -static void cma_debugfs_add_one(struct cma *cma, int idx) >> +static void cma_add_to_cma_mem_list(struct cma_mem *mem) >> +{ >> + spin_lock(&cma_mem_head_lock); >> + hlist_add_head(&mem->node, &cma_mem_head); >> + spin_unlock(&cma_mem_head_lock); >> +} >> + >> +static int cma_alloc_mem(struct cma *cma, int count) >> { >> + struct cma_mem *mem; >> + struct page *p; >> + >> + mem = kzalloc(sizeof(*mem), GFP_KERNEL); >> + if (!mem) >> + return -ENOMEM; >> + >> + p = cma_alloc(cma, count, CONFIG_CMA_ALIGNMENT); >> + if (!p) { >> + kfree(mem); >> + return -ENOMEM; >> + } > > CONFIG_CMA_ALIGNMENT looks not good. It means just maximum aligment > so it is odd to use this value in testing. Is there special meaning > to use it here? No good reason, I stole that from a different piece of code. > Could we also get alignment parameter from user? Something like below. > > echo "4 1" > alloc > 4 for number of pages > 1 for alignment. > > If it is impossible, just 0 looks better than CONFIG_CMA_ALIGNMENT. I'd rather keep it simple and use a single parameter for now. Thanks, Sasha