* [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
* 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
* [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 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