LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] Clean up document and small improvement of atomic pool
@ 2021-12-17  9:08 Baoquan He
  2021-12-17  9:08 ` [PATCH 1/2] docs: kernel-parameters: Update to reflect the current default size " Baoquan He
  2021-12-17  9:08 ` [PATCH 2/2] dma-pool: allow user to disable " Baoquan He
  0 siblings, 2 replies; 6+ messages in thread
From: Baoquan He @ 2021-12-17  9:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-mm, akpm, corbet, hch, cl, bhe

This patchset contains two small patches. Identified this when I tried
to fix a kdump bug that page allocation failure is always warned out
during kdump kernel on x86_64. Since dma atomic pool is initialized very
early during bootup, that allocation failure is always triggered in
dma_atomic_pool_init() firstly. I originally planned to add an kernel
option to disable each of the three dma atomic pool. Finally I realized
it's not only dma atomic pool getting the issue, any later requsting
page from DMA zone will fail. So I changed solution to fix the kdump bug.
While the document clean up and the small fix which allows user to
disable atomic pool makes sense to improve. The small fix which makes
'coherent_pool=0' disable dma atomic pool like 'cma=0', looks more
reasonable than the old behavioud which will take the default atomic
size.

[PATCH RESEND v2 0/5] Avoid requesting page from DMA zone when no managed pages
https://lore.kernel.org/all/20211207030750.30824-1-bhe@redhat.com/T/#u



Baoquan He (2):
  docs: kernel-parameters: Update to reflect the current default size of
    atomic pool
  dma-pool: allow user to disable atomic pool

 Documentation/admin-guide/kernel-parameters.txt | 5 ++++-
 kernel/dma/pool.c                               | 7 +++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.26.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] docs: kernel-parameters: Update to reflect the current default size of atomic pool
  2021-12-17  9:08 [PATCH 0/2] Clean up document and small improvement of atomic pool Baoquan He
@ 2021-12-17  9:08 ` Baoquan He
  2021-12-21  8:50   ` Christoph Hellwig
  2021-12-17  9:08 ` [PATCH 2/2] dma-pool: allow user to disable " Baoquan He
  1 sibling, 1 reply; 6+ messages in thread
From: Baoquan He @ 2021-12-17  9:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-mm, akpm, corbet, hch, cl, bhe

Since commit 1d659236fb43("dma-pool: scale the default DMA coherent pool
size with memory capacity"), the default size of atomic pool has been
changed to take by scaling with system memory capacity. So update the
document in kerenl-parameter.txt accordingly.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9725c546a0d4..ec4d25e854a8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -664,7 +664,9 @@
 
 	coherent_pool=nn[KMG]	[ARM,KNL]
 			Sets the size of memory pool for coherent, atomic dma
-			allocations, by default set to 256K.
+			allocations. Otherwise the default size will be scaled
+			with memory capacity, while clamped between 128K and
+			1 << (PAGE_SHIFT + MAX_ORDER-1).
 
 	com20020=	[HW,NET] ARCnet - COM20020 chipset
 			Format:
-- 
2.26.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] dma-pool: allow user to disable atomic pool
  2021-12-17  9:08 [PATCH 0/2] Clean up document and small improvement of atomic pool Baoquan He
  2021-12-17  9:08 ` [PATCH 1/2] docs: kernel-parameters: Update to reflect the current default size " Baoquan He
@ 2021-12-17  9:08 ` Baoquan He
  2021-12-21  8:52   ` Christoph Hellwig
  1 sibling, 1 reply; 6+ messages in thread
From: Baoquan He @ 2021-12-17  9:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-mm, akpm, corbet, hch, cl, bhe

In the current code, three atomic memory pools are always created,
atomic_pool_kernel|dma|dma32, even though 'coherent_pool=0' is
specified in kernel command line. In fact, atomic pool is only
necessary when CONFIG_DMA_DIRECT_REMAP=y or mem_encrypt_active=y
which are needed on few ARCHes.

So change code to allow user to disable atomic pool by specifying
'coherent_pool=0'.

Meanwhile, update the relevant document in kernel-parameter.txt.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 ++-
 kernel/dma/pool.c                               | 7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ec4d25e854a8..d7015309614b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -664,7 +664,8 @@
 
 	coherent_pool=nn[KMG]	[ARM,KNL]
 			Sets the size of memory pool for coherent, atomic dma
-			allocations. Otherwise the default size will be scaled
+			allocations. A value of 0 disables the three atomic
+			memory pool. Otherwise the default size will be scaled
 			with memory capacity, while clamped between 128K and
 			1 << (PAGE_SHIFT + MAX_ORDER-1).
 
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 5f84e6cdb78e..5a85804b5beb 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -21,7 +21,7 @@ static struct gen_pool *atomic_pool_kernel __ro_after_init;
 static unsigned long pool_size_kernel;
 
 /* Size can be defined by the coherent_pool command line */
-static size_t atomic_pool_size;
+static unsigned long atomic_pool_size = -1;
 
 /* Dynamic background expansion when the atomic pool is near capacity */
 static struct work_struct atomic_pool_work;
@@ -188,11 +188,14 @@ static int __init dma_atomic_pool_init(void)
 {
 	int ret = 0;
 
+	if (!atomic_pool_size)
+		return 0;
+
 	/*
 	 * If coherent_pool was not used on the command line, default the pool
 	 * sizes to 128KB per 1GB of memory, min 128KB, max MAX_ORDER-1.
 	 */
-	if (!atomic_pool_size) {
+	if (atomic_pool_size == -1) {
 		unsigned long pages = totalram_pages() / (SZ_1G / SZ_128K);
 		pages = min_t(unsigned long, pages, MAX_ORDER_NR_PAGES);
 		atomic_pool_size = max_t(size_t, pages << PAGE_SHIFT, SZ_128K);
-- 
2.26.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] docs: kernel-parameters: Update to reflect the current default size of atomic pool
  2021-12-17  9:08 ` [PATCH 1/2] docs: kernel-parameters: Update to reflect the current default size " Baoquan He
@ 2021-12-21  8:50   ` Christoph Hellwig
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2021-12-21  8:50 UTC (permalink / raw)
  To: Baoquan He; +Cc: linux-kernel, linux-mm, akpm, corbet, hch, cl

On Fri, Dec 17, 2021 at 05:08:26PM +0800, Baoquan He wrote:
> Since commit 1d659236fb43("dma-pool: scale the default DMA coherent pool
> size with memory capacity"), the default size of atomic pool has been
> changed to take by scaling with system memory capacity. So update the
> document in kerenl-parameter.txt accordingly.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] dma-pool: allow user to disable atomic pool
  2021-12-17  9:08 ` [PATCH 2/2] dma-pool: allow user to disable " Baoquan He
@ 2021-12-21  8:52   ` Christoph Hellwig
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2021-12-21  8:52 UTC (permalink / raw)
  To: Baoquan He; +Cc: linux-kernel, linux-mm, akpm, corbet, hch, cl

On Fri, Dec 17, 2021 at 05:08:27PM +0800, Baoquan He wrote:
> In the current code, three atomic memory pools are always created,
> atomic_pool_kernel|dma|dma32, even though 'coherent_pool=0' is
> specified in kernel command line. In fact, atomic pool is only
> necessary when CONFIG_DMA_DIRECT_REMAP=y or mem_encrypt_active=y
> which are needed on few ARCHes.
> 
> So change code to allow user to disable atomic pool by specifying
> 'coherent_pool=0'.
> 
> Meanwhile, update the relevant document in kernel-parameter.txt.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 3 ++-
>  kernel/dma/pool.c                               | 7 +++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index ec4d25e854a8..d7015309614b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -664,7 +664,8 @@
>  
>  	coherent_pool=nn[KMG]	[ARM,KNL]
>  			Sets the size of memory pool for coherent, atomic dma
> -			allocations. Otherwise the default size will be scaled
> +			allocations. A value of 0 disables the three atomic
> +			memory pool. Otherwise the default size will be scaled
>  			with memory capacity, while clamped between 128K and
>  			1 << (PAGE_SHIFT + MAX_ORDER-1).
>  
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 5f84e6cdb78e..5a85804b5beb 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -21,7 +21,7 @@ static struct gen_pool *atomic_pool_kernel __ro_after_init;
>  static unsigned long pool_size_kernel;
>  
>  /* Size can be defined by the coherent_pool command line */
> -static size_t atomic_pool_size;
> +static unsigned long atomic_pool_size = -1;

Using UINT_MAX instead of -1 here and below would probably be more
descriptive.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] dma-pool: allow user to disable atomic pool
  2021-06-24  5:20 [PATCH RFC 0/2] " Baoquan He
@ 2021-06-24  5:20 ` Baoquan He
  0 siblings, 0 replies; 6+ messages in thread
From: Baoquan He @ 2021-06-24  5:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, x86, rientjes, rppt, thomas.lendacky, brijesh.singh,
	kexec, bhe

In the current code, three atomic memory pools are provided,
atomic_pool_kernel, atomic_pool_dma, atomic_pool_dma32, initialized
with flag GFP_KERNEL, GFP_KERNEL|GFP_DMA, GFP_KERNEL|GFP_DMA32.
And they are always enabled, even though 'coherent_pool=0' is
specified in kernel command line.

In some cases, atomic pool may not be needed. And worse, it even will
cause problem. E.g in kdump kernel of x86_64, it will cause OOM for
atomic_pool_dma initialization. Because there isn't available memory
for buddy allocatory in DMA zone of kdump kernel since commit
f1d4d47c5851 ("x86/setup: Always reserve the first 1M of RAM"). 
The OOM will cause panic if panic_on_oom is added into kdump kernel.

So change code to adjust the existing coherent_pool to allow user
to disable atomic pool by specifying 'coherent_pool=0'.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 kernel/dma/pool.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 5f84e6cdb78e..5a85804b5beb 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -21,7 +21,7 @@ static struct gen_pool *atomic_pool_kernel __ro_after_init;
 static unsigned long pool_size_kernel;
 
 /* Size can be defined by the coherent_pool command line */
-static size_t atomic_pool_size;
+static unsigned long atomic_pool_size = -1;
 
 /* Dynamic background expansion when the atomic pool is near capacity */
 static struct work_struct atomic_pool_work;
@@ -188,11 +188,14 @@ static int __init dma_atomic_pool_init(void)
 {
 	int ret = 0;
 
+	if (!atomic_pool_size)
+		return 0;
+
 	/*
 	 * If coherent_pool was not used on the command line, default the pool
 	 * sizes to 128KB per 1GB of memory, min 128KB, max MAX_ORDER-1.
 	 */
-	if (!atomic_pool_size) {
+	if (atomic_pool_size == -1) {
 		unsigned long pages = totalram_pages() / (SZ_1G / SZ_128K);
 		pages = min_t(unsigned long, pages, MAX_ORDER_NR_PAGES);
 		atomic_pool_size = max_t(size_t, pages << PAGE_SHIFT, SZ_128K);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-21  8:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-17  9:08 [PATCH 0/2] Clean up document and small improvement of atomic pool Baoquan He
2021-12-17  9:08 ` [PATCH 1/2] docs: kernel-parameters: Update to reflect the current default size " Baoquan He
2021-12-21  8:50   ` Christoph Hellwig
2021-12-17  9:08 ` [PATCH 2/2] dma-pool: allow user to disable " Baoquan He
2021-12-21  8:52   ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2021-06-24  5:20 [PATCH RFC 0/2] " Baoquan He
2021-06-24  5:20 ` [PATCH 2/2] " Baoquan He

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).