diff mbox

cma: Take __GFP_NOWARN into account in cma_alloc()

Message ID 20171004125447.15195-1-boris.brezillon@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Boris BREZILLON Oct. 4, 2017, 12:54 p.m. UTC
cma_alloc() unconditionally prints an INFO message when the CMA
allocation fails. Make this message conditional on the non-presence of
__GFP_NOWARN in gfp_mask.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
Hello,

This patch aims at removing INFO messages that are displayed when the
VC4 driver tries to allocate buffer objects. From the driver perspective
an allocation failure is acceptable, and the driver can possibly do
something to make following allocation succeed (like flushing the VC4
internal cache).

Also, I don't understand why this message is only an INFO message, and
not a WARN (pr_warn()). Please let me know if you have good reasons to
keep it as an unconditional pr_info().

Thanks,

Boris
---
 mm/cma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jaewon Kim Oct. 4, 2017, 4:16 p.m. UTC | #1
Hello

2017-10-04 21:54 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> cma_alloc() unconditionally prints an INFO message when the CMA
> allocation fails. Make this message conditional on the non-presence of
> __GFP_NOWARN in gfp_mask.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Hello,
>
> This patch aims at removing INFO messages that are displayed when the
> VC4 driver tries to allocate buffer objects. From the driver perspective
> an allocation failure is acceptable, and the driver can possibly do
> something to make following allocation succeed (like flushing the VC4
> internal cache).
When I made the patch, there was no GFP.
In my opinion, it is a good idea removing log in case of __GFP_NOWARN.
>
> Also, I don't understand why this message is only an INFO message, and
> not a WARN (pr_warn()). Please let me know if you have good reasons to
> keep it as an unconditional pr_info().
Thank to Michal Hocko, I changed to pr_info rather than just printk in
my first patch code.
https://marc.info/?l=linux-mm&m=148300462103801&w=2
I thought it is just info. It can be pr_warn if need.

Thank you
Jaewon Kim
>
> Thanks,
>
> Boris
> ---
>  mm/cma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/cma.c b/mm/cma.c
> index c0da318c020e..022e52bd8370 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -460,7 +460,7 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
>
>         trace_cma_alloc(pfn, page, count, align);
>
> -       if (ret) {
> +       if (ret && !(gfp_mask & __GFP_NOWARN)) {
>                 pr_info("%s: alloc failed, req-size: %zu pages, ret: %d\n",
>                         __func__, count, ret);
>                 cma_debug_show_areas(cma);
> --
> 2.11.0
>
> --
> 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>
Laura Abbott Oct. 4, 2017, 9:30 p.m. UTC | #2
On 10/04/2017 05:54 AM, Boris Brezillon wrote:
> cma_alloc() unconditionally prints an INFO message when the CMA
> allocation fails. Make this message conditional on the non-presence of
> __GFP_NOWARN in gfp_mask.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Acked-by: Laura Abbott <labbott@redhat.com>

> ---
> Hello,
> 
> This patch aims at removing INFO messages that are displayed when the
> VC4 driver tries to allocate buffer objects. From the driver perspective
> an allocation failure is acceptable, and the driver can possibly do
> something to make following allocation succeed (like flushing the VC4
> internal cache).
> 
> Also, I don't understand why this message is only an INFO message, and
> not a WARN (pr_warn()). Please let me know if you have good reasons to
> keep it as an unconditional pr_info().
> 
> Thanks,
> 
> Boris
> ---
>  mm/cma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/cma.c b/mm/cma.c
> index c0da318c020e..022e52bd8370 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -460,7 +460,7 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
>  
>  	trace_cma_alloc(pfn, page, count, align);
>  
> -	if (ret) {
> +	if (ret && !(gfp_mask & __GFP_NOWARN)) {
>  		pr_info("%s: alloc failed, req-size: %zu pages, ret: %d\n",
>  			__func__, count, ret);
>  		cma_debug_show_areas(cma);
>
Anshuman Khandual Oct. 5, 2017, 2:55 a.m. UTC | #3
On 10/04/2017 06:24 PM, Boris Brezillon wrote:
> cma_alloc() unconditionally prints an INFO message when the CMA
> allocation fails. Make this message conditional on the non-presence of
> __GFP_NOWARN in gfp_mask.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Hello,
> 
> This patch aims at removing INFO messages that are displayed when the
> VC4 driver tries to allocate buffer objects. From the driver perspective
> an allocation failure is acceptable, and the driver can possibly do
> something to make following allocation succeed (like flushing the VC4
> internal cache).
> 
> Also, I don't understand why this message is only an INFO message, and
> not a WARN (pr_warn()). Please let me know if you have good reasons to
> keep it as an unconditional pr_info()

Making it conditional (__GFP_NOWARN based what you already have) with
pr_warn() message makes more sense.
diff mbox

Patch

diff --git a/mm/cma.c b/mm/cma.c
index c0da318c020e..022e52bd8370 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -460,7 +460,7 @@  struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
 
 	trace_cma_alloc(pfn, page, count, align);
 
-	if (ret) {
+	if (ret && !(gfp_mask & __GFP_NOWARN)) {
 		pr_info("%s: alloc failed, req-size: %zu pages, ret: %d\n",
 			__func__, count, ret);
 		cma_debug_show_areas(cma);