* [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction
@ 2023-07-21 9:49 fdmanana
2023-07-21 9:49 ` [PATCH 1/2] btrfs: check if the transaction was aborted at btrfs_wait_for_commit() fdmanana
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: fdmanana @ 2023-07-21 9:49 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
These fix missing error checks and returns when attaching to a transaction
in barrier mode, and waiting for a transaction to commit and finish. These
can make things like an fsync that fallbacks to a transaction commit to
report success to user space when a transaction was aborted and the inode
changes were therefore not persisted. More details on the change logs.
Filipe Manana (2):
btrfs: check if the transaction was aborted at btrfs_wait_for_commit()
btrfs: check for commit error at btrfs_attach_transaction_barrier()
fs/btrfs/transaction.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] btrfs: check if the transaction was aborted at btrfs_wait_for_commit()
2023-07-21 9:49 [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction fdmanana
@ 2023-07-21 9:49 ` fdmanana
2023-07-21 10:18 ` Qu Wenruo
2023-07-21 9:49 ` [PATCH 2/2] btrfs: check for commit error at btrfs_attach_transaction_barrier() fdmanana
2023-07-21 23:17 ` [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction David Sterba
2 siblings, 1 reply; 6+ messages in thread
From: fdmanana @ 2023-07-21 9:49 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At btrfs_wait_for_commit() we wait for a transaction to finish and then
always return 0 (success) without checking if it was aborted, in which
case the transaction didn't happen due to some critical error. Fix this
by checking if the transaction was aborted.
Fixes: 462045928bda ("Btrfs: add START_SYNC, WAIT_SYNC ioctls")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/transaction.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 4743882fa94b..8ab85465cdaa 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -931,6 +931,7 @@ int btrfs_wait_for_commit(struct btrfs_fs_info *fs_info, u64 transid)
}
wait_for_commit(cur_trans, TRANS_STATE_COMPLETED);
+ ret = cur_trans->aborted;
btrfs_put_transaction(cur_trans);
out:
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] btrfs: check for commit error at btrfs_attach_transaction_barrier()
2023-07-21 9:49 [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction fdmanana
2023-07-21 9:49 ` [PATCH 1/2] btrfs: check if the transaction was aborted at btrfs_wait_for_commit() fdmanana
@ 2023-07-21 9:49 ` fdmanana
2023-07-21 10:20 ` Qu Wenruo
2023-07-21 23:17 ` [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction David Sterba
2 siblings, 1 reply; 6+ messages in thread
From: fdmanana @ 2023-07-21 9:49 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
btrfs_attach_transaction_barrier() is used to get a handle pointing to the
current running transaction if the transaction has not started its commit
yet (its state is < TRANS_STATE_COMMIT_START). If the transaction commit
has started, then we wait for the transaction to commit and finish before
returning - however we completely ignore if the transaction was aborted
due to some error during its commit, we simply return ERR_PT(-ENOENT),
which makes the caller assume everything is fine and no errors happened.
This could make an fsync return success (0) to user space when in fact we
had a transaction abort and the target inode changes were therefore not
persisted.
Fix this by checking for the return value from btrfs_wait_for_commit(),
and if it returned an error, return it back to the caller.
Fixes: d4edf39bd5db ("Btrfs: fix uncompleted transaction")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/transaction.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 8ab85465cdaa..4bb9716ad24a 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -826,8 +826,13 @@ btrfs_attach_transaction_barrier(struct btrfs_root *root)
trans = start_transaction(root, 0, TRANS_ATTACH,
BTRFS_RESERVE_NO_FLUSH, true);
- if (trans == ERR_PTR(-ENOENT))
- btrfs_wait_for_commit(root->fs_info, 0);
+ if (trans == ERR_PTR(-ENOENT)) {
+ int ret;
+
+ ret = btrfs_wait_for_commit(root->fs_info, 0);
+ if (ret)
+ return ERR_PTR(ret);
+ }
return trans;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] btrfs: check if the transaction was aborted at btrfs_wait_for_commit()
2023-07-21 9:49 ` [PATCH 1/2] btrfs: check if the transaction was aborted at btrfs_wait_for_commit() fdmanana
@ 2023-07-21 10:18 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2023-07-21 10:18 UTC (permalink / raw)
To: fdmanana, linux-btrfs
On 2023/7/21 17:49, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> At btrfs_wait_for_commit() we wait for a transaction to finish and then
> always return 0 (success) without checking if it was aborted, in which
> case the transaction didn't happen due to some critical error. Fix this
> by checking if the transaction was aborted.
>
> Fixes: 462045928bda ("Btrfs: add START_SYNC, WAIT_SYNC ioctls")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Thanks,
Qu
> ---
> fs/btrfs/transaction.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 4743882fa94b..8ab85465cdaa 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -931,6 +931,7 @@ int btrfs_wait_for_commit(struct btrfs_fs_info *fs_info, u64 transid)
> }
>
> wait_for_commit(cur_trans, TRANS_STATE_COMPLETED);
> + ret = cur_trans->aborted;
> btrfs_put_transaction(cur_trans);
> out:
> return ret;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: check for commit error at btrfs_attach_transaction_barrier()
2023-07-21 9:49 ` [PATCH 2/2] btrfs: check for commit error at btrfs_attach_transaction_barrier() fdmanana
@ 2023-07-21 10:20 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2023-07-21 10:20 UTC (permalink / raw)
To: fdmanana, linux-btrfs
On 2023/7/21 17:49, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> btrfs_attach_transaction_barrier() is used to get a handle pointing to the
> current running transaction if the transaction has not started its commit
> yet (its state is < TRANS_STATE_COMMIT_START). If the transaction commit
> has started, then we wait for the transaction to commit and finish before
> returning - however we completely ignore if the transaction was aborted
> due to some error during its commit, we simply return ERR_PT(-ENOENT),
> which makes the caller assume everything is fine and no errors happened.
>
> This could make an fsync return success (0) to user space when in fact we
> had a transaction abort and the target inode changes were therefore not
> persisted.
>
> Fix this by checking for the return value from btrfs_wait_for_commit(),
> and if it returned an error, return it back to the caller.
>
> Fixes: d4edf39bd5db ("Btrfs: fix uncompleted transaction")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Thanks,
Qu
> ---
> fs/btrfs/transaction.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 8ab85465cdaa..4bb9716ad24a 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -826,8 +826,13 @@ btrfs_attach_transaction_barrier(struct btrfs_root *root)
>
> trans = start_transaction(root, 0, TRANS_ATTACH,
> BTRFS_RESERVE_NO_FLUSH, true);
> - if (trans == ERR_PTR(-ENOENT))
> - btrfs_wait_for_commit(root->fs_info, 0);
> + if (trans == ERR_PTR(-ENOENT)) {
> + int ret;
> +
> + ret = btrfs_wait_for_commit(root->fs_info, 0);
> + if (ret)
> + return ERR_PTR(ret);
> + }
>
> return trans;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction
2023-07-21 9:49 [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction fdmanana
2023-07-21 9:49 ` [PATCH 1/2] btrfs: check if the transaction was aborted at btrfs_wait_for_commit() fdmanana
2023-07-21 9:49 ` [PATCH 2/2] btrfs: check for commit error at btrfs_attach_transaction_barrier() fdmanana
@ 2023-07-21 23:17 ` David Sterba
2 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2023-07-21 23:17 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Fri, Jul 21, 2023 at 10:49:19AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> These fix missing error checks and returns when attaching to a transaction
> in barrier mode, and waiting for a transaction to commit and finish. These
> can make things like an fsync that fallbacks to a transaction commit to
> report success to user space when a transaction was aborted and the inode
> changes were therefore not persisted. More details on the change logs.
>
> Filipe Manana (2):
> btrfs: check if the transaction was aborted at btrfs_wait_for_commit()
> btrfs: check for commit error at btrfs_attach_transaction_barrier()
Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-21 23:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-21 9:49 [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction fdmanana
2023-07-21 9:49 ` [PATCH 1/2] btrfs: check if the transaction was aborted at btrfs_wait_for_commit() fdmanana
2023-07-21 10:18 ` Qu Wenruo
2023-07-21 9:49 ` [PATCH 2/2] btrfs: check for commit error at btrfs_attach_transaction_barrier() fdmanana
2023-07-21 10:20 ` Qu Wenruo
2023-07-21 23:17 ` [PATCH 0/2] btrfs: fixes for missing error reporting when attaching to a transaction David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox