[PATCH] Btrfs: fix missing semaphore unlock

[PATCH] Btrfs: fix missing semaphore unlock

[robbieko]

From: Robbie Ko <robbieko@synology.com>

Fixes: aab15e8ec2576 ("Btrfs: fix rare chances for data loss when doing a fast fsync")
Signed-off-by: Robbie Ko <robbieko@synology.com>
---
 fs/btrfs/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index a16da274c9aa..ae903da21588 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -2124,6 +2124,7 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 	 */
 	ret = start_ordered_ops(inode, start, end);
 	if (ret) {
+		up_write(&BTRFS_I(inode)->dio_sem);
 		inode_unlock(inode);
 		goto out;
 	}
-- 
2.17.1

Re: [PATCH] Btrfs: fix missing semaphore unlock

[Filipe Manana]

On Tue, Mar 17, 2020 at 6:41 AM robbieko <robbieko@synology.com> wrote:
>
> From: Robbie Ko <robbieko@synology.com>
>
> Fixes: aab15e8ec2576 ("Btrfs: fix rare chances for data loss when doing a fast fsync")
> Signed-off-by: Robbie Ko <robbieko@synology.com>

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

Looks good, thanks.

> ---
>  fs/btrfs/file.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index a16da274c9aa..ae903da21588 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -2124,6 +2124,7 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
>          */
>         ret = start_ordered_ops(inode, start, end);
>         if (ret) {
> +               up_write(&BTRFS_I(inode)->dio_sem);
>                 inode_unlock(inode);
>                 goto out;
>         }
> --
> 2.17.1
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH] Btrfs: fix missing semaphore unlock

[David Sterba]

On Tue, Mar 17, 2020 at 02:31:02PM +0800, robbieko wrote:
> From: Robbie Ko <robbieko@synology.com>

This is not a trivial patch that could go without a changelog.

> Fixes: aab15e8ec2576 ("Btrfs: fix rare chances for data loss when doing a fast fsync")
> Signed-off-by: Robbie Ko <robbieko@synology.com>
> ---
>  fs/btrfs/file.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index a16da274c9aa..ae903da21588 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -2124,6 +2124,7 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
>  	 */
>  	ret = start_ordered_ops(inode, start, end);
>  	if (ret) {
> +		up_write(&BTRFS_I(inode)->dio_sem);

I did not spot on first sight that there's was missing semaphore unlock
and a few lines below there's down_write(dio_sem). Turns out there are
two calls to start_ordered_ops, one before dio_sem and one inside the
locked section. So I solved the puzzle but I'd prefer not having to and
get a patch with instructions.