| Message ID | c640ee0669c4454488d2ddacbc3a93884c905b38.1692910732.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: check for BTRFS_FS_ERROR in pending ordered assert | expand |
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 >
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.
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);
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(-)