[PATCH] [v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

[Dinghao Liu]

If of_clk_add_provider() fails in ca8210_register_ext_clock(),
it calls clk_unregister() to release priv->clk and returns an
error. However, the caller ca8210_probe() then calls ca8210_remove(),
where priv->clk is freed again in ca8210_unregister_ext_clock(). In
this case, a use-after-free may happen in the second time we call
clk_unregister().

Fix this by removing the first clk_unregister(). Also, priv->clk could
be an error code on failure of clk_register_fixed_rate(). Use
IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Dinghao Liu 
---

Changelog:

v2: -Remove the first clk_unregister() instead of nulling priv->clk.

v3: -Simplify ca8210_register_ext_clock().
-Add a ';' after return in ca8210_unregister_ext_clock().
---
 drivers/net/ieee802154/ca8210.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index aebb19f1b3a4..ae44a9133937 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2757,18 +2757,8 @@ static int ca8210_register_ext_clock(struct spi_device 
*spi)
dev_crit(&spi->dev, "Failed to register external clk\n");
return PTR_ERR(priv->clk);
}
-   ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
-   if (ret) {
-   clk_unregister(priv->clk);
-   dev_crit(
-   &spi->dev,
-   "Failed to register external clock as clock provider\n"
-   );
-   } else {
-   dev_info(&spi->dev, "External clock set as clock provider\n");
-   }
 
-   return ret;
+   return of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
 }
 
 /**
@@ -2780,8 +2770,8 @@ static void ca8210_unregister_ext_clock(struct spi_device 
*spi)
 {
struct ca8210_priv *priv = spi_get_drvdata(spi);
 
-   if (!priv->clk)
-   return
+   if (IS_ERR_OR_NULL(priv->clk))
+   return;
 
of_clk_del_provider(spi->dev.of_node);
clk_unregister(priv->clk);
-- 
2.17.1

Re: [PATCH] [v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

[dinghao . liu]

> Hello Dinghao,
> 
> 
> On 01.10.23 09:19, kernel test robot wrote:
> > Hi Dinghao,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v6.6-rc3 next-20230929]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:
> > https://github.com/intel-lab-lkp/linux/commits/Dinghao-Liu/ieee802154-ca8210-Fix-a-potential-UAF-in-ca8210_probe/20231001-135130
> > base:   linus/master
> > patch link:
> > https://lore.kernel.org/r/20231001054949.14624-1-dinghao.liu%40zju.edu.cn
> > patch subject: [PATCH] [v3] ieee802154: ca8210: Fix a potential UAF in 
> > ca8210_probe
> > config: m68k-allyesconfig 
> > (https://download.01.org/0day-ci/archive/20231001/202310011548.qyqmuodi-...@intel.com/config)
> > compiler: m68k-linux-gcc (GCC) 13.2.0
> > reproduce (this is a W=1 build): 
> > (https://download.01.org/0day-ci/archive/20231001/202310011548.qyqmuodi-...@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new 
> > version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot 
> > | Closes: 
> > https://lore.kernel.org/oe-kbuild-all/202310011548.qyqmuodi-...@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> > drivers/net/ieee802154/ca8210.c: In function 
> > 'ca8210_register_ext_clock':
> >>> drivers/net/ieee802154/ca8210.c:2743:13: warning: unused variable 'ret' 
> >>> [-Wunused-variable]
> >  2743 | int ret = 0;
> >   | ^~~
> > 
> 
> Please take care of this now unused variable after your re-factor.
> With this fixed and send out as v4 I am happy to get this applied to the 
> wpan tree.

I will resend the v4 patch soon, thanks!

Regards,
Dinghao

[PATCH] [v4] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

[Dinghao Liu]

If of_clk_add_provider() fails in ca8210_register_ext_clock(),
it calls clk_unregister() to release priv->clk and returns an
error. However, the caller ca8210_probe() then calls ca8210_remove(),
where priv->clk is freed again in ca8210_unregister_ext_clock(). In
this case, a use-after-free may happen in the second time we call
clk_unregister().

Fix this by removing the first clk_unregister(). Also, priv->clk could
be an error code on failure of clk_register_fixed_rate(). Use
IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Dinghao Liu 
---

Changelog:

v2: -Remove the first clk_unregister() instead of nulling priv->clk.

v3: -Simplify ca8210_register_ext_clock().
-Add a ';' after return in ca8210_unregister_ext_clock().

v4: -Remove an unused variable 'ret'.
---
 drivers/net/ieee802154/ca8210.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index aebb19f1b3a4..4ec0dab38872 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2740,7 +2740,6 @@ static int ca8210_register_ext_clock(struct spi_device 
*spi)
struct device_node *np = spi->dev.of_node;
struct ca8210_priv *priv = spi_get_drvdata(spi);
struct ca8210_platform_data *pdata = spi->dev.platform_data;
-   int ret = 0;
 
if (!np)
return -EFAULT;
@@ -2757,18 +2756,8 @@ static int ca8210_register_ext_clock(struct spi_device 
*spi)
dev_crit(&spi->dev, "Failed to register external clk\n");
return PTR_ERR(priv->clk);
}
-   ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
-   if (ret) {
-   clk_unregister(priv->clk);
-   dev_crit(
-   &spi->dev,
-   "Failed to register external clock as clock provider\n"
-   );
-   } else {
-   dev_info(&spi->dev, "External clock set as clock provider\n");
-   }
 
-   return ret;
+   return of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
 }
 
 /**
@@ -2780,8 +2769,8 @@ static void ca8210_unregister_ext_clock(struct spi_device 
*spi)
 {
struct ca8210_priv *priv = spi_get_drvdata(spi);
 
-   if (!priv->clk)
-   return
+   if (IS_ERR_OR_NULL(priv->clk))
+   return;
 
of_clk_del_provider(spi->dev.of_node);
clk_unregister(priv->clk);
-- 
2.17.1

[PATCH] nvdimm-btt: fix a potential memleak in btt_freelist_init

[Dinghao Liu]

When an error happens in btt_freelist_init(), its caller
discover_arenas() will directly free arena, which makes
arena->freelist allocated in btt_freelist_init() a leaked
memory. Fix this by freeing arena->freelist in all error
handling paths of btt_freelist_init().

Fixes: 5212e11fde4d ("nd_btt: atomic sector updates")
Signed-off-by: Dinghao Liu 
---
 drivers/nvdimm/btt.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index d5593b0dc700..d8c4ba8bfdda 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -544,8 +544,10 @@ static int btt_freelist_init(struct arena_info *arena)
 
for (i = 0; i < arena->nfree; i++) {
new = btt_log_read(arena, i, &log_new, LOG_NEW_ENT);
-   if (new < 0)
-   return new;
+   if (new < 0) {
+   ret = new;
+   goto out_free;
+   }
 
/* old and new map entries with any flags stripped out */
log_oldmap = ent_lba(le32_to_cpu(log_new.old_map));
@@ -577,7 +579,7 @@ static int btt_freelist_init(struct arena_info *arena)
ret = btt_map_read(arena, le32_to_cpu(log_new.lba), &map_entry,
NULL, NULL, 0);
if (ret)
-   return ret;
+   goto out_free;
 
/*
 * The map_entry from btt_read_map is stripped of any flag bits,
@@ -594,11 +596,16 @@ static int btt_freelist_init(struct arena_info *arena)
ret = btt_map_write(arena, le32_to_cpu(log_new.lba),
le32_to_cpu(log_new.new_map), 0, 0, 0);
if (ret)
-   return ret;
+   goto out_free;
}
}
 
return 0;
+
+out_free:
+   kfree(arena->freelist);
+   arena->freelist = NULL;
+   return ret;
 }
 
 static bool ent_is_padding(struct log_entry *ent)
-- 
2.17.1

Re: [PATCH] nvdimm-btt: fix a potential memleak in btt_freelist_init

[dinghao . liu]

> Dave Jiang wrote:
> > 
> 
> [snip]
> 
> First off thanks for the patch.  This code seems to have a few things to
> clean up.
> 
> > 
> > On 12/6/23 20:43, Dinghao Liu wrote:
> > > When an error happens in btt_freelist_init(), its caller
> > > discover_arenas() will directly free arena, which makes
> > > arena->freelist allocated in btt_freelist_init() a leaked
> > > memory. Fix this by freeing arena->freelist in all error
> > > handling paths of btt_freelist_init().
> > > 
> > > Fixes: 5212e11fde4d ("nd_btt: atomic sector updates")
> > > Signed-off-by: Dinghao Liu 
> > 
> > How about use the new scope based resource management and we can avoid the 
> > goto mess altogether?
> > https://lwn.net/Articles/934679/
> > 
> 
> The freelist is returned as part of arena.  I've not traced both paths of
> btt_freelist_init() completely but devm_kcalloc() looks like a better
> solution here because this memory needs to live past the function scope.
> 
> That said, this patch does not completely fix freelist from leaking in the
> following error path.
> 
>   discover_arenas()
>   btt_freelist_init() -> ok (memory allocated)
>   btt_rtt_init() -> fail
>   goto out;
>   (leak because arena is not yet on btt->arena_list)
>   OR
>   btt_maplocks_init() -> fail
>   goto out;
>   (leak because arena is not yet on btt->arena_list)
> 

Thanks for pointing out this issue! I rechecked discover_arenas() and found
that btt_rtt_init() may also trigger a memleak for the same reason as
btt_freelist_init(). Also, I checked another call trace:

btt_init() -> btt_meta_init() -> btt_maplocks_init()

I think there is a memleak if btt_maplocks_init() succeeds but an error
happens in btt_init() after btt_meta_init() (e.g., when btt_blk_init()
returns an error). Therefore, we may need to fix three functions.

> This error could be fixed by adding to arena_list earlier but devm_*()
> also takes care of this without having to worry about that logic.
> 
> On normal operation all of this memory can be free'ed with the
> corresponding devm_kfree() and/or devm_add_action_*() calls if arenas come
> and go.  I'm not sure off the top of my head.
> 
> In addition, looking at this code.  discover_arenas() could make use of
> the scoped based management for struct btt_sb *super!
> 
> Dinghao would you be willing to submit a series of 2 or 3 patches to fix
> the above issues?
> 

Sure. Currently I plan to send 2 patches as follows:
1. Using devm_kcalloc() to replace kcalloc() in btt_freelist_init(), 
   btt_rtt_init(), and btt_maplocks_init(), and removing the corresponding
   kfree in free_arenas(). I checked some uses of devm_kcalloc() and it
   seems that we need not to call devm_kfree(). The memory is automatically
   freed on driver detach, right?
2. Using the scoped based management for struct btt_sb *super (not a bug,
   but it could improve the code).

I'm not quite sure whether my understanding or bug fixing plan is correct.
If there are any issues, please correct me, thanks!

Regards,
Dinghao

Re: [PATCH] nvdimm-btt: fix a potential memleak in btt_freelist_init

[dinghao . liu]

> dinghao.liu@ wrote:
> > > Dave Jiang wrote:
> 
> [snip]
> 
> > > That said, this patch does not completely fix freelist from leaking in the
> > > following error path.
> > > 
> > >   discover_arenas()
> > >   btt_freelist_init() -> ok (memory allocated)
> > >   btt_rtt_init() -> fail
> > >   goto out;
> > >   (leak because arena is not yet on btt->arena_list)
> > >   OR
> > >   btt_maplocks_init() -> fail
> > >   goto out;
> > >   (leak because arena is not yet on btt->arena_list)
> > > 
> > 
> > Thanks for pointing out this issue! I rechecked discover_arenas() and found
> > that btt_rtt_init() may also trigger a memleak for the same reason as
> > btt_freelist_init(). Also, I checked another call trace:
> > 
> > btt_init() -> btt_meta_init() -> btt_maplocks_init()
> > 
> > I think there is a memleak if btt_maplocks_init() succeeds but an error
> > happens in btt_init() after btt_meta_init() (e.g., when btt_blk_init()
> > returns an error). Therefore, we may need to fix three functions.
> 
> Yea I think we need to trace this code better.  This is why devm_ is nice for
> memory allocated for the life of the device.
> 
> > 
> > > This error could be fixed by adding to arena_list earlier but devm_*()
> > > also takes care of this without having to worry about that logic.
> > > 
> > > On normal operation all of this memory can be free'ed with the
> > > corresponding devm_kfree() and/or devm_add_action_*() calls if arenas come
> > > and go.  I'm not sure off the top of my head.
> > > 
> > > In addition, looking at this code.  discover_arenas() could make use of
> > > the scoped based management for struct btt_sb *super!
> > > 
> > > Dinghao would you be willing to submit a series of 2 or 3 patches to fix
> > > the above issues?
> > > 
> > 
> > Sure. Currently I plan to send 2 patches as follows:
> > 1. Using devm_kcalloc() to replace kcalloc() in btt_freelist_init(), 
> >btt_rtt_init(), and btt_maplocks_init(), and removing the corresponding
> >kfree in free_arenas(). I checked some uses of devm_kcalloc() and it
> >seems that we need not to call devm_kfree(). The memory is automatically
> >freed on driver detach, right?
> 
> On device put yes.  So if these allocations are scoped to the life of the
> device there would be no reason to call devm_kfree() on them at all.  I was 
> not
> sure if they got reallocated at some point or not.
> 
> > 2. Using the scoped based management for struct btt_sb *super (not a bug,
> >but it could improve the code).
> 
> Good!
> 
> > 
> > I'm not quite sure whether my understanding or bug fixing plan is correct.
> > If there are any issues, please correct me, thanks!
> 
> The above sounds right.
> Ira

Thanks for the review! I will send the patches soon.

Regards,
Dinghao

[PATCH] [v2] nvdimm-btt: fix several memleaks

[Dinghao Liu]

Resources allocated by kcalloc() in btt_freelist_init(),
btt_rtt_init(), and btt_maplocks_init() are not correctly
released in their callers when an error happens. For
example, when an error happens in btt_freelist_init(), its
caller discover_arenas() will directly free arena, which makes
arena->freelist a leaked memory. Fix these memleaks by using
devm_kcalloc() to make the memory auto-freed on driver detach.

Fixes: 5212e11fde4d ("nd_btt: atomic sector updates")
Signed-off-by: Dinghao Liu 
---

Changelog:

v2: -Use devm_kcalloc() to fix the memleaks.
-Fix the potential leaked memory in btt_rtt_init()
 and btt_maplocks_init().
---
 drivers/nvdimm/btt.c | 35 ---
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index d5593b0dc700..c55231f42617 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -531,13 +531,13 @@ static int arena_clear_freelist_error(struct arena_info 
*arena, u32 lane)
return ret;
 }
 
-static int btt_freelist_init(struct arena_info *arena)
+static int btt_freelist_init(struct device *dev, struct arena_info *arena)
 {
int new, ret;
struct log_entry log_new;
u32 i, map_entry, log_oldmap, log_newmap;
 
-   arena->freelist = kcalloc(arena->nfree, sizeof(struct free_entry),
+   arena->freelist = devm_kcalloc(dev, arena->nfree, sizeof(struct 
free_entry),
GFP_KERNEL);
if (!arena->freelist)
return -ENOMEM;
@@ -718,20 +718,20 @@ static int log_set_indices(struct arena_info *arena)
return 0;
 }
 
-static int btt_rtt_init(struct arena_info *arena)
+static int btt_rtt_init(struct device *dev, struct arena_info *arena)
 {
-   arena->rtt = kcalloc(arena->nfree, sizeof(u32), GFP_KERNEL);
+   arena->rtt = devm_kcalloc(dev, arena->nfree, sizeof(u32), GFP_KERNEL);
if (arena->rtt == NULL)
return -ENOMEM;
 
return 0;
 }
 
-static int btt_maplocks_init(struct arena_info *arena)
+static int btt_maplocks_init(struct device *dev, struct arena_info *arena)
 {
u32 i;
 
-   arena->map_locks = kcalloc(arena->nfree, sizeof(struct aligned_lock),
+   arena->map_locks = devm_kcalloc(dev, arena->nfree, sizeof(struct 
aligned_lock),
GFP_KERNEL);
if (!arena->map_locks)
return -ENOMEM;
@@ -805,9 +805,6 @@ static void free_arenas(struct btt *btt)
 
list_for_each_entry_safe(arena, next, &btt->arena_list, list) {
list_del(&arena->list);
-   kfree(arena->rtt);
-   kfree(arena->map_locks);
-   kfree(arena->freelist);
debugfs_remove_recursive(arena->debugfs_dir);
kfree(arena);
}
@@ -843,7 +840,7 @@ static void parse_arena_meta(struct arena_info *arena, 
struct btt_sb *super,
arena->flags = le32_to_cpu(super->flags);
 }
 
-static int discover_arenas(struct btt *btt)
+static int discover_arenas(struct device *dev, struct btt *btt)
 {
int ret = 0;
struct arena_info *arena;
@@ -893,15 +890,15 @@ static int discover_arenas(struct btt *btt)
goto out;
}
 
-   ret = btt_freelist_init(arena);
+   ret = btt_freelist_init(dev, arena);
if (ret)
goto out;
 
-   ret = btt_rtt_init(arena);
+   ret = btt_rtt_init(dev, arena);
if (ret)
goto out;
 
-   ret = btt_maplocks_init(arena);
+   ret = btt_maplocks_init(dev, arena);
if (ret)
goto out;
 
@@ -1022,7 +1019,7 @@ static int btt_arena_write_layout(struct arena_info 
*arena)
  * This function completes the initialization for the BTT namespace
  * such that it is ready to accept IOs
  */
-static int btt_meta_init(struct btt *btt)
+static int btt_meta_init(struct device *dev, struct btt *btt)
 {
int ret = 0;
struct arena_info *arena;
@@ -1033,15 +1030,15 @@ static int btt_meta_init(struct btt *btt)
if (ret)
goto unlock;
 
-   ret = btt_freelist_init(arena);
+   ret = btt_freelist_init(dev, arena);
if (ret)
goto unlock;
 
-   ret = btt_rtt_init(arena);
+   ret = btt_rtt_init(dev, arena);
if (ret)
goto unlock;
 
-   ret = btt_maplocks_init(arena);
+   ret = btt_maplocks_init(dev, arena);
if (ret)
goto unlock;
}
@@ -1584,7 +1581,7 @@ static struct btt *btt_init(struct nd_btt *nd_btt, 
unsigned long long rawsize,
nsio = to_nd_namespace_io(&nd_btt->ndns->dev);
 

[PATCH] nvdimm-btt: simplify code with the scope based resource management

[Dinghao Liu]

Use the scope based resource management (defined in
linux/cleanup.h) to automate resource lifetime
control on struct btt_sb *super in discover_arenas().

Signed-off-by: Dinghao Liu 
---
 drivers/nvdimm/btt.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index d5593b0dc700..ff42778b51de 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "btt.h"
 #include "nd.h"
 
@@ -847,7 +848,7 @@ static int discover_arenas(struct btt *btt)
 {
int ret = 0;
struct arena_info *arena;
-   struct btt_sb *super;
+   struct btt_sb *super __free(kfree) = NULL;
size_t remaining = btt->rawsize;
u64 cur_nlba = 0;
size_t cur_off = 0;
@@ -860,10 +861,8 @@ static int discover_arenas(struct btt *btt)
while (remaining) {
/* Alloc memory for arena */
arena = alloc_arena(btt, 0, 0, 0);
-   if (!arena) {
-   ret = -ENOMEM;
-   goto out_super;
-   }
+   if (!arena)
+   return -ENOMEM;
 
arena->infooff = cur_off;
ret = btt_info_read(arena, super);
@@ -919,14 +918,11 @@ static int discover_arenas(struct btt *btt)
btt->nlba = cur_nlba;
btt->init_state = INIT_READY;
 
-   kfree(super);
return ret;
 
  out:
kfree(arena);
free_arenas(btt);
- out_super:
-   kfree(super);
return ret;
 }
 
-- 
2.17.1

Re: [PATCH] nvdimm-btt: simplify code with the scope based resource management

[dinghao . liu]

> 
> On 12/10/23 03:27, Dinghao Liu wrote:
> > Use the scope based resource management (defined in
> > linux/cleanup.h) to automate resource lifetime
> > control on struct btt_sb *super in discover_arenas().
> > 
> > Signed-off-by: Dinghao Liu 
> > ---
> >  drivers/nvdimm/btt.c | 12 
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> > index d5593b0dc700..ff42778b51de 100644
> > --- a/drivers/nvdimm/btt.c
> > +++ b/drivers/nvdimm/btt.c
> > @@ -16,6 +16,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include "btt.h"
> >  #include "nd.h"
> >  
> > @@ -847,7 +848,7 @@ static int discover_arenas(struct btt *btt)
> >  {
> > int ret = 0;
> > struct arena_info *arena;
> > -   struct btt_sb *super;
> > +   struct btt_sb *super __free(kfree) = NULL;
> > size_t remaining = btt->rawsize;
> > u64 cur_nlba = 0;
> > size_t cur_off = 0;
> > @@ -860,10 +861,8 @@ static int discover_arenas(struct btt *btt)
> > while (remaining) {
> > /* Alloc memory for arena */
> > arena = alloc_arena(btt, 0, 0, 0);
> > -   if (!arena) {
> > -   ret = -ENOMEM;
> > -   goto out_super;
> > -   }
> > +   if (!arena)
> > +   return -ENOMEM;
> >  
> > arena->infooff = cur_off;
> > ret = btt_info_read(arena, super);
> > @@ -919,14 +918,11 @@ static int discover_arenas(struct btt *btt)
> > btt->nlba = cur_nlba;
> > btt->init_state = INIT_READY;
> >  
> > -   kfree(super);
> > return ret;
> >  
> >   out:
> > kfree(arena);
> > free_arenas(btt);
> > - out_super:
> > -   kfree(super);
> > return ret;
> >  }
> >  
> 
> I would do the allocation like something below for the first chunk. Otherwise 
> the rest LGTM. 
> 
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index d5593b0dc700..143921e7f26c 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "btt.h"
>  #include "nd.h"
>  
> @@ -845,25 +846,23 @@ static void parse_arena_meta(struct arena_info *arena, 
> struct btt_sb *super,
>  
>  static int discover_arenas(struct btt *btt)
>  {
> +   struct btt_sb *super __free(kfree) =
> +   kzalloc(sizeof(*super), GFP_KERNEL);
> int ret = 0;
> struct arena_info *arena;
> -   struct btt_sb *super;
> size_t remaining = btt->rawsize;
> u64 cur_nlba = 0;
> size_t cur_off = 0;
> int num_arenas = 0;
>  
> -   super = kzalloc(sizeof(*super), GFP_KERNEL);
> if (!super)
> return -ENOMEM;
>  
> while (remaining) {
> /* Alloc memory for arena */

It's a little strange that we do not check super immediately after allocation.
How about this:

 static int discover_arenas(struct btt *btt)
 {
int ret = 0;
struct arena_info *arena;
-   struct btt_sb *super;
size_t remaining = btt->rawsize;
u64 cur_nlba = 0;
size_t cur_off = 0;
int num_arenas = 0;
 
-   super = kzalloc(sizeof(*super), GFP_KERNEL);
+   struct btt_sb *super __free(kfree) = 
+   kzalloc(sizeof(*super), GFP_KERNEL);
if (!super)
return -ENOMEM;
 
while (remaining) {
 

Re: [PATCH] nvdimm-btt: simplify code with the scope based resource management

[dinghao . liu]

> > It's a little strange that we do not check super immediately after 
> > allocation.
> > How about this:
> > 
> >  static int discover_arenas(struct btt *btt)
> >  {
> > int ret = 0;
> > struct arena_info *arena;
> > -   struct btt_sb *super;
> > size_t remaining = btt->rawsize;
> > u64 cur_nlba = 0;
> > size_t cur_off = 0;
> > int num_arenas = 0;
> >  
> > -   super = kzalloc(sizeof(*super), GFP_KERNEL);
> > +   struct btt_sb *super __free(kfree) = 
> > +   kzalloc(sizeof(*super), GFP_KERNEL);
> > if (!super)
> > return -ENOMEM;
> >  
> > while (remaining) {
> >  
> 
> That's fine by me

I will resend a new patch soon, thanks!

Regards,
Dinghao

[PATCH] [v2] nvdimm-btt: simplify code with the scope based resource management

[Dinghao Liu]

Use the scope based resource management (defined in
linux/cleanup.h) to automate resource lifetime
control on struct btt_sb *super in discover_arenas().

Signed-off-by: Dinghao Liu 
---

Changelog:

v2: Set the __free attribute before kzalloc.
---
 drivers/nvdimm/btt.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index d5593b0dc700..32a9e2f543c5 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "btt.h"
 #include "nd.h"
 
@@ -847,23 +848,20 @@ static int discover_arenas(struct btt *btt)
 {
int ret = 0;
struct arena_info *arena;
-   struct btt_sb *super;
size_t remaining = btt->rawsize;
u64 cur_nlba = 0;
size_t cur_off = 0;
int num_arenas = 0;
 
-   super = kzalloc(sizeof(*super), GFP_KERNEL);
+   struct btt_sb *super __free(kfree) = kzalloc(sizeof(*super), 
GFP_KERNEL);
if (!super)
return -ENOMEM;
 
while (remaining) {
/* Alloc memory for arena */
arena = alloc_arena(btt, 0, 0, 0);
-   if (!arena) {
-   ret = -ENOMEM;
-   goto out_super;
-   }
+   if (!arena)
+   return -ENOMEM;
 
arena->infooff = cur_off;
ret = btt_info_read(arena, super);
@@ -919,14 +917,11 @@ static int discover_arenas(struct btt *btt)
btt->nlba = cur_nlba;
btt->init_state = INIT_READY;
 
-   kfree(super);
return ret;
 
  out:
kfree(arena);
free_arenas(btt);
- out_super:
-   kfree(super);
return ret;
 }
 
-- 
2.17.1

Re: [PATCH] [v2] nvdimm-btt: fix several memleaks

[dinghao . liu]

> Ira Weiny wrote:
> > Dinghao Liu wrote:
> 
> [snip]
> 
> -static int btt_freelist_init(struct arena_info *arena)
> +static int btt_freelist_init(struct device *dev, struct arena_info *arena)
> 
> Both struct arena_info and struct btt contain references to struct nd_btt
> which is the device you are passing down this call stack.
> 
> Just use the device in the arena/btt rather than passing a device
> structure.  That makes the code easier to read and ensures that the device
> associated with this arena or btt is used.

Thanks for this suggestion! I will fix this in the v3 patch.

> [snip]
> > >  
> > > -static int btt_maplocks_init(struct arena_info *arena)
> > > +static int btt_maplocks_init(struct device *dev, struct arena_info 
> > > *arena)
> > >  {
> > >   u32 i;
> > >  
> > > - arena->map_locks = kcalloc(arena->nfree, sizeof(struct aligned_lock),
> > > + arena->map_locks = devm_kcalloc(dev, arena->nfree, sizeof(struct 
> > > aligned_lock),
> > >   GFP_KERNEL);
> > >   if (!arena->map_locks)
> > >   return -ENOMEM;
> > > @@ -805,9 +805,6 @@ static void free_arenas(struct btt *btt)
> > >  
> > >   list_for_each_entry_safe(arena, next, &btt->arena_list, list) {
> > >   list_del(&arena->list);
> > > - kfree(arena->rtt);
> > > - kfree(arena->map_locks);
> > > - kfree(arena->freelist);
> > 
> > This does not quite work.
> > 
> > free_arenas() is used in the error paths of create_arenas() and
> > discover_arenas().  In those cases devm_kfree() is probably a better way
> > to clean up this.

Here I'm a little confused about when devm_kfree() should be used.
Code in btt_init() implies that resources allocated by devm_* could be
auto freed in both error and success paths of probe/attach (e.g., btt 
allocated by devm_kzalloc is never freed by devm_kfree).
Using devm_kfree() in free_arenas() is ok for me, but I want to know
whether not using devm_kfree() constitutes a bug.

> > 
> > However...
> > 
> > >   debugfs_remove_recursive(arena->debugfs_dir);
> > >   kfree(arena);
> > 
> > Why can't arena be allocated with devm_*?
> > 
> > We need to change this up a bit more to handle the error path vs regular
> > device shutdown free (automatic devm frees).
> 

At first, I think the use of arena is correct. Therefore, allocating arena
with devm_* should be a code style optimization. However, I rechecked 
discover_arenas and found arena might also be leaked (e.g., if alloc_arena() 
fails in the second loop, the previously allocated resources in the loop is 
leaked). The correct code could be found in create_arenas(), where free_arenas 
is called on failure of alloc_arena().

To fix this issue, I think it's better to change the 'goto out_super' tag
to 'goto out'. We could also use devm_* for arena to simplify the error path
in discover_arenas(). 

> We might want to look at using no_free_ptr() in this code.  See this
> patch[1] for an example of how to inhibit the cleanup and pass the pointer
> on when the function succeeds.
> 
> [1] 
> https://lore.kernel.org/all/170261791914.1714654.6447680285357545638.st...@dwillia2-xfh.jf.intel.com/
> 
> Ira

Thanks for this example. But it seems that no_free_ptr() is used to handle
the scope based resource management. Changes in this patch does not introduce
this feature. Do I understand this correctly?

Regards,
Dinghao

Re: [PATCH] [v2] nvdimm-btt: fix several memleaks

[dinghao . liu]

> dinghao.liu@ wrote:
> > > Ira Weiny wrote:
> > > > Dinghao Liu wrote:

[snip]

> > > > 
> > > > This does not quite work.
> > > > 
> > > > free_arenas() is used in the error paths of create_arenas() and
> > > > discover_arenas().  In those cases devm_kfree() is probably a better way
> > > > to clean up this.
> > 
> > Here I'm a little confused about when devm_kfree() should be used.
> 
> Over all it should be used whenever memory is allocated for the lifetime
> of the device.
> 
> > Code in btt_init() implies that resources allocated by devm_* could be
> > auto freed in both error and success paths of probe/attach (e.g., btt 
> > allocated by devm_kzalloc is never freed by devm_kfree).
> > Using devm_kfree() in free_arenas() is ok for me, but I want to know
> > whether not using devm_kfree() constitutes a bug.
> 
> Unfortunately I'm not familiar enough with this code to know for sure.
> 
> After my quick checks before I thought it was.  But each time I look at it
> I get confused.  This is why I was thinking maybe not using devm_*() and
> using no_free_ptr() may be a better solution to make sure things don't
> leak without changing the success path (which is likely working fine
> because no bugs have been found.)

We have the same confusion here... I find a discussion about this problem,
which implies that not using devm_kfree() may delay the release, but the memory 
will be freed later and no memory is leaked:

https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2009561.html

> > 
> > > We might want to look at using no_free_ptr() in this code.  See this
> > > patch[1] for an example of how to inhibit the cleanup and pass the
> > > pointer on when the function succeeds.
> > > 
> > > [1]
> > > https://lore.kernel.org/all/170261791914.1714654.6447680285357545638.st...@dwillia2-xfh.jf.intel.com/
> > > 
> > > Ira
> > 
> > Thanks for this example. But it seems that no_free_ptr() is used to
> > handle the scope based resource management. Changes in this patch does
> > not introduce this feature. Do I understand this correctly?
> 
> You do understand but I was thinking that perhaps using no_free_ptr()
> rather than devm_*() might be an easier way to fix this bug without trying
> to decode the lifetime of everything.
> 

My concern is that no_free_ptr() may not be able to completely fix all
memleaks because some of them are triggered in (part of) success paths (e.g.,
when btt_freelist_init succeeds but btt_rtt_init fails, discover_arenas still 
needs to clean up the memory allocated in btt_freelist_init).

I checked the design of no_free_ptr(), and it seems that it will generate
a new pointer on success and the memory still leaks in the above case. 
Therefore, I think using devm_*() is still the best solution for this bug. 

Regards,
Dinghao

[PATCH] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

[Dinghao Liu]

If of_clk_add_provider() fails in ca8210_register_ext_clock(),
it calls clk_unregister() to release priv->clk and returns an
error. However, the caller ca8210_probe() then calls ca8210_remove(),
where priv->clk is freed again in ca8210_unregister_ext_clock(). In
this case, a use-after-free may happen in the second time we call
clk_unregister().

Fix this by nulling priv->clk after the first clk_unregister(). Also
refine the pointer checking in ca8210_unregister_ext_clock().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Dinghao Liu 
---
 drivers/net/ieee802154/ca8210.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index aebb19f1b3a4..1d545879c000 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2760,6 +2760,7 @@ static int ca8210_register_ext_clock(struct spi_device 
*spi)
ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
if (ret) {
clk_unregister(priv->clk);
+   priv->clk = NULL;
dev_crit(
&spi->dev,
"Failed to register external clock as clock provider\n"
@@ -2780,7 +2781,7 @@ static void ca8210_unregister_ext_clock(struct spi_device 
*spi)
 {
struct ca8210_priv *priv = spi_get_drvdata(spi);
 
-   if (!priv->clk)
+   if (IS_ERR_OR_NULL(priv->clk))
return
 
of_clk_del_provider(spi->dev.of_node);
-- 
2.17.1

Re: [PATCH] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

[dinghao . liu]

Hi Miquèl,

> > index aebb19f1b3a4..1d545879c000 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -2760,6 +2760,7 @@ static int ca8210_register_ext_clock(struct 
> > spi_device *spi)
> > ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
> > if (ret) {
> > clk_unregister(priv->clk);
> > +   priv->clk = NULL;
> 
> This function is a bit convoluted. You could just return the result of
> of_clk_add_provider() (keep the printk's if you want, they don't seem
> very useful) and let ca8210_unregister_ext_clock() do the cleanup.

Thanks for your advice! I will resend a new patch as suggested.

> 
> > dev_crit(
> > &spi->dev,
> > "Failed to register external clock as clock provider\n"
> > @@ -2780,7 +2781,7 @@ static void ca8210_unregister_ext_clock(struct 
> > spi_device *spi)
> >  {
> > struct ca8210_priv *priv = spi_get_drvdata(spi);
> >  
> > -   if (!priv->clk)
> > +   if (IS_ERR_OR_NULL(priv->clk))
> 
> Does not look useful as you are enforcing priv->clock to be valid or
> NULL, it cannot be an error code.

I find that ca8210_register_ext_clock() uses IS_ERR to check priv->clk
after calling clk_register_fixed_rate(). So I think priv->clk could be
a non-null pointer even on failure. And a null pointer check may miss
this case in ca8210_unregister_ext_clock(). 

Regards,
Dinghao

[PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

[Dinghao Liu]

If of_clk_add_provider() fails in ca8210_register_ext_clock(),
it calls clk_unregister() to release priv->clk and returns an
error. However, the caller ca8210_probe() then calls ca8210_remove(),
where priv->clk is freed again in ca8210_unregister_ext_clock(). In
this case, a use-after-free may happen in the second time we call
clk_unregister().

Fix this by removing the first clk_unregister(). Also, priv->clk could
be an error code on failure of clk_register_fixed_rate(). Use
IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Dinghao Liu 
---

Changelog:

v2: -Remove the first clk_unregister() instead of nulling priv->clk.
---
 drivers/net/ieee802154/ca8210.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index aebb19f1b3a4..b35c6f59bd1a 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2759,7 +2759,6 @@ static int ca8210_register_ext_clock(struct spi_device 
*spi)
}
ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
if (ret) {
-   clk_unregister(priv->clk);
dev_crit(
&spi->dev,
"Failed to register external clock as clock provider\n"
@@ -2780,7 +2779,7 @@ static void ca8210_unregister_ext_clock(struct spi_device 
*spi)
 {
struct ca8210_priv *priv = spi_get_drvdata(spi);
 
-   if (!priv->clk)
+   if (IS_ERR_OR_NULL(priv->clk))
return
 
of_clk_del_provider(spi->dev.of_node);
-- 
2.17.1

Re: [PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

[dinghao . liu]

> Missing Cc stable, this needs to be backported.

I will cc stable (sta...@vger.kernel.org) for the next version, thanks!
> > diff --git a/drivers/net/ieee802154/ca8210.c 
> > b/drivers/net/ieee802154/ca8210.c
> > index aebb19f1b3a4..b35c6f59bd1a 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -2759,7 +2759,6 @@ static int ca8210_register_ext_clock(struct 
> > spi_device *spi)
> > }
> > ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
> > if (ret) {
> > -   clk_unregister(priv->clk);
> > dev_crit(
> > &spi->dev,
> > "Failed to register external clock as clock provider\n"
> 
> I was hoping you would simplify this function a bit more.

I understand. In the next patch version, I will just return 
of_clk_add_provider(). 

> 
> > @@ -2780,7 +2779,7 @@ static void ca8210_unregister_ext_clock(struct 
> > spi_device *spi)
> >  {
> > struct ca8210_priv *priv = spi_get_drvdata(spi);
> >  
> > -   if (!priv->clk)
> > +   if (IS_ERR_OR_NULL(priv->clk))
> > return
> >  
> > of_clk_del_provider(spi->dev.of_node);
> 
> Alex, Stefan, who handles wpan and wpan/next this release?
>
 
Is there any problem I need to handle in the next patch?

Regards,
Dinghao

[PATCH] ASoC: Intel: Skylake: Fix missing check in skl_pcm_trigger

[Dinghao Liu]

When cmd == SNDRV_PCM_TRIGGER_STOP, we should also check
the return value of skl_decoupled_trigger() just like what
we have done in case SNDRV_PCM_TRIGGER_PAUSE_RELEASE.

Signed-off-by: Dinghao Liu 
---
 sound/soc/intel/skylake/skl-pcm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/intel/skylake/skl-pcm.c 
b/sound/soc/intel/skylake/skl-pcm.c
index b1ca64d2f7ea..a5b1f333a500 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -516,6 +516,9 @@ static int skl_pcm_trigger(struct snd_pcm_substream 
*substream, int cmd,
return ret;
 
ret = skl_decoupled_trigger(substream, cmd);
+   if (ret < 0)
+   return ret;
+
if ((cmd == SNDRV_PCM_TRIGGER_SUSPEND) && !w->ignore_suspend) {
/* save the dpib and lpib positions */
stream->dpib = readl(bus->remap_addr +
-- 
2.17.1

Re: Re: [PATCH] ASoC: codecs: Fix rumtime PM imbalance in tas2552_probe

[dinghao . liu]

> On Wed, Apr 07, 2021 at 02:54:00PM +0800, Dinghao Liu wrote:
> 
> > -   pm_runtime_set_active(&client->dev);
> > -   pm_runtime_set_autosuspend_delay(&client->dev, 1000);
> > -   pm_runtime_use_autosuspend(&client->dev);
> > -   pm_runtime_enable(&client->dev);
> > -   pm_runtime_mark_last_busy(&client->dev);
> > -   pm_runtime_put_sync_autosuspend(&client->dev);
> > -
> > dev_set_drvdata(&client->dev, data);
> >  
> > ret = devm_snd_soc_register_component(&client->dev,
> > @@ -733,6 +726,13 @@ static int tas2552_probe(struct i2c_client *client,
> > if (ret < 0)
> > dev_err(&client->dev, "Failed to register component: %d\n", 
> > ret);
> >  
> > +   pm_runtime_set_active(&client->dev);
> > +   pm_runtime_set_autosuspend_delay(&client->dev, 1000);
> > +   pm_runtime_use_autosuspend(&client->dev);
> 
> It's not clear to me that just moving the operations after the
> registration is a good fix - once the component is registered we could
> start trying to do runtime PM operations with it which AFAIR won't count
> references and so on properly if runtime PM isn't enabled so if we later
> enable runtime PM we might have the rest of the code in a confused state
> about what's going on.

Thanks for your advice. I checked the use of devm_snd_soc_register_component() 
in the kernel and found sometimes runtime PM is enabled before registration 
and sometimes after registration. To be on the safe side, I will send a new
patch to fix this in error handling path.

Regards,
Dinghao

Re: Re: [PATCH] Input: cyapa - Fix rumtime PM imbalance on error

[dinghao . liu]

> Hi Dinghao,
> 
> On Wed, Apr 07, 2021 at 12:07:38PM +0800, Dinghao Liu wrote:
> > When mutex_lock_interruptible() fails, a pairing PM usage
> > counter decrement is needed to keep the counter balanced.
> 
> Thank you for the patch.
> 
> > 
> > Signed-off-by: Dinghao Liu 
> > ---
> >  drivers/input/mouse/cyapa.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> > index 77cc653edca2..e411ab45a218 100644
> > --- a/drivers/input/mouse/cyapa.c
> > +++ b/drivers/input/mouse/cyapa.c
> > @@ -904,8 +904,10 @@ static ssize_t cyapa_update_rt_suspend_scanrate(struct 
> > device *dev,
> > pm_runtime_get_sync(dev);
> >  
> > error = mutex_lock_interruptible(&cyapa->state_sync_lock);
> > -   if (error)
> > +   if (error) {
> > +   pm_runtime_put_noidle(dev);
> 
> Why "noidle" and not pm_runtime_put_sync_autosuspend() like we do in
> case of normal flow?
> 

pm_runtime_put_noidle() only decrease the refcount, while 
pm_runtime_put_sync_autosuspend() will execute an extra
pm_runtime_autosuspend(). I'm not sure if the autosuspend is necessary
in this error handling path, so I only balance the counter.

Regards,
Dinghao

[PATCH] [v2] ASoC: codecs: Fix runtime PM imbalance in tas2552_probe

[Dinghao Liu]

There is a rumtime PM imbalance between the error handling path
after devm_snd_soc_register_component() and all other error
handling paths. Add a PM runtime increment to balance refcount.

Signed-off-by: Dinghao Liu 
---

Changelog:

v2: - Add a PM runtime increment to fix it instead of moving
  the PM related operations after the registration.
---
 sound/soc/codecs/tas2552.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c
index bd00c35116cd..700baa6314aa 100644
--- a/sound/soc/codecs/tas2552.c
+++ b/sound/soc/codecs/tas2552.c
@@ -730,8 +730,10 @@ static int tas2552_probe(struct i2c_client *client,
ret = devm_snd_soc_register_component(&client->dev,
  &soc_component_dev_tas2552,
  tas2552_dai, ARRAY_SIZE(tas2552_dai));
-   if (ret < 0)
+   if (ret < 0) {
dev_err(&client->dev, "Failed to register component: %d\n", 
ret);
+   pm_runtime_get_noresume(&client->dev);
+   }
 
return ret;
 }
-- 
2.17.1

[PATCH] media: atomisp: Fix error handling in atomisp_open

[Dinghao Liu]

Some error paths in atomisp_open will execute PM runtime
decrement and unregister hmm pool even before we increase
the PM refcount and registration. Fix this by adjusting
jump labels on error.

Signed-off-by: Dinghao Liu 
---
 drivers/staging/media/atomisp/pci/atomisp_fops.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c 
b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 453bb6913550..8f552d6f1f19 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -837,7 +837,7 @@ static int atomisp_open(struct file *file)
ret = pm_runtime_get_sync(vdev->v4l2_dev->dev);
if (ret < 0) {
dev_err(isp->dev, "Failed to power on device\n");
-   goto error;
+   goto pm_error;
}
 
if (dypool_enable) {
@@ -878,9 +878,10 @@ static int atomisp_open(struct file *file)
 
 css_error:
atomisp_css_uninit(isp);
-error:
hmm_pool_unregister(HMM_POOL_TYPE_DYNAMIC);
+pm_error:
pm_runtime_put(vdev->v4l2_dev->dev);
+error:
rt_mutex_unlock(&isp->mutex);
return ret;
 }
-- 
2.17.1

[PATCH] media: ti-vpe: cal: Fix runtime PM imbalance in cal_probe

[Dinghao Liu]

pm_runtime_get_sync() will increase the rumtime PM counter
even it returns an error. Thus a pairing decrement is needed
to prevent refcount leak. Fix this by replacing this API with
pm_runtime_resume_and_get(), which will not change the runtime
PM counter on error.

Signed-off-by: Dinghao Liu 
---
 drivers/media/platform/ti-vpe/cal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/ti-vpe/cal.c 
b/drivers/media/platform/ti-vpe/cal.c
index fa0931788040..ce4e53b9be10 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1010,7 +1010,7 @@ static int cal_probe(struct platform_device *pdev)
 
/* Read the revision and hardware info to verify hardware access. */
pm_runtime_enable(&pdev->dev);
-   ret = pm_runtime_get_sync(&pdev->dev);
+   ret = pm_runtime_resume_and_get(&pdev->dev);
if (ret)
goto error_pm_runtime;
 
-- 
2.17.1

[PATCH] dmaengine: tegra20: Fix runtime PM imbalance in tegra_dma_issue_pending

[Dinghao Liu]

pm_runtime_get_sync() will increase the rumtime PM counter
even it returns an error. Thus a pairing decrement is needed
to prevent refcount leak. Fix this by replacing this API with
pm_runtime_resume_and_get(), which will not change the runtime
PM counter on error.

Signed-off-by: Dinghao Liu 
---
 drivers/dma/tegra20-apb-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 71827d9b0aa1..73178afaf4c2 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -723,7 +723,7 @@ static void tegra_dma_issue_pending(struct dma_chan *dc)
goto end;
}
if (!tdc->busy) {
-   err = pm_runtime_get_sync(tdc->tdma->dev);
+   err = pm_runtime_resume_and_get(tdc->tdma->dev);
if (err < 0) {
dev_err(tdc2dev(tdc), "Failed to enable DMA\n");
goto end;
-- 
2.17.1

[PATCH] slimbus: qcom-ngd-ctrl: Fix runtime PM imbalance in qcom_slim_ngd_enable

[Dinghao Liu]

When slim_register_controller() fails, a pairing PM usage counter
increment is needed to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/slimbus/qcom-ngd-ctrl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index c054e83ab636..99cf2ab3d862 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1268,6 +1268,7 @@ static int qcom_slim_ngd_enable(struct qcom_slim_ngd_ctrl 
*ctrl, bool enable)
ret = slim_register_controller(&ctrl->ctrl);
if (ret) {
dev_err(ctrl->dev, "error adding slim controller\n");
+   pm_runtime_get_noresume(ctrl->dev);
return ret;
}
 
-- 
2.17.1

[PATCH] PCI: rcar: Fix runtime PM imbalance in rcar_pcie_ep_probe

[Dinghao Liu]

pm_runtime_get_sync() will increase the runtime PM counter
even it returns an error. Thus a pairing decrement is needed
to prevent refcount leak. Fix this by replacing this API with
pm_runtime_resume_and_get(), which will not change the runtime
PM counter on error.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/pcie-rcar-ep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-rcar-ep.c 
b/drivers/pci/controller/pcie-rcar-ep.c
index b4a288e24aaf..c91d85b15129 100644
--- a/drivers/pci/controller/pcie-rcar-ep.c
+++ b/drivers/pci/controller/pcie-rcar-ep.c
@@ -492,9 +492,9 @@ static int rcar_pcie_ep_probe(struct platform_device *pdev)
pcie->dev = dev;
 
pm_runtime_enable(dev);
-   err = pm_runtime_get_sync(dev);
+   err = pm_runtime_resume_and_get(dev);
if (err < 0) {
-   dev_err(dev, "pm_runtime_get_sync failed\n");
+   dev_err(dev, "pm_runtime_resume_and_get failed\n");
goto err_pm_disable;
}
 
-- 
2.17.1

[PATCH] PCI: tegra: Fix runtime PM imbalance in pex_ep_event_pex_rst_deassert

[Dinghao Liu]

pm_runtime_get_sync() will increase the runtime PM counter
even it returns an error. Thus a pairing decrement is needed
to prevent refcount leak. Fix this by replacing this API with
pm_runtime_resume_and_get(), which will not change the runtime
PM counter on error.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index 6fa216e52d14..0e94190ca4e8 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1645,7 +1645,7 @@ static void pex_ep_event_pex_rst_deassert(struct 
tegra_pcie_dw *pcie)
if (pcie->ep_state == EP_STATE_ENABLED)
return;
 
-   ret = pm_runtime_get_sync(dev);
+   ret = pm_runtime_resume_and_get(dev);
if (ret < 0) {
dev_err(dev, "Failed to get runtime sync for PCIe dev: %d\n",
ret);
-- 
2.17.1

[PATCH] media: atomisp: Fix runtime PM imbalance in atomisp_pci_probe

[Dinghao Liu]

When hmm_pool_register() fails, a pairing PM usage counter
increment is needed to keep the counter balanced. It's the
same for the following error paths.

Signed-off-by: Dinghao Liu 
---
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c 
b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 0295e2e32d79..02f774ed80c8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1815,6 +1815,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const 
struct pci_device_id *i
hmm_cleanup();
hmm_pool_unregister(HMM_POOL_TYPE_RESERVED);
 hmm_pool_fail:
+   pm_runtime_get_noresume(&pdev->dev);
destroy_workqueue(isp->wdt_work_queue);
 wdt_work_queue_fail:
atomisp_acc_cleanup(isp);
-- 
2.17.1

[PATCH] media: imx: imx7-mipi-csis: Fix runtime PM imbalance in mipi_csis_s_stream

[Dinghao Liu]

When v4l2_subdev_call() fails, a pairing PM usage counter
decrement is needed to keep the counter balanced. It's the
same for the following error paths in case 'enable' is on.

Signed-off-by: Dinghao Liu 
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
b/drivers/staging/media/imx/imx7-mipi-csis.c
index a01a7364b4b9..2a3fff231a40 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -627,21 +627,26 @@ static int mipi_csis_s_stream(struct v4l2_subdev 
*mipi_sd, int enable)
return ret;
}
ret = v4l2_subdev_call(state->src_sd, core, s_power, 1);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(&state->pdev->dev);
return ret;
+   }
}
 
mutex_lock(&state->lock);
if (enable) {
if (state->flags & ST_SUSPENDED) {
ret = -EBUSY;
+   pm_runtime_put_noidle(&state->pdev->dev);
goto unlock;
}
 
mipi_csis_start_stream(state);
ret = v4l2_subdev_call(state->src_sd, video, s_stream, 1);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(&state->pdev->dev);
goto unlock;
+   }
 
mipi_csis_log_counters(state, true);
 
-- 
2.17.1

[PATCH] spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe

[Dinghao Liu]

When platform_get_irq() fails, a pairing PM usage counter
increment is needed to keep the counter balanced. It's the
same for the following error paths.

Signed-off-by: Dinghao Liu 
---
 drivers/spi/spi-zynqmp-gqspi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index c8fa6ee18ae7..95963a2de64a 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -1197,6 +1197,7 @@ static int zynqmp_qspi_probe(struct platform_device *pdev)
return 0;
 
 clk_dis_all:
+   pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_disable(&pdev->dev);
clk_disable_unprepare(xqspi->refclk);
-- 
2.17.1

Re: Re: [PATCH] spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe

[dinghao . liu]

> Hi Dinghao,
> 
> On 4/8/21 6:33 PM, Michal Simek wrote:
> > ++
> >
> > On 4/8/21 11:25 AM, Dinghao Liu wrote:
> >> When platform_get_irq() fails, a pairing PM usage counter
> >> increment is needed to keep the counter balanced. It's the
> >> same for the following error paths.
> >>
> >> Signed-off-by: Dinghao Liu 
> >> ---
> >>   drivers/spi/spi-zynqmp-gqspi.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/spi/spi-zynqmp-gqspi.c 
> >> b/drivers/spi/spi-zynqmp-gqspi.c
> >> index c8fa6ee18ae7..95963a2de64a 100644
> >> --- a/drivers/spi/spi-zynqmp-gqspi.c
> >> +++ b/drivers/spi/spi-zynqmp-gqspi.c
> >> @@ -1197,6 +1197,7 @@ static int zynqmp_qspi_probe(struct platform_device 
> >> *pdev)
> >>return 0;
> >>   
> >>   clk_dis_all:
> >> +  pm_runtime_get_noresume(&pdev->dev);
> >>pm_runtime_set_suspended(&pdev->dev);
> >>pm_runtime_disable(&pdev->dev);
> >>clk_disable_unprepare(xqspi->refclk);
> >>
> The imbalance is because pm_runtime_put_autosuspend is called to make 
> counter to be -1.
> 
> It looks strange that there is no counter increament op before 
> pm_runtime_put_autosuspend.
> 
> In my limited understanding, it should look like:
> 
> ..
> 
> pm_runtime_enable
> 
> pm_runtime_get_sync   //increase counter to one to resume device
> 
> DO OPERATIONS HERE
> 
> pm_runtime_mark_last_busy
> pm_runtime_put_autosuspend   //decrease counter to zero and trigger suspend
> 
> return 0;
> 
> error_path:
> 
> pm_runtime_put_sync
> 
> pm_runtime_disable
> 
> return err;
> 
> 
> Am I missing something?
> 

Thanks for point out this! Usually there is an increment refcount in a 
_probe function and a decrement refcount in a _remove function. Sometimes 
the refcount decrement is in the _probe and the increment is in the _remove. 
But the refcount is balanced in both cases. So I think zynqmp_qspi_remove()
needs a refcount increment to fix this bug.

Regards,
Dinghao

Re: Re: [PATCH] media: imx: imx7-mipi-csis: Fix runtime PM imbalance in mipi_csis_s_stream

[dinghao . liu]

> Hi Liu,
> Thanks for your patch.
> 
> On Thu, Apr 08, 2021 at 05:08:27PM +0800, Dinghao Liu wrote:
> > When v4l2_subdev_call() fails, a pairing PM usage counter
> > decrement is needed to keep the counter balanced. It's the
> > same for the following error paths in case 'enable' is on.
> > 
> > Signed-off-by: Dinghao Liu 
> > ---
> >  drivers/staging/media/imx/imx7-mipi-csis.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> > b/drivers/staging/media/imx/imx7-mipi-csis.c
> > index a01a7364b4b9..2a3fff231a40 100644
> > --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> > @@ -627,21 +627,26 @@ static int mipi_csis_s_stream(struct v4l2_subdev 
> > *mipi_sd, int enable)
> > return ret;
> > }
> > ret = v4l2_subdev_call(state->src_sd, core, s_power, 1);
> > -   if (ret < 0)
> > +   if (ret < 0) {
> > +   pm_runtime_put_noidle(&state->pdev->dev);
> 
> I think here we should go completely pm_runtime_put to call the
> mipi_csis_pm_suspend down the line, right?
> 
> > return ret;
> > +   }
> > }
> >  
> > mutex_lock(&state->lock);
> > if (enable) {
> > if (state->flags & ST_SUSPENDED) {
> > ret = -EBUSY;
> > +   pm_runtime_put_noidle(&state->pdev->dev);
> 
> since we are in ST_SUSPENDED state, for sure the pm counter was
> already 0.
> 
> > goto unlock;
> > }
> >  
> > mipi_csis_start_stream(state);
> > ret = v4l2_subdev_call(state->src_sd, video, s_stream, 1);
> > -   if (ret < 0)
> > +   if (ret < 0) {
> > +   pm_runtime_put_noidle(&state->pdev->dev);
> 
> here also we need the pm_runtime_put, maybe just changing the unlock
> tag bellow from:
> if (!enable)
> pm_runtime_put(&state->pdev->dev);
> 
> to 
> if (!enable || (ret < 0))
> pm_runtime_put(&state->pdev->dev);
> 
> will not hurt the first case and will complete the suspend routine
> afterward in the second case.
> 

This is much clearer, thanks! I will fix this and send a new patch soon.

Regards,
Dinghao

[PATCH] [v2] media: imx: imx7-mipi-csis: Fix runtime PM imbalance in mipi_csis_s_stream

[Dinghao Liu]

When v4l2_subdev_call() fails, a pairing PM usage counter
decrement is needed to keep the counter balanced. It's the
same for the following error paths in case 'enable' is on.

Signed-off-by: Dinghao Liu 
---

Changelog:

v2: - Use pm_runtime_put() to balance the refcount.
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
b/drivers/staging/media/imx/imx7-mipi-csis.c
index a01a7364b4b9..6f3e8a15e7c4 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -628,7 +628,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev *mipi_sd, 
int enable)
}
ret = v4l2_subdev_call(state->src_sd, core, s_power, 1);
if (ret < 0)
-   return ret;
+   goto pm_put;
}
 
mutex_lock(&state->lock);
@@ -657,7 +657,8 @@ static int mipi_csis_s_stream(struct v4l2_subdev *mipi_sd, 
int enable)
 
 unlock:
mutex_unlock(&state->lock);
-   if (!enable)
+pm_put:
+   if (!enable || (ret < 0))
pm_runtime_put(&state->pdev->dev);
 
return ret;
-- 
2.17.1

Re: Re: [PATCH] dmaengine: tegra20: Fix runtime PM imbalance in tegra_dma_issue_pending

[dinghao . liu]

> On 08/04/2021 08:11, Dinghao Liu wrote:
> > pm_runtime_get_sync() will increase the rumtime PM counter
> > even it returns an error. Thus a pairing decrement is needed
> > to prevent refcount leak. Fix this by replacing this API with
> > pm_runtime_resume_and_get(), which will not change the runtime
> > PM counter on error.
> > 
> > Signed-off-by: Dinghao Liu 
> > ---
> >  drivers/dma/tegra20-apb-dma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> > index 71827d9b0aa1..73178afaf4c2 100644
> > --- a/drivers/dma/tegra20-apb-dma.c
> > +++ b/drivers/dma/tegra20-apb-dma.c
> > @@ -723,7 +723,7 @@ static void tegra_dma_issue_pending(struct dma_chan *dc)
> > goto end;
> > }
> > if (!tdc->busy) {
> > -   err = pm_runtime_get_sync(tdc->tdma->dev);
> > +   err = pm_runtime_resume_and_get(tdc->tdma->dev);
> > if (err < 0) {
> > dev_err(tdc2dev(tdc), "Failed to enable DMA\n");
> > goto end;
> > 
> 
> 
> Thanks! Looks like there are two instances of this that need fixing.
> 

Thanks for pointing out this! I will fix this and send a new patch soon.

Regards,
Dinghao

[PATCH] [v2] dmaengine: tegra20: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() will increase the runtime PM counter
even it returns an error. Thus a pairing decrement is needed
to prevent refcount leak. Fix this by replacing this API with
pm_runtime_resume_and_get(), which will not change the runtime
PM counter on error.

Signed-off-by: Dinghao Liu 
---

Changelog:

v2: - Fix another similar case in tegra_dma_synchronize().
---
 drivers/dma/tegra20-apb-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 71827d9b0aa1..b7260749e8ee 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -723,7 +723,7 @@ static void tegra_dma_issue_pending(struct dma_chan *dc)
goto end;
}
if (!tdc->busy) {
-   err = pm_runtime_get_sync(tdc->tdma->dev);
+   err = pm_runtime_resume_and_get(tdc->tdma->dev);
if (err < 0) {
dev_err(tdc2dev(tdc), "Failed to enable DMA\n");
goto end;
@@ -818,7 +818,7 @@ static void tegra_dma_synchronize(struct dma_chan *dc)
struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
int err;
 
-   err = pm_runtime_get_sync(tdc->tdma->dev);
+   err = pm_runtime_resume_and_get(tdc->tdma->dev);
if (err < 0) {
dev_err(tdc2dev(tdc), "Failed to synchronize DMA: %d\n", err);
return;
-- 
2.17.1

Re: Re: [PATCH] ASoC: Intel: Skylake: Fix missing check in skl_pcm_trigger

[dinghao . liu]

> 
> On 2/15/21 7:13 AM, Dinghao Liu wrote:
> > When cmd == SNDRV_PCM_TRIGGER_STOP, we should also check
> > the return value of skl_decoupled_trigger() just like what
> > we have done in case SNDRV_PCM_TRIGGER_PAUSE_RELEASE.
> >
> > Signed-off-by: Dinghao Liu 
> > ---
> >  sound/soc/intel/skylake/skl-pcm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/sound/soc/intel/skylake/skl-pcm.c 
> > b/sound/soc/intel/skylake/skl-pcm.c
> > index b1ca64d2f7ea..a5b1f333a500 100644
> > --- a/sound/soc/intel/skylake/skl-pcm.c
> > +++ b/sound/soc/intel/skylake/skl-pcm.c
> > @@ -516,6 +516,9 @@ static int skl_pcm_trigger(struct snd_pcm_substream 
> > *substream, int cmd,
> > return ret;
> 
> Is there any additional error handling to be done for the
> 
> skl_stop_pipe that just happened ?
> 

I think the check is enough here.

Regards,
Dinghao

[PATCH] ALSA: intel8x0: Fix missing check in snd_intel8x0m_create

[Dinghao Liu]

When device_type == DEVICE_ALI, we should also check the return
value of pci_iomap() to avoid potential null pointer dereference.

Signed-off-by: Dinghao Liu 
---
 sound/pci/intel8x0m.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/pci/intel8x0m.c b/sound/pci/intel8x0m.c
index 1b7df0c4e57c..19872cecc9d2 100644
--- a/sound/pci/intel8x0m.c
+++ b/sound/pci/intel8x0m.c
@@ -1129,13 +1129,14 @@ static int snd_intel8x0m_create(struct snd_card *card,
chip->bmaddr = pci_iomap(pci, 3, 0);
else
chip->bmaddr = pci_iomap(pci, 1, 0);
+
+port_inited:
if (!chip->bmaddr) {
dev_err(card->dev, "Controller space ioremap problem\n");
snd_intel8x0m_free(chip);
return -EIO;
}
 
- port_inited:
/* initialize offsets */
chip->bdbars_count = 2;
tbl = intel_regs;
-- 
2.17.1

[PATCH] RDMA/siw: Fix missing check in siw_get_hdr

[Dinghao Liu]

We should also check the range of opcode after calling
__rdmap_get_opcode() in the else branch to prevent potential
overflow.

Fixes: 8b6a361b8c482 ("rdma/siw: receive path")
Signed-off-by: Dinghao Liu 
---
 drivers/infiniband/sw/siw/siw_qp_rx.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/infiniband/sw/siw/siw_qp_rx.c 
b/drivers/infiniband/sw/siw/siw_qp_rx.c
index 60116f20653c..301e7fe2c61a 100644
--- a/drivers/infiniband/sw/siw/siw_qp_rx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_rx.c
@@ -1072,6 +1072,16 @@ static int siw_get_hdr(struct siw_rx_stream *srx)
siw_dbg_qp(rx_qp(srx), "new header, opcode %u\n", opcode);
} else {
opcode = __rdmap_get_opcode(c_hdr);
+
+   if (opcode > RDMAP_TERMINATE) {
+   pr_warn("siw: received unknown packet type %u\n",
+   opcode);
+
+   siw_init_terminate(rx_qp(srx), TERM_ERROR_LAYER_RDMAP,
+  RDMAP_ETYPE_REMOTE_OPERATION,
+  RDMAP_ECODE_OPCODE, 0);
+   return -EINVAL;
+   }
}
set_rx_fpdu_context(qp, opcode);
frx = qp->rx_fpdu;
-- 
2.17.1

[PATCH] mmc: sdhci-pci-o2micro: Add missing checks in sdhci_pci_o2_probe

[Dinghao Liu]

It's odd to adopt different error handling on failure of
pci_read_config_dword(). Check the return value and terminate
execution flow on failure of all pci_read_config_dword() calls
in this function.

Signed-off-by: Dinghao Liu 
---
 drivers/mmc/host/sdhci-pci-o2micro.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pci-o2micro.c 
b/drivers/mmc/host/sdhci-pci-o2micro.c
index 94e3f72f6405..51d55a87aebe 100644
--- a/drivers/mmc/host/sdhci-pci-o2micro.c
+++ b/drivers/mmc/host/sdhci-pci-o2micro.c
@@ -706,6 +706,8 @@ static int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
ret = pci_read_config_dword(chip->pdev,
O2_SD_FUNC_REG0,
&scratch_32);
+   if (ret)
+   return ret;
scratch_32 = ((scratch_32 & 0xFF00) >> 24);
 
/* Check Whether subId is 0x11 or 0x12 */
@@ -716,6 +718,8 @@ static int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
ret = pci_read_config_dword(chip->pdev,
O2_SD_FUNC_REG4,
&scratch_32);
+   if (ret)
+   return ret;
 
/* Enable Base Clk setting change */
scratch_32 |= O2_SD_FREG4_ENABLE_CLK_SET;
@@ -795,6 +799,8 @@ static int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
 
ret = pci_read_config_dword(chip->pdev,
O2_SD_PLL_SETTING, &scratch_32);
+   if (ret)
+   return ret;
 
if ((scratch_32 & 0xff00) == 0x0100) {
scratch_32 &= 0x;
@@ -812,6 +818,8 @@ static int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
ret = pci_read_config_dword(chip->pdev,
O2_SD_FUNC_REG4,
&scratch_32);
+   if (ret)
+   return ret;
scratch_32 |= (1 << 22);
pci_write_config_dword(chip->pdev,
   O2_SD_FUNC_REG4, scratch_32);
-- 
2.17.1

Re: Re: [PATCH] RDMA/siw: Fix missing check in siw_get_hdr

[dinghao . liu]

"Bernard Metzler" <b...@zurich.ibm.com>写道：
> -"Dinghao Liu"  wrote: -
> 
> >To: dinghao....@zju.edu.cn, k...@umn.edu
> >From: "Dinghao Liu" 
> >Date: 02/26/2021 08:56AM
> >Cc: "Bernard Metzler" , "Doug Ledford"
> >, "Jason Gunthorpe" ,
> >linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> >Subject: [EXTERNAL] [PATCH] RDMA/siw: Fix missing check in
> >siw_get_hdr
> >
> >We should also check the range of opcode after calling
> >__rdmap_get_opcode() in the else branch to prevent potential
> >overflow.
> 
> Hi Dinghao,
> No this is not needed. We always first read the minimum
> header information (MPA len, DDP flags, RDMAP opcode,
> STag, target offset). Only if we have received that
> into local buffer, we check for the opcode this one time.
> Now the opcode determines the remaining length of the
> variably sized part of the header to be received.
> 
> We do not have to check the opcode again, since we
> already received and checked it.
> 

It's clear to me, thanks!

Regards,
Dinghao

[PATCH] [v2] i2c: imx-lpi2c: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
Changelog:

v2: - Use pm_runtime_put_noidle() instead of
  pm_runtime_put_autosuspend().
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c 
b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 94743ba581fe..bdee02dff284 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -260,8 +260,10 @@ static int lpi2c_imx_master_enable(struct lpi2c_imx_struct 
*lpi2c_imx)
int ret;
 
ret = pm_runtime_get_sync(lpi2c_imx->adapter.dev.parent);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(lpi2c_imx->adapter.dev.parent);
return ret;
+   }
 
temp = MCR_RST;
writel(temp, lpi2c_imx->base + LPI2C_MCR);
-- 
2.17.1

Re: Re: [PATCH v2] i2c: imx-lpi2c: Fix runtime PM imbalance on error in lpi2c_imx_master_enable()

[dinghao . liu]

Hi, Markus, 

> * How do you think about to replace the word “pairing” by “corresponding”?
> 
> * Will it be helpful to add an imperative wording?
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=ffeb595d84811dde16a28b33d8a7cf26d51d51b3#n151
> 
> * Would you like to add the tag “Fixes” to the commit message?
> 
> * Are you going to take such possibilities into account for any more patches?
> 

Thank you for your advice! I will fix them soon in the next version of patch.

Regards,
Dinghao

Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

[dinghao . liu]

> 
> > It's the same when gm20b_clk_new() returns from elsewhere following this 
> > call.
> 
> I suggest to reconsider the interpretation of the software situation once 
> more.
> Can it be that the allocated clock object should be kept usable even after
> a successful return from this function?
> 

It's possible that we expect an usable clk pointer, though I could not find
the exact usage yet. For security, I will release this pointer only on error 
paths in this function.

> 
> Would you like to add the tag “Fixes” to the commit message?
> 

Thank you for your advice! I will add this tag in the next version of patch.

Regards,
Dinghao

Re: Re: drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

[dinghao . liu]

> 
> > For security, I will release this pointer only on error paths in this 
> > function.
> 
> Do you tend to release objects (which are referenced by pointers)?
> 

I just found that clk is referenced by pclk in this function. When clk is 
freed, 
pclk will be allocated in gm20b_clk_new_speedo0(). Thus we should not release 
clk
in this function and there is no bug here. Thank you for reminding me!

Regards,
Dinghao

Re: Re: drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

[dinghao . liu]

> > I just found that clk is referenced by pclk in this function. When clk is 
> > freed,
> > pclk will be allocated in gm20b_clk_new_speedo0(). Thus we should not 
> > release clk
> > in this function and there is no bug here.
> 
> Can there be a need to release a clock object after a failed gk20a_clk_ctor() 
> call?
> 

I think this mainly depends on pclk pointer. It seems that the caller of 
gm20b_clk_new() always expects pclk to be allocated unless it returns -ENOMEM,
which means kzalloc() failed. If gk20a_clk_ctor() never returns such an error 
code, we may need not to release this clock object.

Regards,
Dinghao

Re: Re: drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

[dinghao . liu]

> > If gk20a_clk_ctor() never returns such an error code,
> > we may need not to release this clock object.
> 
> Would you like to achieve complete exception handling
> also for this function implementation?
> 

It seems that it's possible to get -ENOMEM from gk20a_clk_ctor().
The call chain is as follows:
gk20a_clk_ctor() <- nvkm_clk_ctor() <- nvkm_notify_init()

When nvkm_notify_init() returns -ENOMEM, all of its callers (and 
callers of callers) will be influenced if there is a failed
kzalloc inside which. 

In this case, maybe we should check the return value of 
gk20a_clk_ctor() and release clk if it returns -ENOMEM. 
And many other functions also have the same issue (e.g.,
gm20b_clk_new_speedo0). Do you have any idea about this 
problem?

Regards,
Dinghao

Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new

[dinghao . liu]

Hi Ben,

> > When gk20a_clk_ctor() returns an error code, pointer "clk"
> > should be released. It's the same when gm20b_clk_new()
> > returns from elsewhere following this call.
> This shouldn't be necessary.  If a subdev constructor fails, and
> returns a pointer, the core will call the destructor to clean things
> up.
> 

I'm not familiar with the behavior of the caller of gm20b_clk_new(). 
If the subdev constructor fails, the core will check the pointer
(here is "pclk"), then it's ok and there is no bug (Do you mean 
this?). If the core executes error handling code only according to 
the error code, there may be a memory leak bug (the caller cannot 
know if -ENOMEM comes from the failure of kzalloc or gk20a_clk_ctor). 
If the core always calls the destructor as long as the constructor 
fails (even if the kzalloc fails), we may have a double free bug. 

Would you like to give a more detailed explanation about the behavior
of the core? 

Regards,
Dinghao

Re: Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new

[dinghao . liu]

> > If there's *any* error, it'll check the pointer, if it's non-NULL,
> > it'll call the destructor.  If kzalloc() fails, the pointer will be
> > NULL, there's no double-free bug.  *every* subdev is written this way
> > to avoid duplicating cleanup logic.
> Actually, gm20b_clk_new_speedo0() may have a bug here if kzalloc()
> fails as it doesn't overwrite the previous pointer from
> gm20b_clk_new().  That whole ctor() sequence is written a little
> strangely.
> 

It's clear to me, thank your for your explanation! As for 
gm20b_clk_new_speedo0(), I think its bug pattern is not very 
clear. Maybe we should keep it until we find an use chain that
could lead to a bug.

Regards,
Dinghao

[PATCH] [v3] i2c: imx-lpi2c: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a corresponding decrement is
needed on the error handling path to keep the counter balanced.

Fix this by adding the missed function call.

Fixes: 13d6eb20fc79a ("i2c: imx-lpi2c: add runtime pm support")
Co-developed-by: Markus Elfring 
Signed-off-by: Markus Elfring 
Signed-off-by: Dinghao Liu 
---

Changelog:

v2: - Use pm_runtime_put_noidle() instead of
  pm_runtime_put_autosuspend().

v3: - Refine commit message.
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c 
b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 94743ba581fe..bdee02dff284 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -260,8 +260,10 @@ static int lpi2c_imx_master_enable(struct lpi2c_imx_struct 
*lpi2c_imx)
int ret;
 
ret = pm_runtime_get_sync(lpi2c_imx->adapter.dev.parent);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(lpi2c_imx->adapter.dev.parent);
return ret;
+   }
 
temp = MCR_RST;
writel(temp, lpi2c_imx->base + LPI2C_MCR);
-- 
2.17.1

[PATCH] crypto: sun8i-ce - fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c 
b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
index 3e4e4bbda34c..18f0f251204c 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
@@ -598,8 +598,10 @@ static int sun8i_ce_probe(struct platform_device *pdev)
goto error_alg;
 
err = pm_runtime_get_sync(ce->dev);
-   if (err < 0)
+   if (err < 0) {
+   pm_runtime_put_sync(ce->dev);
goto error_alg;
+   }
 
v = readl(ce->base + CE_CTR);
v >>= CE_DIE_ID_SHIFT;
-- 
2.17.1

[PATCH] power: supply: bq24190_charger: fix runtime pm imbalance

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/power/supply/bq24190_charger.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq24190_charger.c 
b/drivers/power/supply/bq24190_charger.c
index 453d6332d43a..4f7c6b6abb56 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -481,8 +481,10 @@ static ssize_t bq24190_sysfs_store(struct device *dev,
return ret;
 
ret = pm_runtime_get_sync(bdi->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(bdi->dev);
return ret;
+   }
 
ret = bq24190_write_mask(bdi, info->reg, info->mask, info->shift, v);
if (ret)
-- 
2.17.1

[PATCH] crypto: sun8i-ss - fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c 
b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c
index 84d52fc3a2da..b1c766bf4483 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c
@@ -359,6 +359,7 @@ int sun8i_ss_cipher_init(struct crypto_tfm *tfm)
 
return 0;
 error_pm:
+   pm_runtime_put_sync(op->ss->dev);
crypto_free_sync_skcipher(op->fallback_tfm);
return err;
 }
-- 
2.17.1

[PATCH] crypto: sun8i-ss - fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c 
b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c
index 6b301afffd11..41841415ead6 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c
@@ -571,8 +571,10 @@ static int sun8i_ss_probe(struct platform_device *pdev)
goto error_alg;
 
err = pm_runtime_get_sync(ss->dev);
-   if (err < 0)
+   if (err < 0) {
+   pm_runtime_put_sync(ss->dev);
goto error_alg;
+   }
 
v = readl(ss->base + SS_CTL_REG);
v >>= SS_DIE_ID_SHIFT;
-- 
2.17.1

[PATCH] PCI: rcar: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/pcie-rcar.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-rcar.c 
b/drivers/pci/controller/pcie-rcar.c
index 759c6542c5c8..a9de65438051 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -1207,9 +1207,8 @@ static int rcar_pcie_probe(struct platform_device *pdev)
irq_dispose_mapping(pcie->msi.irq1);
 
 err_pm_put:
-   pm_runtime_put(dev);
-
 err_pm_disable:
+   pm_runtime_put(dev);
pm_runtime_disable(dev);
pci_free_resource_list(&pcie->resources);
 
-- 
2.17.1

[PATCH] PCI: tegra: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Also This driver forgets to call pm_runtime_disable() when
pm_runtime_get_sync() returns an error code.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/pci-tegra.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c 
b/drivers/pci/controller/pci-tegra.c
index 3e64ba6a36a8..00236dd65b5b 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2712,7 +2712,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
err = pm_runtime_get_sync(pcie->dev);
if (err < 0) {
dev_err(dev, "fail to enable pcie controller: %d\n", err);
-   goto teardown_msi;
+   goto pm_runtime_put;
}
 
host->busnr = bus->start;
@@ -2746,7 +2746,6 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 pm_runtime_put:
pm_runtime_put_sync(pcie->dev);
pm_runtime_disable(pcie->dev);
-teardown_msi:
tegra_pcie_msi_teardown(pcie);
 put_resources:
tegra_pcie_put_resources(pcie);
-- 
2.17.1

[PATCH] PCI: dwc: pci-dra7xx: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/dwc/pci-dra7xx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c 
b/drivers/pci/controller/dwc/pci-dra7xx.c
index 3b0e58f2de58..8fd9f2281e02 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -1000,9 +1000,8 @@ static int __init dra7xx_pcie_probe(struct 
platform_device *pdev)
return 0;
 
 err_gpio:
-   pm_runtime_put(dev);
-
 err_get_sync:
+   pm_runtime_put(dev);
pm_runtime_disable(dev);
dra7xx_pcie_disable_phy(dra7xx);
 
-- 
2.17.1

[PATCH] PCI: qcom: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/dwc/pcie-qcom.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c 
b/drivers/pci/controller/dwc/pcie-qcom.c
index 138e1a2d21cc..35686930df1d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1340,8 +1340,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
-   pm_runtime_disable(dev);
-   return ret;
+   goto err_pm_runtime_put;
}
 
pci->dev = dev;
-- 
2.17.1

[PATCH] PCI: tegra: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index ae30a2fd3716..a69f9e49dcb5 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1651,8 +1651,8 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw 
*pcie)
 fail_host_init:
tegra_pcie_deinit_controller(pcie);
 fail_pinctrl:
-   pm_runtime_put_sync(dev);
 fail_pm_get_sync:
+   pm_runtime_put_sync(dev);
pm_runtime_disable(dev);
return ret;
 }
-- 
2.17.1

[PATCH] PCI: cadence: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/cadence/pcie-cadence-plat.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c 
b/drivers/pci/controller/cadence/pcie-cadence-plat.c
index f5c6bf6dfcb8..33c3868e6dbd 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-plat.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-plat.c
@@ -115,9 +115,8 @@ static int cdns_plat_pcie_probe(struct platform_device 
*pdev)
}
 
  err_init:
-   pm_runtime_put_sync(dev);
-
  err_get_sync:
+   pm_runtime_put_sync(dev);
pm_runtime_disable(dev);
cdns_pcie_disable_phy(cdns_plat_pcie->pcie);
phy_count = cdns_plat_pcie->pcie->phy_count;
-- 
2.17.1

[PATCH] media: staging: tegra-vde: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/staging/media/tegra-vde/vde.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c 
b/drivers/staging/media/tegra-vde/vde.c
index d3e63512a765..dd134a3a15c7 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -777,7 +777,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
 
ret = pm_runtime_get_sync(dev);
if (ret < 0)
-   goto unlock;
+   goto put_runtime_pm;
 
/*
 * We rely on the VDE registers reset value, otherwise VDE
-- 
2.17.1

[PATCH] drm/etnaviv: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index a31eeff2b297..da3f6ca5849f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1691,6 +1691,9 @@ static int etnaviv_gpu_bind(struct device *dev, struct 
device *master,
return 0;
 
 out_sched:
+#ifdef CONFIG_PM
+   pm_runtime_put_autosuspend(gpu->dev);
+#endif
etnaviv_sched_fini(gpu);
 
 out_workqueue:
-- 
2.17.1

[PATCH] drm/nouveau/debugfs: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c 
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 15a3d40edf02..db3711436577 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -181,8 +181,11 @@ nouveau_debugfs_pstate_set(struct file *file, const char 
__user *ubuf,
}
 
ret = pm_runtime_get_sync(drm->dev);
-   if (ret < 0 && ret != -EACCES)
+   if (ret < 0 && ret != -EACCES) {
+   pm_runtime_put_autosuspend(drm->dev);
return ret;
+   }
+
ret = nvif_mthd(ctrl, NVIF_CONTROL_PSTATE_USER, &args, sizeof(args));
pm_runtime_put_autosuspend(drm->dev);
if (ret < 0)
-- 
2.17.1

[PATCH] drm/nouveau: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/gpu/drm/nouveau/nouveau_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index f5ece1f94973..6697f960dd89 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -76,8 +76,10 @@ nouveau_gem_object_open(struct drm_gem_object *gem, struct 
drm_file *file_priv)
return ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0 && ret != -EACCES)
+   if (ret < 0 && ret != -EACCES) {
+   pm_runtime_put_autosuspend(dev);
goto out;
+   }
 
ret = nouveau_vma_new(nvbo, vmm, &vma);
pm_runtime_mark_last_busy(dev);
-- 
2.17.1

[PATCH] drm/nouveau: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index f5ece1f94973..125cefbb6210 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -157,8 +157,8 @@ nouveau_gem_object_close(struct drm_gem_object *gem, struct 
drm_file *file_priv)
if (!WARN_ON(ret < 0 && ret != -EACCES)) {
nouveau_gem_object_unmap(nvbo, vma);
pm_runtime_mark_last_busy(dev);
-   pm_runtime_put_autosuspend(dev);
}
+   pm_runtime_put_autosuspend(dev);
}
}
ttm_bo_unreserve(&nvbo->bo);
-- 
2.17.1

[PATCH] drm/nouveau/dispnv50: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 6be9df1820c5..e670756664ff 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1123,8 +1123,10 @@ nv50_mstc_detect(struct drm_connector *connector,
return connector_status_disconnected;
 
ret = pm_runtime_get_sync(connector->dev->dev);
-   if (ret < 0 && ret != -EACCES)
+   if (ret < 0 && ret != -EACCES) {
+   pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+   }
 
ret = drm_dp_mst_detect_port(connector, ctx, mstc->port->mgr,
 mstc->port);
-- 
2.17.1

[PATCH] drm/panfrost: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/gpu/drm/panfrost/panfrost_job.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 7914b1570841..5719e356c969 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -146,8 +146,10 @@ static void panfrost_job_hw_submit(struct panfrost_job 
*job, int js)
int ret;
 
ret = pm_runtime_get_sync(pfdev->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_sync_autosuspend(pfdev->dev);
return;
+   }
 
if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js {
pm_runtime_put_sync_autosuspend(pfdev->dev);
-- 
2.17.1

[PATCH] drm/v3d: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/gpu/drm/v3d/v3d_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 549dde83408b..d53c683ed74c 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -440,8 +440,10 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file 
*file_priv,
job->free = free;
 
ret = pm_runtime_get_sync(v3d->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(v3d->dev);
return ret;
+   }
 
xa_init_flags(&job->deps, XA_FLAGS_ALLOC);
 
-- 
2.17.1

[PATCH] can: xilinx_can: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/net/can/xilinx_can.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index c1dbab8c896d..a9e8184cc611 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1823,8 +1823,8 @@ static int xcan_probe(struct platform_device *pdev)
return 0;
 
 err_disableclks:
-   pm_runtime_put(priv->dev);
 err_pmdisable:
+   pm_runtime_put(priv->dev);
pm_runtime_disable(&pdev->dev);
 err_free:
free_candev(ndev);
-- 
2.17.1

[PATCH] wlcore: fix runtime pm imbalance in wl1271_tx_work

[Dinghao Liu]

There are two error handling paths in this functon. When
wlcore_tx_work_locked() returns an error code, we should
decrease the runtime PM usage counter the same way as the
error handling path beginning from pm_runtime_get_sync().

Signed-off-by: Dinghao Liu 
---
 drivers/net/wireless/ti/wlcore/tx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ti/wlcore/tx.c 
b/drivers/net/wireless/ti/wlcore/tx.c
index 90e56d4c3df3..e20e18cd04ae 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -863,6 +863,7 @@ void wl1271_tx_work(struct work_struct *work)
 
ret = wlcore_tx_work_locked(wl);
if (ret < 0) {
+   pm_runtime_put_noidle(wl->dev);
wl12xx_queue_recovery_work(wl);
goto out;
}
-- 
2.17.1

[PATCH] wlcore: fix runtime pm imbalance in wlcore_regdomain_config

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/net/wireless/ti/wlcore/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index f140f7d7f553..c7e4f5a80b9e 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3662,8 +3662,10 @@ void wlcore_regdomain_config(struct wl1271 *wl)
goto out;
 
ret = pm_runtime_get_sync(wl->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(wl->dev);
goto out;
+   }
 
ret = wlcore_cmd_regdomain_config_locked(wl);
if (ret < 0) {
-- 
2.17.1

[PATCH] wlcore: fix runtime pm imbalance in wl1271_op_suspend

[Dinghao Liu]

When wlcore_hw_interrupt_notify() returns an error code,
a pairing runtime PM usage counter decrement is needed to
keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/net/wireless/ti/wlcore/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index f140f7d7f553..8faee8cec1bc 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1746,9 +1746,7 @@ static int __maybe_unused wl1271_op_suspend(struct 
ieee80211_hw *hw,
 
ret = wl1271_configure_suspend(wl, wlvif, wow);
if (ret < 0) {
-   mutex_unlock(&wl->mutex);
-   wl1271_warning("couldn't prepare device to suspend");
-   return ret;
+   goto out_sleep;
}
}
 
-- 
2.17.1

[PATCH] wlcore: fix runtime pm imbalance in __wl1271_op_remove_interface

[Dinghao Liu]

When wl12xx_cmd_role_disable() returns an error code,
a pairing runtime PM usage counter decrement is needed to
keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/net/wireless/ti/wlcore/main.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index f140f7d7f553..e6c299efbc2e 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2698,12 +2698,16 @@ static void __wl1271_op_remove_interface(struct wl1271 
*wl,
 
if (!wlcore_is_p2p_mgmt(wlvif)) {
ret = wl12xx_cmd_role_disable(wl, &wlvif->role_id);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(wl->dev);
goto deinit;
+   }
} else {
ret = wl12xx_cmd_role_disable(wl, &wlvif->dev_role_id);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(wl->dev);
goto deinit;
+   }
}
 
pm_runtime_mark_last_busy(wl->dev);
-- 
2.17.1

[PATCH] hwrng: exynos - fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/char/hw_random/exynos-trng.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/exynos-trng.c 
b/drivers/char/hw_random/exynos-trng.c
index 8e1fe3f8dd2d..133e017db577 100644
--- a/drivers/char/hw_random/exynos-trng.c
+++ b/drivers/char/hw_random/exynos-trng.c
@@ -165,9 +165,8 @@ static int exynos_trng_probe(struct platform_device *pdev)
clk_disable_unprepare(trng->clk);
 
 err_clock:
-   pm_runtime_put_sync(&pdev->dev);
-
 err_pm_get:
+   pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
 
return ret;
-- 
2.17.1

[PATCH] hwrng: ks-sa - fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/char/hw_random/ks-sa-rng.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/hw_random/ks-sa-rng.c 
b/drivers/char/hw_random/ks-sa-rng.c
index e2330e757f1f..85c81da4a8af 100644
--- a/drivers/char/hw_random/ks-sa-rng.c
+++ b/drivers/char/hw_random/ks-sa-rng.c
@@ -244,6 +244,7 @@ static int ks_sa_rng_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
dev_err(dev, "Failed to enable SA power-domain\n");
+   pm_runtime_put_sync(dev);
pm_runtime_disable(dev);
return ret;
}
-- 
2.17.1

[PATCH] Input: omap-keypad - fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/input/keyboard/omap4-keypad.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c 
b/drivers/input/keyboard/omap4-keypad.c
index 94c94d7f5155..d13d81c796d2 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -371,8 +371,8 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 err_free_input:
input_free_device(input_dev);
 err_pm_put_sync:
-   pm_runtime_put_sync(&pdev->dev);
 err_unmap:
+   pm_runtime_put_sync(&pdev->dev);
iounmap(keypad_data->base);
 err_release_mem:
release_mem_region(res->start, resource_size(res));
-- 
2.17.1

[PATCH] i2c: imx-lpi2c: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c 
b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 94743ba581fe..89908c24c8b3 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -260,8 +260,10 @@ static int lpi2c_imx_master_enable(struct lpi2c_imx_struct 
*lpi2c_imx)
int ret;
 
ret = pm_runtime_get_sync(lpi2c_imx->adapter.dev.parent);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(lpi2c_imx->adapter.dev.parent);
return ret;
+   }
 
temp = MCR_RST;
writel(temp, lpi2c_imx->base + LPI2C_MCR);
-- 
2.17.1

[PATCH] [v2] PCI: tegra: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Also, call pm_runtime_disable() when pm_runtime_get_sync() returns
an error code.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/pci-tegra.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c 
b/drivers/pci/controller/pci-tegra.c
index 3e64ba6a36a8..00236dd65b5b 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2712,7 +2712,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
err = pm_runtime_get_sync(pcie->dev);
if (err < 0) {
dev_err(dev, "fail to enable pcie controller: %d\n", err);
-   goto teardown_msi;
+   goto pm_runtime_put;
}
 
host->busnr = bus->start;
@@ -2746,7 +2746,6 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 pm_runtime_put:
pm_runtime_put_sync(pcie->dev);
pm_runtime_disable(pcie->dev);
-teardown_msi:
tegra_pcie_msi_teardown(pcie);
 put_resources:
tegra_pcie_put_resources(pcie);
-- 
2.17.1

Re: Re: [PATCH] PCI: tegra: fix runtime pm imbalance on error

[dinghao . liu]

Thank you for your advice. I think tegra194 is a good choice and 
I will use it in the next edition of patch.

"Bjorn Helgaas" <helg...@kernel.org>写道：
> On Wed, May 20, 2020 at 11:59:08AM +0200, Thierry Reding wrote:
> > On Wed, May 20, 2020 at 04:52:23PM +0800, Dinghao Liu wrote:
> > > pm_runtime_get_sync() increments the runtime PM usage counter even
> > > it returns an error code. Thus a pairing decrement is needed on
> > 
> > s/even it/even when it/
> > 
> > Might also be a good idea to use a different subject prefix because I
> > was almost not going to look at the other patch, taking this to be a
> > replacement for it.
> 
> Amen.  This would be a good change to start using "PCI: tegra194" or
> something for pcie-tegra194.c.  Or will there be tegra195, tegra 196,
> etc added to this driver?
> 
> Also, please capitalize the first word and "PM" in the subjects:
> 
>   PCI: tegra194: Fix runtime PM imbalance on error
> 
> Bjorn

Re: Re: [PATCH] PCI: tegra: fix runtime pm imbalance on error

[dinghao . liu]

Thank you for your advice. I will fix these problems in the next edition of 
patch.

"Thierry Reding" <thierry.red...@gmail.com>写道：
> On Wed, May 20, 2020 at 04:52:23PM +0800, Dinghao Liu wrote:
> > pm_runtime_get_sync() increments the runtime PM usage counter even
> > it returns an error code. Thus a pairing decrement is needed on
> 
> s/even it/even when it/
> 
> Might also be a good idea to use a different subject prefix because I
> was almost not going to look at the other patch, taking this to be a
> replacement for it.
> 
> Although, looking at the log we have used this same prefix for both
> drivers in the past...
> 
> > the error handling path to keep the counter balanced.
> > 
> > Signed-off-by: Dinghao Liu 
> > ---
> >  drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
> > b/drivers/pci/controller/dwc/pcie-tegra194.c
> > index ae30a2fd3716..a69f9e49dcb5 100644
> > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > @@ -1651,8 +1651,8 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw 
> > *pcie)
> >  fail_host_init:
> > tegra_pcie_deinit_controller(pcie);
> >  fail_pinctrl:
> > -   pm_runtime_put_sync(dev);
> >  fail_pm_get_sync:
> 
> Either of those two labels is now no longer needed. Of course it'll now
> be odd to jump to fail_pm_get_sync on pinctrl_pm_select_default_state()
> failure, but that's one of the reasons why label should have names
> describing what they do rather than describe the failure location. I
> guess we can live with that for now. I'll make a note to send a cleanup
> patch for that later on.
> 
> With the fixup in the commit message and either of the labels removed:
> 
> Acked-by: Thierry Reding 

[PATCH] [v2] PCI: tegra194: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index ae30a2fd3716..2c0d2ce16b47 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1623,7 +1623,7 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw 
*pcie)
ret = pinctrl_pm_select_default_state(dev);
if (ret < 0) {
dev_err(dev, "Failed to configure sideband pins: %d\n", ret);
-   goto fail_pinctrl;
+   goto fail_pm_get_sync;
}
 
tegra_pcie_init_controller(pcie);
@@ -1650,9 +1650,8 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw 
*pcie)
 
 fail_host_init:
tegra_pcie_deinit_controller(pcie);
-fail_pinctrl:
-   pm_runtime_put_sync(dev);
 fail_pm_get_sync:
+   pm_runtime_put_sync(dev);
pm_runtime_disable(dev);
return ret;
 }
-- 
2.17.1

Re: Re: [PATCH] media: staging: tegra-vde: fix runtime pm imbalance on error

[dinghao . liu]

Hi, Dan,

I agree the best solution is to fix __pm_runtime_resume(). But there are also 
many cases that assume pm_runtime_get_sync() will change PM usage 
counter on error. According to my static analysis results, the number of these 
"right" cases are larger. Adjusting __pm_runtime_resume() directly will 
introduce 
more new bugs. Therefore I think we should resolve the "bug" cases individually.

I think that Dmitry's patch is more reasonable than mine. 

Dinghao

"Dan Carpenter" <dan.carpen...@oracle.com>写道：
> On Wed, May 20, 2020 at 01:15:44PM +0300, Dmitry Osipenko wrote:
> > 20.05.2020 12:51, Dinghao Liu пишет:
> > > pm_runtime_get_sync() increments the runtime PM usage counter even
> > > it returns an error code. Thus a pairing decrement is needed on
> > > the error handling path to keep the counter balanced.
> > > 
> > > Signed-off-by: Dinghao Liu 
> > > ---
> > >  drivers/staging/media/tegra-vde/vde.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/media/tegra-vde/vde.c 
> > > b/drivers/staging/media/tegra-vde/vde.c
> > > index d3e63512a765..dd134a3a15c7 100644
> > > --- a/drivers/staging/media/tegra-vde/vde.c
> > > +++ b/drivers/staging/media/tegra-vde/vde.c
> > > @@ -777,7 +777,7 @@ static int tegra_vde_ioctl_decode_h264(struct 
> > > tegra_vde *vde,
> > >  
> > >   ret = pm_runtime_get_sync(dev);
> > >   if (ret < 0)
> > > - goto unlock;
> > > + goto put_runtime_pm;
> > >  
> > >   /*
> > >* We rely on the VDE registers reset value, otherwise VDE
> > > 
> > 
> > Hello Dinghao,
> > 
> > Thank you for the patch. I sent out a similar patch a week ago [1].
> > 
> > [1]
> > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200514210847.9269-2-dig...@gmail.com/
> > 
> > The pm_runtime_put_noidle() should have the same effect as yours
> > variant, although my variant won't change the last_busy RPM time, which
> > I think is a bit more appropriate behavior.
> 
> I don't think either patch is correct.  The right thing to do is to fix
> __pm_runtime_resume() so it doesn't leak a reference count on error.
> 
> The problem is that a lot of functions don't check the return so
> possibly we are relying on that behavior.  We may need to introduce a
> new function which cleans up properly instead of leaking reference
> counts?
> 
> Also it's not documented that pm_runtime_get_sync() returns 1 sometimes
> on success so it leads to a few bugs.
> 
> drivers/gpu/drm/stm/ltdc.c: ret = pm_runtime_get_sync(ddev->dev);
> drivers/gpu/drm/stm/ltdc.c- if (ret) {
> --
> drivers/gpu/drm/stm/ltdc.c: ret = pm_runtime_get_sync(ddev->dev);
> drivers/gpu/drm/stm/ltdc.c- if (ret) {
> 
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c:  ret = 
> pm_runtime_get_sync(pm->dev);
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c-  if (ret)
> 
> drivers/media/platform/ti-vpe/cal.c:ret = pm_runtime_get_sync(&pdev->dev);
> drivers/media/platform/ti-vpe/cal.c-if (ret)
> 
> drivers/mfd/arizona-core.c: ret = 
> pm_runtime_get_sync(arizona->dev);
> drivers/mfd/arizona-core.c- if (ret != 0)
> 
> drivers/remoteproc/qcom_q6v5_adsp.c:ret = pm_runtime_get_sync(adsp->dev);
> drivers/remoteproc/qcom_q6v5_adsp.c-if (ret)
> 
> drivers/spi/spi-img-spfi.c: ret = pm_runtime_get_sync(dev);
> drivers/spi/spi-img-spfi.c- if (ret)
> 
> drivers/usb/dwc3/dwc3-pci.c:ret = pm_runtime_get_sync(&dwc3->dev);
> drivers/usb/dwc3/dwc3-pci.c-if (ret)
> 
> drivers/watchdog/rti_wdt.c: ret = pm_runtime_get_sync(dev);
> drivers/watchdog/rti_wdt.c- if (ret) {
> 
> regards,
> dan carpenter
> 
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 99c7da112c95..e280991a977d 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1082,6 +1082,9 @@ int __pm_runtime_resume(struct device *dev, int 
> rpmflags)
>   retval = rpm_resume(dev, rpmflags);
>   spin_unlock_irqrestore(&dev->power.lock, flags);
>  
> + if (retval < 0 && rpmflags & RPM_GET_PUT)
> + atomic_dec(&dev->power.usage_count);
> +
>   return retval;
>  }
>  EXPORT_SYMBOL_GPL(__pm_runtime_resume);

Re: Re: [PATCH] wlcore: fix runtime pm imbalance in wl1271_op_suspend

[dinghao . liu]

There is a check against ret after out_sleep tag. If 
wl1271_configure_suspend_ap()
returns an error code, ret will be caught by this check and a warning will be 
issued.


"Tony Lindgren" <t...@atomide.com>写道：
> * Dinghao Liu  [200520 12:58]:
> > When wlcore_hw_interrupt_notify() returns an error code,
> > a pairing runtime PM usage counter decrement is needed to
> > keep the counter balanced.
> 
> We should probably keep the warning though, nothing will
> get shown for wl1271_configure_suspend_ap() errors.
> 
> Otherwise looks good to me.
> 
> Regards,
> 
> Tony

Re: Re: [PATCH] Input: omap-keypad - fix runtime pm imbalance on error

[dinghao . liu]

Fixing this in the PM core will influence all callers of pm_runtime_get_sync().
Therefore I think the better solution is to fix its misused callers.

Regards,
Dinghao

"Dmitry Torokhov" <dmitry.torok...@gmail.com>写道：
> Hi Dinghao,
> 
> On Wed, May 20, 2020 at 6:35 AM Dinghao Liu  wrote:
> >
> > pm_runtime_get_sync() increments the runtime PM usage counter even
> > the call returns an error code. Thus a pairing decrement is needed
> > on the error handling path to keep the counter balanced.
> 
> This is a very surprising behavior and I wonder if this should be
> fixed in the PM core (or the required cleanup steps need to be called
> out in the function description). I also see that a few drivers that
> handle this situation correctly (?) call pm_runtime_put_noidle()
> instead of pm_runtime_put_sync() in the error path.
> 
> Rafael, do you have any guidance here?
> 
> Thanks.
> 
> -- 
> Dmitry

Re: Re: [PATCH] hwrng: ks-sa - fix runtime pm imbalance on error

[dinghao . liu]

Hi Alexander,

There are large amounts of cases that assume pm_runtime_get_sync()
will modify runtime PM usage counter on error. Fixing this in PM 
subsystem will influence all callers of pm_runtime_get_sync() and
introduce new bugs. Therefore I think the better solution is to fix
misused cases individually.

Dinghao

> Hello Dinghao,
> 
> On Wed, 2020-05-20 at 21:29 +0800, Dinghao Liu wrote:
> > pm_runtime_get_sync() increments the runtime PM usage counter even
> > the call returns an error code. Thus a pairing decrement is needed
> > on the error handling path to keep the counter balanced.
> 
> I believe, this is the wrong place for such kind of fix.
> pm_runtime_get_sync() has obviously a broken semantics with regards to
> your observation but no other driver does what you propose.
> I think the proper fix belong into PM subsystem, please take a look
> onto commit 15bcb91d7e60 "PM / Runtime: Implement autosuspend support".
> 
> > Signed-off-by: Dinghao Liu 
> > ---
> >  drivers/char/hw_random/ks-sa-rng.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/char/hw_random/ks-sa-rng.c 
> > b/drivers/char/hw_random/ks-sa-rng.c
> > index e2330e757f1f..85c81da4a8af 100644
> > --- a/drivers/char/hw_random/ks-sa-rng.c
> > +++ b/drivers/char/hw_random/ks-sa-rng.c
> > @@ -244,6 +244,7 @@ static int ks_sa_rng_probe(struct platform_device *pdev)
> > ret = pm_runtime_get_sync(dev);
> > if (ret < 0) {
> > dev_err(dev, "Failed to enable SA power-domain\n");
> > +   pm_runtime_put_sync(dev);
> > pm_runtime_disable(dev);
> > return ret;
> > }
> -- 
> Alexander Sverdlin.
> 

[PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---

Changelog:

v2: - Remove unused label 'unlock'
---
 drivers/staging/media/tegra-vde/vde.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c 
b/drivers/staging/media/tegra-vde/vde.c
index d3e63512a765..3fdf2cd0b99e 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -777,7 +777,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
 
ret = pm_runtime_get_sync(dev);
if (ret < 0)
-   goto unlock;
+   goto put_runtime_pm;
 
/*
 * We rely on the VDE registers reset value, otherwise VDE
@@ -843,8 +843,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
 put_runtime_pm:
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
-
-unlock:
mutex_unlock(&vde->lock);
 
 release_dpb_frames:
-- 
2.17.1

Re: Re: [PATCH] drm/panfrost: fix runtime pm imbalance on error

[dinghao . liu]

Hi Steve,

There are two bailing out points in panfrost_job_hw_submit(): one is 
the error path beginning from pm_runtime_get_sync(), the other one is 
the error path beginning from WARN_ON() in the if statement. The pm 
imbalance fixed in this patch is between these two paths. I think the 
caller of panfrost_job_hw_submit() cannot distinguish this imbalance 
outside this function. 

panfrost_job_timedout() calls pm_runtime_put_noidle() for every job it 
finds, but all jobs are added to the pfdev->jobs just before calling
panfrost_job_hw_submit(). Therefore I think the imbalance still exists.
But I'm not very sure if we should add pm_runtime_put on the error path
after pm_runtime_get_sync(), or remove pm_runtime_put one the error path
after WARN_ON(). 

As for the problem about panfrost_devfreq_record_busy(), this may be a 
new bug and requires independent patch to fix it.

Regards,
Dinghao


> On 20/05/2020 12:05, Dinghao Liu wrote:
> > pm_runtime_get_sync() increments the runtime PM usage counter even
> > the call returns an error code. Thus a pairing decrement is needed
> > on the error handling path to keep the counter balanced.
> > 
> > Signed-off-by: Dinghao Liu 
> 
> Actually I think we have the opposite problem. To be honest we don't 
> handle this situation very well. By the time panfrost_job_hw_submit() is 
> called the job has already been added to the pfdev->jobs array, so it's 
> considered submitted even if it never actually lands on the hardware. So 
> in the case of this function bailing out early we will then (eventually) 
> hit a timeout and trigger a GPU reset.
> 
> panfrost_job_timedout() iterates through the pfdev->jobs array and calls 
> pm_runtime_put_noidle() for each job it finds. So there's no inbalance 
> here that I can see.
> 
> Have you actually observed the situation where pm_runtime_get_sync() 
> returns a failure?
> 
> HOWEVER, it appears that by bailing out early the call to 
> panfrost_devfreq_record_busy() is never made, which as far as I can see 
> means that there may be an extra call to panfrost_devfreq_record_idle() 
> when the jobs have timed out. Which could underflow the counter.
> 
> But equally looking at panfrost_job_timedout(), we only call 
> panfrost_devfreq_record_idle() *once* even though multiple jobs might be 
> processed.
> 
> There's a completely untested patch below which in theory should fix that...
> 
> Steve
> 
> 8<---
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
> b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 7914b1570841..f9519afca29d 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -145,6 +145,8 @@ static void panfrost_job_hw_submit(struct 
> panfrost_job *job, int js)
>   u64 jc_head = job->jc;
>   int ret;
> 
> + panfrost_devfreq_record_busy(pfdev);
> +
>   ret = pm_runtime_get_sync(pfdev->dev);
>   if (ret < 0)
>   return;
> @@ -155,7 +157,6 @@ static void panfrost_job_hw_submit(struct 
> panfrost_job *job, int js)
>   }
> 
>   cfg = panfrost_mmu_as_get(pfdev, &job->file_priv->mmu);
> - panfrost_devfreq_record_busy(pfdev);
> 
>   job_write(pfdev, JS_HEAD_NEXT_LO(js), jc_head & 0x);
>   job_write(pfdev, JS_HEAD_NEXT_HI(js), jc_head >> 32);
> @@ -410,12 +411,12 @@ static void panfrost_job_timedout(struct 
> drm_sched_job *sched_job)
>   for (i = 0; i < NUM_JOB_SLOTS; i++) {
>   if (pfdev->jobs[i]) {
>   pm_runtime_put_noidle(pfdev->dev);
> + panfrost_devfreq_record_idle(pfdev);
>   pfdev->jobs[i] = NULL;
>   }
>   }
>   spin_unlock_irqrestore(&pfdev->js->job_lock, flags);
> 
> - panfrost_devfreq_record_idle(pfdev);
>   panfrost_device_reset(pfdev);
> 
>   for (i = 0; i < NUM_JOB_SLOTS; i++)

[PATCH] i2c: stm32f7: Fix runtime PM imbalance in stm32f7_i2c_reg_slave

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/i2c/busses/i2c-stm32f7.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 330ffed011e0..602cf35649c8 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1767,8 +1767,10 @@ static int stm32f7_i2c_reg_slave(struct i2c_client 
*slave)
return ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(dev);
return ret;
+   }
 
if (!stm32f7_i2c_is_slave_registered(i2c_dev))
stm32f7_i2c_enable_wakeup(i2c_dev, true);
-- 
2.17.1

[PATCH] i2c: stm32f7: Fix runtime PM imbalance in stm32f7_i2c_unreg_slave

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/i2c/busses/i2c-stm32f7.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 330ffed011e0..6f5f0fa68385 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1837,8 +1837,10 @@ static int stm32f7_i2c_unreg_slave(struct i2c_client 
*slave)
WARN_ON(!i2c_dev->slave[id]);
 
ret = pm_runtime_get_sync(i2c_dev->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(i2c_dev->dev);
return ret;
+   }
 
if (id == 0) {
mask = STM32F7_I2C_OAR1_OA1EN;
-- 
2.17.1

[PATCH] xhci: Fix runtime PM imbalance on error

[Dinghao Liu]

When dma_set_mask_and_coherent() returns an error code,
a pairing runtime PM usage counter decrement is needed
to keep the counter balanced.

Also, call pm_runtime_disable() when dma_set_mask_and_coherent()
returns an error code.

Signed-off-by: Dinghao Liu 
---
 drivers/usb/host/xhci-histb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
index 5546e7e013a8..1d22a664477f 100644
--- a/drivers/usb/host/xhci-histb.c
+++ b/drivers/usb/host/xhci-histb.c
@@ -240,7 +240,7 @@ static int xhci_histb_probe(struct platform_device *pdev)
/* Initialize dma_mask and coherent_dma_mask to 32-bits */
ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
if (ret)
-   return ret;
+   goto disable_pm
 
hcd = usb_create_hcd(driver, dev, dev_name(dev));
if (!hcd) {
-- 
2.17.1

[PATCH] usb: musb: Fix runtime PM imbalance on error

[Dinghao Liu]

When copy_from_user() returns an error code, a pairing
runtime PM usage counter decrement is needed to keep
the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/usb/musb/musb_debugfs.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
index 7b6281ab62ed..837c38a5e4ef 100644
--- a/drivers/usb/musb/musb_debugfs.c
+++ b/drivers/usb/musb/musb_debugfs.c
@@ -178,8 +178,11 @@ static ssize_t musb_test_mode_write(struct file *file,
 
memset(buf, 0x00, sizeof(buf));
 
-   if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
+   if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) {
+   pm_runtime_mark_last_busy(musb->controller);
+   pm_runtime_put_autosuspend(musb->controller);
return -EFAULT;
+   }
 
if (strstarts(buf, "force host full-speed"))
test = MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_FS;
-- 
2.17.1

[PATCH] usb: cdns3: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/usb/cdns3/cdns3-ti.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/cdns3/cdns3-ti.c b/drivers/usb/cdns3/cdns3-ti.c
index 5685ba11480b..e701ab56b0a7 100644
--- a/drivers/usb/cdns3/cdns3-ti.c
+++ b/drivers/usb/cdns3/cdns3-ti.c
@@ -138,7 +138,7 @@ static int cdns_ti_probe(struct platform_device *pdev)
error = pm_runtime_get_sync(dev);
if (error < 0) {
dev_err(dev, "pm_runtime_get_sync failed: %d\n", error);
-   goto err_get;
+   goto err;
}
 
/* assert RESET */
@@ -185,7 +185,6 @@ static int cdns_ti_probe(struct platform_device *pdev)
 
 err:
pm_runtime_put_sync(data->dev);
-err_get:
pm_runtime_disable(data->dev);
 
return error;
-- 
2.17.1

[PATCH] spi: spi-ti-qspi: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/spi/spi-ti-qspi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 366a3e5cca6b..6d24cff2693c 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -175,6 +175,7 @@ static int ti_qspi_setup(struct spi_device *spi)
ret = pm_runtime_get_sync(qspi->dev);
if (ret < 0) {
dev_err(qspi->dev, "pm_runtime_get_sync() failed\n");
+   pm_runtime_put_autosuspend(qspi->dev);
return ret;
}
 
-- 
2.17.1

[PATCH] spi: tegra20-slink: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/spi/spi-tegra20-slink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index 7f4d932dade7..15361db00982 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -1118,6 +1118,7 @@ static int tegra_slink_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0) {
dev_err(&pdev->dev, "pm runtime get failed, e = %d\n", ret);
+   pm_runtime_put(&pdev->dev);
goto exit_pm_disable;
}
tspi->def_command_reg  = SLINK_M_S;
-- 
2.17.1

[PATCH] spi: tegra20-sflash: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/spi/spi-tegra20-sflash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-tegra20-sflash.c b/drivers/spi/spi-tegra20-sflash.c
index 514429379206..159efbc55a53 100644
--- a/drivers/spi/spi-tegra20-sflash.c
+++ b/drivers/spi/spi-tegra20-sflash.c
@@ -491,6 +491,7 @@ static int tegra_sflash_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0) {
dev_err(&pdev->dev, "pm runtime get failed, e = %d\n", ret);
+   pm_runtime_put(&pdev->dev);
goto exit_pm_disable;
}
 
-- 
2.17.1

[PATCH] spi: tegra114: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/spi/spi-tegra114.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index 83edabdb41ad..5b3faa29f945 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -1398,6 +1398,7 @@ static int tegra_spi_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0) {
dev_err(&pdev->dev, "pm runtime get failed, e = %d\n", ret);
+   pm_runtime_put(&pdev->dev);
goto exit_pm_disable;
}
 
-- 
2.17.1

[PATCH] watchdog: Fix runtime PM imbalance on error

[Dinghao Liu]

When watchdog_register_device() returns an error code,
a pairing runtime PM usage counter decrement is needed
to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/watchdog/omap_wdt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 9b91882fe3c4..1616f93dfad7 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -273,6 +273,7 @@ static int omap_wdt_probe(struct platform_device *pdev)
 
ret = watchdog_register_device(&wdev->wdog);
if (ret) {
+   pm_runtime_put(wdev->dev);
pm_runtime_disable(wdev->dev);
return ret;
}
-- 
2.17.1

[PATCH] dmaengine: tegra210-adma: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/dma/tegra210-adma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index c4ce5dfb149b..8f5e4949d720 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -870,7 +870,7 @@ static int tegra_adma_probe(struct platform_device *pdev)
 
ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0)
-   goto rpm_disable;
+   goto rpm_put;
 
ret = tegra_adma_init(tdma);
if (ret)
@@ -921,7 +921,6 @@ static int tegra_adma_probe(struct platform_device *pdev)
dma_async_device_unregister(&tdma->dma_dev);
 rpm_put:
pm_runtime_put_sync(&pdev->dev);
-rpm_disable:
pm_runtime_disable(&pdev->dev);
 irq_dispose:
while (--i >= 0)
-- 
2.17.1

Re: Re: [PATCH] spi: tegra20-slink: Fix runtime PM imbalance on error

[dinghao . liu]

Hi Andy,

Thank you for your advice! I will fix the problem in the next edition
of patch. The coccinelle script will be very helpful and I'm looking 
forward to it.

Regards,
Dinghao 

"Andy Shevchenko" <andy.shevche...@gmail.com>写道：
> On Thu, May 21, 2020 at 11:04 AM Andy Shevchenko
>  wrote:
> > On Thu, May 21, 2020 at 10:50 AM Dinghao Liu  wrote:
> 
> Any I have coccinelle script for this, I can share with you.
> 
> -- 
> With Best Regards,
> Andy Shevchenko

[PATCH] dmaengine: ti: edma: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/dma/ti/edma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
index c4a5c170c1f9..609ce2607eb7 100644
--- a/drivers/dma/ti/edma.c
+++ b/drivers/dma/ti/edma.c
@@ -2402,8 +2402,7 @@ static int edma_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
dev_err(dev, "pm_runtime_get_sync() failed\n");
-   pm_runtime_disable(dev);
-   return ret;
+   goto err_disable_pm;
}
 
/* Get eDMA3 configuration from IP */
-- 
2.17.1

[PATCH] sata_rcar: Fix runtime PM imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu 
---
 drivers/ata/sata_rcar.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 980aacdbcf3b..cfa0e732dfd0 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -907,7 +907,7 @@ static int sata_rcar_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0)
-   goto err_pm_disable;
+   goto err_pm_put;
 
host = ata_host_alloc(dev, 1);
if (!host) {
@@ -937,7 +937,6 @@ static int sata_rcar_probe(struct platform_device *pdev)
 
 err_pm_put:
pm_runtime_put(dev);
-err_pm_disable:
pm_runtime_disable(dev);
return ret;
 }
-- 
2.17.1