2020-11-07 10:07:19

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH] dma-pool: no need to check return value of debugfs_create functions

When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.

Signed-off-by: Tiezhu Yang <[email protected]>
---
kernel/dma/pool.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index d4637f7..5f84e6c 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -38,9 +38,6 @@ static void __init dma_atomic_pool_debugfs_init(void)
struct dentry *root;

root = debugfs_create_dir("dma_pools", NULL);
- if (IS_ERR_OR_NULL(root))
- return;
-
debugfs_create_ulong("pool_size_dma", 0400, root, &pool_size_dma);
debugfs_create_ulong("pool_size_dma32", 0400, root, &pool_size_dma32);
debugfs_create_ulong("pool_size_kernel", 0400, root, &pool_size_kernel);
--
2.1.0


2020-11-09 11:47:16

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] dma-pool: no need to check return value of debugfs_create functions

On 2020-11-07 10:03, Tiezhu Yang wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.

Well, the only difference in behaviour is that it won't attempt to call
further debugfs functions if they're definitely going to fail anyway, so
no "real" logic is affected. AFAICS it's not possible for
debugfs_create_dir() to return NULL, so this check makes no practical
difference, just means that if it did ever fail we would save a bit of
unnecessary work by not subsequently calling all the way down to the "if
(IS_ERR(parent))" check in start_creating() several times.

So the given justification is a little overdramatic for this particular
situation, when it's really just that it's not worth optimising an
unexpected failure case, but the change itself is obviously fine.

Reviewed-by: Robin Murphy <[email protected]>

> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> kernel/dma/pool.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index d4637f7..5f84e6c 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -38,9 +38,6 @@ static void __init dma_atomic_pool_debugfs_init(void)
> struct dentry *root;
>
> root = debugfs_create_dir("dma_pools", NULL);
> - if (IS_ERR_OR_NULL(root))
> - return;
> -
> debugfs_create_ulong("pool_size_dma", 0400, root, &pool_size_dma);
> debugfs_create_ulong("pool_size_dma32", 0400, root, &pool_size_dma32);
> debugfs_create_ulong("pool_size_kernel", 0400, root, &pool_size_kernel);
>