RE: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hi Hiroshi, On Thursday, August 23, 2012 8:15 AM Hiroshi Doyu wrote: > On Thu, 23 Aug 2012 07:58:34 +0200 > Marek Szyprowski wrote: > > > Hello, > > > > On Wednesday, August 22, 2012 3:37 PM Hiroshi Doyu wrote: > > > > > KyongHo Cho wrote @ Wed, 22 Aug 2012 14:47:00 > > > +0200: > > > > > > > vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic > > > > context. > > > > > > Right. > > > > > > I've been thinking that kzalloc() may be enough here, since > > > vzalloc() was introduced to avoid allocation failure for big chunk of > > > memory, but I think that it's unlikely that the number of page array > > > can be so big. So I propose to drop vzalloc() here, and just simply to > > > use kzalloc only as below(*1). > > > > We already had a discussion about this, so I don't think it makes much > > sense to > > change it back to kzalloc. This vmalloc() call won't hurt anyone. It should > > not > > be considered a problem for atomic allocations, because no sane driver will > > try > > to allocate buffers larger than a dozen KiB with GFP_ATOMIC flag. I would > > call > > such try a serious bug, which we should not care here. > > Ok, I've already sent v2 just now, where, instead of changing it back, > just with GFP_ATOMIC, kzalloc() would be selected, just in case. I guess > that this would be ok(a bit safer?) I've posted some comments to v2. If you agree with my suggestion, no changes around those vmalloc() calls will be needed. Best regards -- Marek Szyprowski Samsung Poland R Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hi, On Thu, 23 Aug 2012 07:58:34 +0200 Marek Szyprowski wrote: > Hello, > > On Wednesday, August 22, 2012 3:37 PM Hiroshi Doyu wrote: > > > KyongHo Cho wrote @ Wed, 22 Aug 2012 14:47:00 > > +0200: > > > > > vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic > > > context. > > > > Right. > > > > I've been thinking that kzalloc() may be enough here, since > > vzalloc() was introduced to avoid allocation failure for big chunk of > > memory, but I think that it's unlikely that the number of page array > > can be so big. So I propose to drop vzalloc() here, and just simply to > > use kzalloc only as below(*1). > > We already had a discussion about this, so I don't think it makes much sense > to > change it back to kzalloc. This vmalloc() call won't hurt anyone. It should > not > be considered a problem for atomic allocations, because no sane driver will > try > to allocate buffers larger than a dozen KiB with GFP_ATOMIC flag. I would call > such try a serious bug, which we should not care here. Ok, I've already sent v2 just now, where, instead of changing it back, just with GFP_ATOMIC, kzalloc() would be selected, just in case. I guess that this would be ok(a bit safer?) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hi, On Thu, 23 Aug 2012 07:58:34 +0200 Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On Wednesday, August 22, 2012 3:37 PM Hiroshi Doyu wrote: KyongHo Cho pullip@samsung.com wrote @ Wed, 22 Aug 2012 14:47:00 +0200: vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic context. Right. I've been thinking that kzalloc() may be enough here, since vzalloc() was introduced to avoid allocation failure for big chunk of memory, but I think that it's unlikely that the number of page array can be so big. So I propose to drop vzalloc() here, and just simply to use kzalloc only as below(*1). We already had a discussion about this, so I don't think it makes much sense to change it back to kzalloc. This vmalloc() call won't hurt anyone. It should not be considered a problem for atomic allocations, because no sane driver will try to allocate buffers larger than a dozen KiB with GFP_ATOMIC flag. I would call such try a serious bug, which we should not care here. Ok, I've already sent v2 just now, where, instead of changing it back, just with GFP_ATOMIC, kzalloc() would be selected, just in case. I guess that this would be ok(a bit safer?) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hi Hiroshi, On Thursday, August 23, 2012 8:15 AM Hiroshi Doyu wrote: On Thu, 23 Aug 2012 07:58:34 +0200 Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On Wednesday, August 22, 2012 3:37 PM Hiroshi Doyu wrote: KyongHo Cho pullip@samsung.com wrote @ Wed, 22 Aug 2012 14:47:00 +0200: vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic context. Right. I've been thinking that kzalloc() may be enough here, since vzalloc() was introduced to avoid allocation failure for big chunk of memory, but I think that it's unlikely that the number of page array can be so big. So I propose to drop vzalloc() here, and just simply to use kzalloc only as below(*1). We already had a discussion about this, so I don't think it makes much sense to change it back to kzalloc. This vmalloc() call won't hurt anyone. It should not be considered a problem for atomic allocations, because no sane driver will try to allocate buffers larger than a dozen KiB with GFP_ATOMIC flag. I would call such try a serious bug, which we should not care here. Ok, I've already sent v2 just now, where, instead of changing it back, just with GFP_ATOMIC, kzalloc() would be selected, just in case. I guess that this would be ok(a bit safer?) I've posted some comments to v2. If you agree with my suggestion, no changes around those vmalloc() calls will be needed. Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hello, On Wednesday, August 22, 2012 3:37 PM Hiroshi Doyu wrote: > KyongHo Cho wrote @ Wed, 22 Aug 2012 14:47:00 +0200: > > > vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic > > context. > > Right. > > I've been thinking that kzalloc() may be enough here, since > vzalloc() was introduced to avoid allocation failure for big chunk of > memory, but I think that it's unlikely that the number of page array > can be so big. So I propose to drop vzalloc() here, and just simply to > use kzalloc only as below(*1). We already had a discussion about this, so I don't think it makes much sense to change it back to kzalloc. This vmalloc() call won't hurt anyone. It should not be considered a problem for atomic allocations, because no sane driver will try to allocate buffers larger than a dozen KiB with GFP_ATOMIC flag. I would call such try a serious bug, which we should not care here. Best regards -- Marek Szyprowski Samsung Poland R Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
On Wed, Aug 22, 2012 at 03:36:48PM +0200, Hiroshi Doyu wrote: > Hi, > > KyongHo Cho wrote @ Wed, 22 Aug 2012 14:47:00 +0200: > > > vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic > > context. > > Right. > > I've been thinking that kzalloc() may be enough here, since > vzalloc() was introduced to avoid allocation failure for big chunk of > memory, but I think that it's unlikely that the number of page array > can be so big. So I propose to drop vzalloc() here, and just simply to > use kzalloc only as below(*1). > > For example, > > 1920(H) x 1080(W) x 4(bytes) ~= 8MiB > > For 8 MiB buffer, > 8(MiB) * 1024 = 8192(KiB) > 8192(KiB) / 4(KiB/page) = 2048 pages > sizeof(struct page *) = 4 bytes > 2048(pages) * 4(bytes/page) = 8192(bytes) = 8(KiB) > 8(KiB) / 4(KiB/page) = 2 pages > > If the above estimation is right(I hope;)), the necessary pages are > _at most_ 2 pages. If the system gets into the situation to fail to > allocate 2 contiguous pages, that's real the problem. I guess that > that kind of fragmentation problem would be solved with page migration > or something, especially nowadays devices are getting larger memories. In atomic context, VM have no choice except relying on kswapd so high order allocation can fail easily when memory fragementation is high. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hi, KyongHo Cho wrote @ Wed, 22 Aug 2012 14:47:00 +0200: > vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic context. Right. I've been thinking that kzalloc() may be enough here, since vzalloc() was introduced to avoid allocation failure for big chunk of memory, but I think that it's unlikely that the number of page array can be so big. So I propose to drop vzalloc() here, and just simply to use kzalloc only as below(*1). For example, 1920(H) x 1080(W) x 4(bytes) ~= 8MiB For 8 MiB buffer, 8(MiB) * 1024 = 8192(KiB) 8192(KiB) / 4(KiB/page) = 2048 pages sizeof(struct page *) = 4 bytes 2048(pages) * 4(bytes/page) = 8192(bytes) = 8(KiB) 8(KiB) / 4(KiB/page) = 2 pages If the above estimation is right(I hope;)), the necessary pages are _at most_ 2 pages. If the system gets into the situation to fail to allocate 2 contiguous pages, that's real the problem. I guess that that kind of fragmentation problem would be solved with page migration or something, especially nowadays devices are getting larger memories. *1: >From a613c40d1b3d4fb1577cdb0807a74e8dbd08a3e6 Mon Sep 17 00:00:00 2001 From: Hiroshi Doyu Date: Wed, 22 Aug 2012 16:25:54 +0300 Subject: [PATCH 1/1] ARM: dma-mapping: Use only kzalloc without vzalloc Use only kzalloc for atomic allocation. Signed-off-by: Hiroshi Doyu --- arch/arm/mm/dma-mapping.c | 10 ++ 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 4656c0f..d4f1cf2 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1083,10 +1083,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, int count = size >> PAGE_SHIFT; int array_size = count * sizeof(struct page *); - if (array_size <= PAGE_SIZE) - pages = kzalloc(array_size, gfp); - else - pages = vzalloc(array_size); + pages = kzalloc(array_size, gfp); if (!pages) return NULL; @@ -1107,10 +1104,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, return pages; error: - if (array_size <= PAGE_SIZE) - kfree(pages); - else - vfree(pages); + kfree(pages); return NULL; } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Marek Szyprowski wrote @ Wed, 22 Aug 2012 14:29:47
+0200:
> Hello,
>
> On Wednesday, August 22, 2012 12:20 PM Hiroshi Doyu wrote:
>
> > Makes use of the same atomic pool from DMA, and skips kernel page
> > mapping which can involves sleep'able operation at allocating a kernel
> > page table.
> >
> > Signed-off-by: Hiroshi Doyu
> > ---
> > arch/arm/mm/dma-mapping.c | 22 ++
> > 1 files changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index aec0c06..9260107 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -1028,7 +1028,6 @@ static struct page **__iommu_alloc_buffer(struct
> > device *dev, size_t
> > size,
> > struct page **pages;
> > int count = size >> PAGE_SHIFT;
> > int array_size = count * sizeof(struct page *);
> > - int err;
> >
> > if (array_size <= PAGE_SIZE)
> > pages = kzalloc(array_size, gfp);
> > @@ -1037,9 +1036,20 @@ static struct page **__iommu_alloc_buffer(struct
> > device *dev, size_t
> > size,
> > if (!pages)
> > return NULL;
> >
> > - err = __alloc_fill_pages(, count, gfp);
> > - if (err)
> > - goto error
> > + if (gfp & GFP_ATOMIC) {
> > + struct page *page;
> > + int i;
> > + void *addr = __alloc_from_pool(size, );
> > + if (!addr)
> > + goto err_out;
> > +
> > + for (i = 0; i < count; i++)
> > + pages[i] = page + i;
> > + } else {
> > + int err = __alloc_fill_pages(, count, gfp);
> > + if (err)
> > + goto error;
> > + }
> >
> > return pages;
> > error:
> > @@ -1055,6 +1065,10 @@ static int __iommu_free_buffer(struct device *dev,
> > struct page **pages,
> > size_t s
> > int count = size >> PAGE_SHIFT;
> > int array_size = count * sizeof(struct page *);
> > int i;
> > +
> > + if (__free_from_pool(page_address(pages[0]), size))
> > + return 0;
>
> You leak memory here. pages array should be also freed.
Right, I'll fix as below:
Modified arch/arm/mm/dma-mapping.c
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 47c4978..4656c0f 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1121,11 +1121,12 @@ static int __iommu_free_buffer(struct device *dev,
struct page **pages, size_t s
int i;
if (__free_from_pool(page_address(pages[0]), size))
- return 0;
+ goto out;
for (i = 0; i < count; i++)
if (pages[i])
__free_pages(pages[i], 0);
+out:
if (array_size <= PAGE_SIZE)
kfree(pages);
else
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hello,
On Wednesday, August 22, 2012 12:20 PM Hiroshi Doyu wrote:
> Makes use of the same atomic pool from DMA, and skips kernel page
> mapping which can involves sleep'able operation at allocating a kernel
> page table.
>
> Signed-off-by: Hiroshi Doyu
> ---
> arch/arm/mm/dma-mapping.c | 22 ++
> 1 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index aec0c06..9260107 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1028,7 +1028,6 @@ static struct page **__iommu_alloc_buffer(struct device
> *dev, size_t
> size,
> struct page **pages;
> int count = size >> PAGE_SHIFT;
> int array_size = count * sizeof(struct page *);
> - int err;
>
> if (array_size <= PAGE_SIZE)
> pages = kzalloc(array_size, gfp);
> @@ -1037,9 +1036,20 @@ static struct page **__iommu_alloc_buffer(struct
> device *dev, size_t
> size,
> if (!pages)
> return NULL;
>
> - err = __alloc_fill_pages(, count, gfp);
> - if (err)
> - goto error
> + if (gfp & GFP_ATOMIC) {
> + struct page *page;
> + int i;
> + void *addr = __alloc_from_pool(size, );
> + if (!addr)
> + goto err_out;
> +
> + for (i = 0; i < count; i++)
> + pages[i] = page + i;
> + } else {
> + int err = __alloc_fill_pages(, count, gfp);
> + if (err)
> + goto error;
> + }
>
> return pages;
> error:
> @@ -1055,6 +1065,10 @@ static int __iommu_free_buffer(struct device *dev,
> struct page **pages,
> size_t s
> int count = size >> PAGE_SHIFT;
> int array_size = count * sizeof(struct page *);
> int i;
> +
> + if (__free_from_pool(page_address(pages[0]), size))
> + return 0;
You leak memory here. pages array should be also freed.
> +
> for (i = 0; i < count; i++)
> if (pages[i])
> __free_pages(pages[i], 0);
> --
> 1.7.5.4
Best regards
--
Marek Szyprowski
Samsung Poland R Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hello,
On Wednesday, August 22, 2012 12:20 PM Hiroshi Doyu wrote:
Makes use of the same atomic pool from DMA, and skips kernel page
mapping which can involves sleep'able operation at allocating a kernel
page table.
Signed-off-by: Hiroshi Doyu hd...@nvidia.com
---
arch/arm/mm/dma-mapping.c | 22 ++
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index aec0c06..9260107 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1028,7 +1028,6 @@ static struct page **__iommu_alloc_buffer(struct device
*dev, size_t
size,
struct page **pages;
int count = size PAGE_SHIFT;
int array_size = count * sizeof(struct page *);
- int err;
if (array_size = PAGE_SIZE)
pages = kzalloc(array_size, gfp);
@@ -1037,9 +1036,20 @@ static struct page **__iommu_alloc_buffer(struct
device *dev, size_t
size,
if (!pages)
return NULL;
- err = __alloc_fill_pages(pages, count, gfp);
- if (err)
- goto error
+ if (gfp GFP_ATOMIC) {
+ struct page *page;
+ int i;
+ void *addr = __alloc_from_pool(size, page);
+ if (!addr)
+ goto err_out;
+
+ for (i = 0; i count; i++)
+ pages[i] = page + i;
+ } else {
+ int err = __alloc_fill_pages(pages, count, gfp);
+ if (err)
+ goto error;
+ }
return pages;
error:
@@ -1055,6 +1065,10 @@ static int __iommu_free_buffer(struct device *dev,
struct page **pages,
size_t s
int count = size PAGE_SHIFT;
int array_size = count * sizeof(struct page *);
int i;
+
+ if (__free_from_pool(page_address(pages[0]), size))
+ return 0;
You leak memory here. pages array should be also freed.
+
for (i = 0; i count; i++)
if (pages[i])
__free_pages(pages[i], 0);
--
1.7.5.4
Best regards
--
Marek Szyprowski
Samsung Poland RD Center
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Marek Szyprowski m.szyprow...@samsung.com wrote @ Wed, 22 Aug 2012 14:29:47
+0200:
Hello,
On Wednesday, August 22, 2012 12:20 PM Hiroshi Doyu wrote:
Makes use of the same atomic pool from DMA, and skips kernel page
mapping which can involves sleep'able operation at allocating a kernel
page table.
Signed-off-by: Hiroshi Doyu hd...@nvidia.com
---
arch/arm/mm/dma-mapping.c | 22 ++
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index aec0c06..9260107 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1028,7 +1028,6 @@ static struct page **__iommu_alloc_buffer(struct
device *dev, size_t
size,
struct page **pages;
int count = size PAGE_SHIFT;
int array_size = count * sizeof(struct page *);
- int err;
if (array_size = PAGE_SIZE)
pages = kzalloc(array_size, gfp);
@@ -1037,9 +1036,20 @@ static struct page **__iommu_alloc_buffer(struct
device *dev, size_t
size,
if (!pages)
return NULL;
- err = __alloc_fill_pages(pages, count, gfp);
- if (err)
- goto error
+ if (gfp GFP_ATOMIC) {
+ struct page *page;
+ int i;
+ void *addr = __alloc_from_pool(size, page);
+ if (!addr)
+ goto err_out;
+
+ for (i = 0; i count; i++)
+ pages[i] = page + i;
+ } else {
+ int err = __alloc_fill_pages(pages, count, gfp);
+ if (err)
+ goto error;
+ }
return pages;
error:
@@ -1055,6 +1065,10 @@ static int __iommu_free_buffer(struct device *dev,
struct page **pages,
size_t s
int count = size PAGE_SHIFT;
int array_size = count * sizeof(struct page *);
int i;
+
+ if (__free_from_pool(page_address(pages[0]), size))
+ return 0;
You leak memory here. pages array should be also freed.
Right, I'll fix as below:
Modified arch/arm/mm/dma-mapping.c
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 47c4978..4656c0f 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1121,11 +1121,12 @@ static int __iommu_free_buffer(struct device *dev,
struct page **pages, size_t s
int i;
if (__free_from_pool(page_address(pages[0]), size))
- return 0;
+ goto out;
for (i = 0; i count; i++)
if (pages[i])
__free_pages(pages[i], 0);
+out:
if (array_size = PAGE_SIZE)
kfree(pages);
else
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hi, KyongHo Cho pullip@samsung.com wrote @ Wed, 22 Aug 2012 14:47:00 +0200: vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic context. Right. I've been thinking that kzalloc() may be enough here, since vzalloc() was introduced to avoid allocation failure for big chunk of memory, but I think that it's unlikely that the number of page array can be so big. So I propose to drop vzalloc() here, and just simply to use kzalloc only as below(*1). For example, 1920(H) x 1080(W) x 4(bytes) ~= 8MiB For 8 MiB buffer, 8(MiB) * 1024 = 8192(KiB) 8192(KiB) / 4(KiB/page) = 2048 pages sizeof(struct page *) = 4 bytes 2048(pages) * 4(bytes/page) = 8192(bytes) = 8(KiB) 8(KiB) / 4(KiB/page) = 2 pages If the above estimation is right(I hope;)), the necessary pages are _at most_ 2 pages. If the system gets into the situation to fail to allocate 2 contiguous pages, that's real the problem. I guess that that kind of fragmentation problem would be solved with page migration or something, especially nowadays devices are getting larger memories. *1: From a613c40d1b3d4fb1577cdb0807a74e8dbd08a3e6 Mon Sep 17 00:00:00 2001 From: Hiroshi Doyu hd...@nvidia.com Date: Wed, 22 Aug 2012 16:25:54 +0300 Subject: [PATCH 1/1] ARM: dma-mapping: Use only kzalloc without vzalloc Use only kzalloc for atomic allocation. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- arch/arm/mm/dma-mapping.c | 10 ++ 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 4656c0f..d4f1cf2 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1083,10 +1083,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, int count = size PAGE_SHIFT; int array_size = count * sizeof(struct page *); - if (array_size = PAGE_SIZE) - pages = kzalloc(array_size, gfp); - else - pages = vzalloc(array_size); + pages = kzalloc(array_size, gfp); if (!pages) return NULL; @@ -1107,10 +1104,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, return pages; error: - if (array_size = PAGE_SIZE) - kfree(pages); - else - vfree(pages); + kfree(pages); return NULL; } -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
On Wed, Aug 22, 2012 at 03:36:48PM +0200, Hiroshi Doyu wrote: Hi, KyongHo Cho pullip@samsung.com wrote @ Wed, 22 Aug 2012 14:47:00 +0200: vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic context. Right. I've been thinking that kzalloc() may be enough here, since vzalloc() was introduced to avoid allocation failure for big chunk of memory, but I think that it's unlikely that the number of page array can be so big. So I propose to drop vzalloc() here, and just simply to use kzalloc only as below(*1). For example, 1920(H) x 1080(W) x 4(bytes) ~= 8MiB For 8 MiB buffer, 8(MiB) * 1024 = 8192(KiB) 8192(KiB) / 4(KiB/page) = 2048 pages sizeof(struct page *) = 4 bytes 2048(pages) * 4(bytes/page) = 8192(bytes) = 8(KiB) 8(KiB) / 4(KiB/page) = 2 pages If the above estimation is right(I hope;)), the necessary pages are _at most_ 2 pages. If the system gets into the situation to fail to allocate 2 contiguous pages, that's real the problem. I guess that that kind of fragmentation problem would be solved with page migration or something, especially nowadays devices are getting larger memories. In atomic context, VM have no choice except relying on kswapd so high order allocation can fail easily when memory fragementation is high. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC 2/4] ARM: dma-mapping: IOMMU allocates pages from pool with GFP_ATOMIC
Hello, On Wednesday, August 22, 2012 3:37 PM Hiroshi Doyu wrote: KyongHo Cho pullip@samsung.com wrote @ Wed, 22 Aug 2012 14:47:00 +0200: vzalloc() call in __iommu_alloc_buffer() also causes BUG() in atomic context. Right. I've been thinking that kzalloc() may be enough here, since vzalloc() was introduced to avoid allocation failure for big chunk of memory, but I think that it's unlikely that the number of page array can be so big. So I propose to drop vzalloc() here, and just simply to use kzalloc only as below(*1). We already had a discussion about this, so I don't think it makes much sense to change it back to kzalloc. This vmalloc() call won't hurt anyone. It should not be considered a problem for atomic allocations, because no sane driver will try to allocate buffers larger than a dozen KiB with GFP_ATOMIC flag. I would call such try a serious bug, which we should not care here. Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
