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

[Greg KH <gregkh@linuxfoundation.org>]

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