[PATCH 0/2] fuse: fixes for remote locking

[PATCH 0/2] fuse: fixes for remote locking

[Jiachen Zhang]

Hi all,

This patchset fixes some small issues of fuse remoting locking.

The first patch fixes the missing automatic unlock of fcntl(2) OFD lock.
The second patch remove some deadcode for clearness.

Thanks,
Jiachen

Jiachen Zhang (2):
  fuse: support unlock remote OFD locks on file release
  fuse: remove an unnecessary if statement

 fs/fuse/file.c   | 21 ++++++++++++++-------
 fs/fuse/fuse_i.h |  2 +-
 2 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.20.1

[PATCH 1/2] fuse: support unlock remote OFD locks on file release

[Jiachen Zhang]

Like flock(2), the fcntl(2) OFD locks also use struct file addresses as
the lock owner ID, and also should be unlocked on file release.

The commit 37fb3a30b462 ("fuse: fix flock") fixed the flock unlocking
issue on file release. This commit aims to fix the OFD lock by reusing
the release_flag 'FUSE_RELEASE_FLOCK_UNLOCK'. The FUSE daemons should
unlock both OFD locks and flocks in the FUSE_RELEASE handler.

To make it more clear, rename 'ff->flock' to 'ff->unlock_on_release', as
it would be used for both flock and OFD lock. It will be set true if the
value of fl->fl_owner equals to the struct file address.

Fixes: 37fb3a30b462 ("fuse: fix flock")
Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
---
 fs/fuse/file.c   | 17 ++++++++++++++---
 fs/fuse/fuse_i.h |  2 +-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index de37a3a06a71..7fe9d405969e 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -312,7 +312,7 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 
 	fuse_prepare_release(fi, ff, open_flags, opcode);
 
-	if (ff->flock) {
+	if (ff->unlock_on_release) {
 		ra->inarg.release_flags |= FUSE_RELEASE_FLOCK_UNLOCK;
 		ra->inarg.lock_owner = fuse_lock_owner_id(ff->fm->fc, id);
 	}
@@ -2650,8 +2650,19 @@ static int fuse_file_lock(struct file *file, int cmd, struct file_lock *fl)
 	} else {
 		if (fc->no_lock)
 			err = posix_lock_file(file, fl, NULL);
-		else
+		else {
+			/*
+			 * Like flock, the OFD lock also uses the struct
+			 * file address as the fl_owner, and should be
+			 * unlocked on file release.
+			 */
+			if (file == fl->fl_owner) {
+				struct fuse_file *ff = file->private_data;
+
+				ff->unlock_on_release = true;
+			}
 			err = fuse_setlk(file, fl, 0);
+		}
 	}
 	return err;
 }
@@ -2668,7 +2679,7 @@ static int fuse_file_flock(struct file *file, int cmd, struct file_lock *fl)
 		struct fuse_file *ff = file->private_data;
 
 		/* emulate flock with POSIX locks */
-		ff->flock = true;
+		ff->unlock_on_release = true;
 		err = fuse_setlk(file, fl, 1);
 	}
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 9b7fc7d3c7f1..574f67bd5684 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -225,7 +225,7 @@ struct fuse_file {
 	wait_queue_head_t poll_wait;
 
 	/** Has flock been performed on this file? */
-	bool flock:1;
+	bool unlock_on_release:1;
 };
 
 /** One input argument of a request */
-- 
2.20.1

[PATCH 2/2] fuse: remove an unnecessary if statement

[Jiachen Zhang]

FUSE remote locking code paths never add any locking state to
inode->i_flctx, so the locks_remove_posix() function called on
file close will return without calling fuse_setlk().

Therefore, as the if statement to be removed in this commit will
always be false, remove it for clearness.

Fixes: 7142125937e1 ("[PATCH] fuse: add POSIX file locking support")
Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
---
 fs/fuse/file.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 7fe9d405969e..57789215c666 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2619,10 +2619,6 @@ static int fuse_setlk(struct file *file, struct file_lock *fl, int flock)
 		return -ENOLCK;
 	}
 
-	/* Unlock on close is handled by the flush method */
-	if ((fl->fl_flags & FL_CLOSE_POSIX) == FL_CLOSE_POSIX)
-		return 0;
-
 	fuse_lk_fill(&args, file, fl, opcode, pid_nr, flock, &inarg);
 	err = fuse_simple_request(fm, &args);
 
-- 
2.20.1

Re: [PATCH 1/2] fuse: support unlock remote OFD locks on file release

[Jochen Friedrich]

Tested-By: Jochen Friedrich <jochen@scram.de>

On an unpatched Kernel, running Volume migrations in QEMU on a fuse 
backed file (linke Quobyte) fails with 'Failed to get "consistent read" 
lock' error messages.
With this patch applied, all works well :-)

Best regards, Jochen

Am 08.06.2023 um 10:46 schrieb Jiachen Zhang:
> Like flock(2), the fcntl(2) OFD locks also use struct file addresses as
> the lock owner ID, and also should be unlocked on file release.
>
> The commit 37fb3a30b462 ("fuse: fix flock") fixed the flock unlocking
> issue on file release. This commit aims to fix the OFD lock by reusing
> the release_flag 'FUSE_RELEASE_FLOCK_UNLOCK'. The FUSE daemons should
> unlock both OFD locks and flocks in the FUSE_RELEASE handler.
>
> To make it more clear, rename 'ff->flock' to 'ff->unlock_on_release', as
> it would be used for both flock and OFD lock. It will be set true if the
> value of fl->fl_owner equals to the struct file address.
>
> Fixes: 37fb3a30b462 ("fuse: fix flock")
> Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
> ---
>   fs/fuse/file.c   | 17 ++++++++++++++---
>   fs/fuse/fuse_i.h |  2 +-
>   2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index de37a3a06a71..7fe9d405969e 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -312,7 +312,7 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
>   
>   	fuse_prepare_release(fi, ff, open_flags, opcode);
>   
> -	if (ff->flock) {
> +	if (ff->unlock_on_release) {
>   		ra->inarg.release_flags |= FUSE_RELEASE_FLOCK_UNLOCK;
>   		ra->inarg.lock_owner = fuse_lock_owner_id(ff->fm->fc, id);
>   	}
> @@ -2650,8 +2650,19 @@ static int fuse_file_lock(struct file *file, int cmd, struct file_lock *fl)
>   	} else {
>   		if (fc->no_lock)
>   			err = posix_lock_file(file, fl, NULL);
> -		else
> +		else {
> +			/*
> +			 * Like flock, the OFD lock also uses the struct
> +			 * file address as the fl_owner, and should be
> +			 * unlocked on file release.
> +			 */
> +			if (file == fl->fl_owner) {
> +				struct fuse_file *ff = file->private_data;
> +
> +				ff->unlock_on_release = true;
> +			}
>   			err = fuse_setlk(file, fl, 0);
> +		}
>   	}
>   	return err;
>   }
> @@ -2668,7 +2679,7 @@ static int fuse_file_flock(struct file *file, int cmd, struct file_lock *fl)
>   		struct fuse_file *ff = file->private_data;
>   
>   		/* emulate flock with POSIX locks */
> -		ff->flock = true;
> +		ff->unlock_on_release = true;
>   		err = fuse_setlk(file, fl, 1);
>   	}
>   
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 9b7fc7d3c7f1..574f67bd5684 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -225,7 +225,7 @@ struct fuse_file {
>   	wait_queue_head_t poll_wait;
>   
>   	/** Has flock been performed on this file? */
> -	bool flock:1;
> +	bool unlock_on_release:1;
>   };
>   
>   /** One input argument of a request */
>
>  From patchwork Thu Jun  8 08:46:09 2023
> Content-Type: text/plain; charset="utf-8"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> X-Patchwork-Submitter: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
> X-Patchwork-Id: 13271765
> Return-Path: <linux-fsdevel-owner@vger.kernel.org>
> X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
> 	aws-us-west-2-korg-lkml-1.web.codeaurora.org
> Received: from vger.kernel.org (vger.kernel.org [23.128.96.18])
> 	by smtp.lore.kernel.org (Postfix) with ESMTP id 5194CC7EE25
> 	for <linux-fsdevel@archiver.kernel.org>;
>   Thu,  8 Jun 2023 08:47:37 +0000 (UTC)
> Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
>          id S235560AbjFHIrf (ORCPT
>          <rfc822;linux-fsdevel@archiver.kernel.org>);
>          Thu, 8 Jun 2023 04:47:35 -0400
> Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43358 "EHLO
>          lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
>          with ESMTP id S235284AbjFHIrc (ORCPT
>          <rfc822;linux-fsdevel@vger.kernel.org>);
>          Thu, 8 Jun 2023 04:47:32 -0400
> Received: from mail-pf1-x432.google.com (mail-pf1-x432.google.com
>   [IPv6:2607:f8b0:4864:20::432])
>          by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6B2062733
>          for <linux-fsdevel@vger.kernel.org>;
>   Thu,  8 Jun 2023 01:47:31 -0700 (PDT)
> Received: by mail-pf1-x432.google.com with SMTP id
>   d2e1a72fcca58-651f2f38634so284228b3a.0
>          for <linux-fsdevel@vger.kernel.org>;
>   Thu, 08 Jun 2023 01:47:31 -0700 (PDT)
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>          d=bytedance.com; s=google; t=1686214051; x=1688806051;
>          h=content-transfer-encoding:mime-version:references:in-reply-to
>           :message-id:date:subject:cc:to:from:from:to:cc:subject:date
>           :message-id:reply-to;
>          bh=M478tZ0BJPqFdShribkt4DG9LiqmYQZeG2OJxmtwTQI=;
>          b=Yq2Nya/66Vu5w6nNpTUv8LEnsCGR+JlJ895K/cvR0mlClcb2DLPGqhvMmAhPRnVC6P
>           5lyLBb0BanEOEB5t67nNrVRDwmL/nXyneeJKirBxtqfYoX82wSUEyObyvO5lBx5WlZjd
>           7IFu2/h9klmSHROrvtKaxHRzYYf97RrRZ7R6kUT/MptavElCUHxFZYVt8v39v0QUxqH8
>           MBekCo2B/5+W5SBtVF33Auv200I2Z82VjkUUpo4jXKJb6wedh1dfBUxH68MnoVoVRJcq
>           CehSyCf+cG+5K5kWw96LsHynVXZEpos6blrNXcKzRev92YSyY59ZKV4R2rqBlgxN/CeI
>           vyVw==
> X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>          d=1e100.net; s=20221208; t=1686214051; x=1688806051;
>          h=content-transfer-encoding:mime-version:references:in-reply-to
>           :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc
>           :subject:date:message-id:reply-to;
>          bh=M478tZ0BJPqFdShribkt4DG9LiqmYQZeG2OJxmtwTQI=;
>          b=IIdJ5BqHdko+u8iHJ25sRJlbsFEuTdUrBtxh2l6ZHZ1APaHEHOUIYEdUQk0/jb+fq8
>           FBlIf7K/TzO9LyYIshf+7CTZ2HhxfopgnafeaiMeUyID2x+G1Fc3wvAvPSvF7dZKXYJz
>           ebR+R0Prljz6lJHDAydvFYURpRfMT5E6Rnwp+CCofymPXlYmcWBgQwA+blAp50FZE4yw
>           DMCkSYNzyPbSpA77j4sK3F3ptNJOAdlUzf3w0zlKDX4a146eq+uocseNLb5SftdHWL2z
>           FVqlKkGRjuBws2ss1/Iqhhw1q8tPoIHB1yUicoLntg1ukKxFBLsbHIn5juBAcnz7fmQY
>           xw6w==
> X-Gm-Message-State: AC+VfDxt+ymlCMH+c9NbnO3d+FD0m+JQp5D2xU5rkCmmjrwYyh9vgl+6
>          JeUTJJz5Eg0m9YrHXMK4n1CSOQ==
> X-Google-Smtp-Source:
>   ACHHUZ7154zUvrdNtTgZVFuJ8WpQ3S+Vr8G9uLG4z30KZw1UZuEHkqXzo6u/4y2aW+LeH1v8GBnjxw==
> X-Received: by 2002:a05:6a20:7d85:b0:10d:d0cd:c1c7 with SMTP id
>   v5-20020a056a207d8500b0010dd0cdc1c7mr7184475pzj.15.1686214050909;
>          Thu, 08 Jun 2023 01:47:30 -0700 (PDT)
> Received: from localhost.localdomain ([61.213.176.13])
>          by smtp.gmail.com with ESMTPSA id
>   23-20020aa79157000000b0063b806b111csm614160pfi.169.2023.06.08.01.47.25
>          (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
>          Thu, 08 Jun 2023 01:47:30 -0700 (PDT)
> From: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
> To: Miklos Szeredi <miklos@szeredi.hu>, linux-fsdevel@vger.kernel.org,
>          linux-kernel@vger.kernel.org
> Cc: Andrew Morton <akpm@osdl.org>, me@jcix.top,
>          Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
> Subject: [PATCH 2/2] fuse: remove an unnecessary if statement
> Date: Thu,  8 Jun 2023 16:46:09 +0800
> Message-Id: <20230608084609.14245-3-zhangjiachen.jaycee@bytedance.com>
> X-Mailer: git-send-email 2.35.1
> In-Reply-To: <20230608084609.14245-1-zhangjiachen.jaycee@bytedance.com>
> References: <20230608084609.14245-1-zhangjiachen.jaycee@bytedance.com>
> MIME-Version: 1.0
> Precedence: bulk
> List-ID: <linux-fsdevel.vger.kernel.org>
> X-Mailing-List: linux-fsdevel@vger.kernel.org
>
> FUSE remote locking code paths never add any locking state to
> inode->i_flctx, so the locks_remove_posix() function called on
> file close will return without calling fuse_setlk().
>
> Therefore, as the if statement to be removed in this commit will
> always be false, remove it for clearness.
>
> Fixes: 7142125937e1 ("[PATCH] fuse: add POSIX file locking support")
> Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
> ---
>   fs/fuse/file.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 7fe9d405969e..57789215c666 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2619,10 +2619,6 @@ static int fuse_setlk(struct file *file, struct file_lock *fl, int flock)
>   		return -ENOLCK;
>   	}
>   
> -	/* Unlock on close is handled by the flush method */
> -	if ((fl->fl_flags & FL_CLOSE_POSIX) == FL_CLOSE_POSIX)
> -		return 0;
> -
>   	fuse_lk_fill(&args, file, fl, opcode, pid_nr, flock, &inarg);
>   	err = fuse_simple_request(fm, &args);
>   

Re: [PATCH 2/2] fuse: remove an unnecessary if statement

[Miklos Szeredi]

On Thu, 8 Jun 2023 at 10:47, Jiachen Zhang
<zhangjiachen.jaycee@bytedance.com> wrote:
>
> FUSE remote locking code paths never add any locking state to
> inode->i_flctx, so the locks_remove_posix() function called on
> file close will return without calling fuse_setlk().
>
> Therefore, as the if statement to be removed in this commit will
> always be false, remove it for clearness.
>
> Fixes: 7142125937e1 ("[PATCH] fuse: add POSIX file locking support")
> Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>

Applied, thanks.

Miklos