(this patch was already merged to stable last window, I was late on
review -- this is more feedback for master that has the same code)
Greg Kroah-Hartman wrote on Sun, Sep 01, 2024 at 06:17:04PM +0200:
> From: David Sterba <dsterba@xxxxxxxx>
>
> [ Upstream commit be73f4448b607e6b7ce41cd8ef2214fdf6e7986f ]
>
> The pointer to root is initialized in btrfs_init_delayed_node(), no need
> to check for it again. Change the BUG_ON to assertion.
>
> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Reviewed-by: Anand Jain <anand.jain@xxxxxxxxxx>
> Signed-off-by: David Sterba <dsterba@xxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
> fs/btrfs/delayed-inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index cdfc791b3c405..e2afaa70ae5e5 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -986,7 +986,7 @@ static void btrfs_release_delayed_inode(struct btrfs_delayed_node *delayed_node)
>
> if (delayed_node &&
> test_bit(BTRFS_DELAYED_NODE_INODE_DIRTY, &delayed_node->flags)) {
> - BUG_ON(!delayed_node->root);
> + ASSERT(delayed_node->root);
Note that if !delayed_node->root we will bug on a null deref immediately anyway.
This patch was missing just one line of context, I've added it below:
> clear_bit(BTRFS_DELAYED_NODE_INODE_DIRTY, &delayed_node->flags);
> delayed_node->count--;
>
> delayed_root = delayed_node->root->fs_info->delayed_root;
So, ASSERT doesn't feel appropriate -- if you want to protect
non-developers from BUGs then the check needs to be spelled out and
return early or not enter the if; or it really cannot happen (it used to
be a BUG...) and the check can be removed altogether.
--
Dominique Martinet | Asmadeus
