[PATCH] driver:fix a bug of function remove_nodes in "drivers/base/devres.c"

[PATCH] driver:fix a bug of function remove_nodes in "drivers/base/devres.c"

[Changhang Luo]

The resource management mechanism in Linux divides resources 
into different groups for easier management. Each closed group
has a start and an close resource node to represent the scope
of the resource group. The function "remove_nodes" is try to move
the nodes in the same group into the todo list. Fisrt the function
moves the normal nodes into the todo list, then scan the left node
and color the group value of two if the start and close resource
nodes contained in [current node, end).

While, in the second pass section here is:
  if (list_empty(&grp->node[1].entry))
     grp->color++;
which means that the color is set to 2 when the grp_node[1] is not
in the list.This situation is inconsistent with the original design
intention that color value is set to 2 if the start and close nodes
 both contained in [current node, end).

So it should be
  if (!list_empty(&grp->node[1].entry))
    grep->color++;
that the grep->node[1] is verified to be contained 
[current node, end) and then color is increased to 2.

Finally,
  if(grp->color ==2){
    list_move_tail(&grp->node[0].entry,tode);
    list_del_init(&grp->node[1].entry);
  }
the closed group can be removed currectly.

then
  list_del_init(grep->node[1].entry);
while

Signed-off-by: Changhang Luo <luochanghang_nudt@163.com>
---
 drivers/base/devres.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index d8a733ea5..68f64256a 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -477,7 +477,7 @@ static int remove_nodes(struct device *dev,
 		BUG_ON(!grp || list_empty(&grp->node[0].entry));
 
 		grp->color++;
-		if (list_empty(&grp->node[1].entry))
+		if (!list_empty(&grp->node[1].entry))
 			grp->color++;
 
 		BUG_ON(grp->color <= 0 || grp->color > 2);
-- 
2.25.1

Re: [PATCH] driver:fix a bug of function remove_nodes in "drivers/base/devres.c"

[Greg KH]

On Sat, May 03, 2025 at 04:04:54PM +0800, Changhang Luo wrote:
> The resource management mechanism in Linux divides resources 
> into different groups for easier management. Each closed group
> has a start and an close resource node to represent the scope
> of the resource group. The function "remove_nodes" is try to move
> the nodes in the same group into the todo list. Fisrt the function
> moves the normal nodes into the todo list, then scan the left node
> and color the group value of two if the start and close resource
> nodes contained in [current node, end).
> 
> While, in the second pass section here is:
>   if (list_empty(&grp->node[1].entry))
>      grp->color++;
> which means that the color is set to 2 when the grp_node[1] is not
> in the list.This situation is inconsistent with the original design
> intention that color value is set to 2 if the start and close nodes
>  both contained in [current node, end).
> 
> So it should be
>   if (!list_empty(&grp->node[1].entry))
>     grep->color++;
> that the grep->node[1] is verified to be contained 
> [current node, end) and then color is increased to 2.
> 
> Finally,
>   if(grp->color ==2){
>     list_move_tail(&grp->node[0].entry,tode);
>     list_del_init(&grp->node[1].entry);
>   }
> the closed group can be removed currectly.
> 
> then
>   list_del_init(grep->node[1].entry);
> while
> 
> Signed-off-by: Changhang Luo <luochanghang_nudt@163.com>
> ---
>  drivers/base/devres.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index d8a733ea5..68f64256a 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -477,7 +477,7 @@ static int remove_nodes(struct device *dev,
>  		BUG_ON(!grp || list_empty(&grp->node[0].entry));
>  
>  		grp->color++;
> -		if (list_empty(&grp->node[1].entry))
> +		if (!list_empty(&grp->node[1].entry))
>  			grp->color++;

Are you sure about this?  How was this tested?  Why hasn't it shown up
before?  And what commit id does this "fix"?

thanks,

greg k-h