[PATCH] btrfs: check for BTRFS_FS_ERROR in pending ordered assert

[PATCH] btrfs: check for BTRFS_FS_ERROR in pending ordered assert

[Josef Bacik]

If we do fast tree logging we increment a counter on the current
transaction for every ordered extent we need to wait for.  This means we
expect the transaction to still be there when we clear pending on the
ordered extent.  However if we happen to abort the transaction and clean
it up, there could be no running transaction, and thus we'll trip the

ASSERT(trans)

check.  This is obviously incorrect, and the code properly deals with
the case that the trans doesn't exist.  Fix this ASSERT() to only fire
if there's no trans and we don't have BTRFS_FS_ERROR() set on the file
system.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ordered-data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 09b274d9ba18..69a2cb50c197 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -659,7 +659,7 @@ void btrfs_remove_ordered_extent(struct btrfs_inode *btrfs_inode,
 			refcount_inc(&trans->use_count);
 		spin_unlock(&fs_info->trans_lock);
 
-		ASSERT(trans);
+		ASSERT(trans || BTRFS_FS_ERROR(fs_info));
 		if (trans) {
 			if (atomic_dec_and_test(&trans->pending_ordered))
 				wake_up(&trans->pending_wait);
-- 
2.26.3

Re: [PATCH] btrfs: check for BTRFS_FS_ERROR in pending ordered assert

[Filipe Manana]

On Thu, Aug 24, 2023 at 04:59:04PM -0400, Josef Bacik wrote:
> If we do fast tree logging we increment a counter on the current
> transaction for every ordered extent we need to wait for.  This means we
> expect the transaction to still be there when we clear pending on the
> ordered extent.  However if we happen to abort the transaction and clean
> it up, there could be no running transaction, and thus we'll trip the
> 
> ASSERT(trans)
> 
> check.  This is obviously incorrect, and the code properly deals with
> the case that the trans doesn't exist.  Fix this ASSERT() to only fire
> if there's no trans and we don't have BTRFS_FS_ERROR() set on the file
> system.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, thanks.

> ---
>  fs/btrfs/ordered-data.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 09b274d9ba18..69a2cb50c197 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -659,7 +659,7 @@ void btrfs_remove_ordered_extent(struct btrfs_inode *btrfs_inode,
>  			refcount_inc(&trans->use_count);
>  		spin_unlock(&fs_info->trans_lock);
>  
> -		ASSERT(trans);
> +		ASSERT(trans || BTRFS_FS_ERROR(fs_info));
>  		if (trans) {
>  			if (atomic_dec_and_test(&trans->pending_ordered))
>  				wake_up(&trans->pending_wait);
> -- 
> 2.26.3
> 

Re: [PATCH] btrfs: check for BTRFS_FS_ERROR in pending ordered assert

[David Sterba]

On Thu, Aug 24, 2023 at 04:59:04PM -0400, Josef Bacik wrote:
> If we do fast tree logging we increment a counter on the current
> transaction for every ordered extent we need to wait for.  This means we
> expect the transaction to still be there when we clear pending on the
> ordered extent.  However if we happen to abort the transaction and clean
> it up, there could be no running transaction, and thus we'll trip the
> 
> ASSERT(trans)
> 
> check.  This is obviously incorrect, and the code properly deals with
> the case that the trans doesn't exist.  Fix this ASSERT() to only fire
> if there's no trans and we don't have BTRFS_FS_ERROR() set on the file
> system.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Added to misc-next, thanks.