| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <5A0C7391.3090801@samsung.com>
Date: Thu, 16 Nov 2017 02:04:17 +0900
From: Jaewon Kim <jaewon31.kim@...sung.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>, hch@....de,
robin.murphy@....com, gregkh@...uxfoundation.org,
iommu@...ts.linux-foundation.org
Cc: akpm@...ux-foundation.org, mhocko@...e.com, vbabka@...e.cz,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
jaewon31.kim@...il.com
Subject: Re: [RFC PATCH] drivers: base: dma-coherent: find free region
without alignment
Hello Marek
On 2017년 11월 14일 20:07, Marek Szyprowski wrote:
> Hi Jaewon,
>
> On 2017-11-14 09:42, Jaewon Kim wrote:
>> dma-coherent uses bitmap API which internally consider align based on the
>> requested size. Depending on some usage pattern, using align, I think, may
>> be good for fast search and anti-fragmentation. But with the align, an
>> allocation may be failed.
>>
>> This is a example, total size is 30MB, only few memory at front is being
>> used, and 9MB is being requsted. Then 9MB will be aligned to 16MB. The
>> first try on offset 0MB will be failed because of others already using. The
>> second try on offset 16MB will be failed because of ouf of bound.
>>
>> So if the align is not necessary on dma-coherent, this patch removes the
>> align policy to allow allocation without increasing the total size.
>
> You are right that keeping strict alignment is waste of memory for large
> allocations. However for the smaller ones, typically under 1MiB, it helps
> to reduce memory fragmentation. The alignment of the allocated buffers is
> de-facto guaranteed by the memory management framework in Linux kernel
> and there are drivers that depends on this feature.
>
> Maybe it would make sense to keep alignment for buffers smaller than some
> predefined value (like 1MiB), something similar to config
> ARM_DMA_IOMMU_ALIGNMENT in arch/arm/Kconfig. Otherwise I would expect that
> some drivers will be broken by this patch.
Thank you for your comment.
I looked ARM_DMA_IOMMU_ALIGNMENT in ARM, it looks similar but it is using
bitmap_find_next_zero_area rather than bitmap_find_free_region. bitmap_find_next_zero_area
apply aligning only onto offset but not onto size. So I think ARM_DMA_IOMMU_ALIGNMENT way
is not perfect on this dma-coherent APIs which tries to align even on size.
Let me say another way where each reserved_mem from device tree can decide if it wants aligning.
This could be implemented like below. I need to change other dma-coherent APIs though.
I will wait for your comment on this.
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -17,6 +17,7 @@ struct dma_coherent_mem {
unsigned long *bitmap;
spinlock_t spinlock;
bool use_dev_dma_pfn_offset;
+ bool no_align;
};
static struct dma_coherent_mem *dma_coherent_default_memory __ro_after_init;
@@ -162,7 +163,6 @@ EXPORT_SYMBOL(dma_mark_declared_memory_occupied);
static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
ssize_t size, dma_addr_t *dma_handle)
{
- int order = get_order(size);
unsigned long flags;
int pageno;
void *ret;
@@ -172,9 +172,21 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
if (unlikely(size > (mem->size << PAGE_SHIFT)))
goto err;
- pageno = bitmap_find_free_region(mem->bitmap, mem->size, order);
- if (unlikely(pageno < 0))
- goto err;
+ if (mem->no_align) {
+ int nr_page = PAGE_ALIGN(size) >> PAGE_SHIFT;
+
+ pageno = bitmap_find_next_zero_area(mem->bitmap, mem->size, 0,
+ nr_page, 0);
+ if (unlikely(pageno >= mem->size))
+ goto err;
+ bitmap_set(mem->bitmap, pageno, nr_page);
+ } else {
+ int order = get_order(size);
+
+ pageno = bitmap_find_free_region(mem->bitmap, mem->size, order);
+ if (unlikely(pageno < 0))
+ goto err;
+ }
/*
* Memory was found in the coherent area.
@@ -346,6 +358,7 @@ static struct reserved_mem *dma_reserved_default_memory __initdata;
static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
{
struct dma_coherent_mem *mem = rmem->priv;
+ unsigned long node = rmem->fdt_node;
int ret;
if (!mem) {
@@ -360,6 +373,8 @@ static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
}
mem->use_dev_dma_pfn_offset = true;
rmem->priv = mem;
+ if (of_get_flat_dt_prop(node, "no-align", NULL))
+ mem->no_align = true;
dma_assign_coherent_memory(dev, mem);
return 0;
}
>
>> Signed-off-by: Jaewon Kim <jaewon31.kim@...sung.com>
>> ---
>> drivers/base/dma-coherent.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
>> index 744f64f43454..b86a96d0cd07 100644
>> --- a/drivers/base/dma-coherent.c
>> +++ b/drivers/base/dma-coherent.c
>> @@ -162,7 +162,7 @@ EXPORT_SYMBOL(dma_mark_declared_memory_occupied);
>> static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>> ssize_t size, dma_addr_t *dma_handle)
>> {
>> - int order = get_order(size);
>> + int nr_page = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> unsigned long flags;
>> int pageno;
>> void *ret;
>> @@ -172,9 +172,11 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>> if (unlikely(size > (mem->size << PAGE_SHIFT)))
>> goto err;
>> - pageno = bitmap_find_free_region(mem->bitmap, mem->size, order);
>> - if (unlikely(pageno < 0))
>> + pageno = bitmap_find_next_zero_area(mem->bitmap, mem->size, 0,
>> + nr_page, 0);
>> + if (unlikely(pageno >= mem->size)) {
>> goto err;
>> + bitmap_set(mem->bitmap, pageno, nr_page);
>> /*
>> * Memory was found in the coherent area.
>
> Best regards
Powered by blists - more mailing lists