2023-06-08 08:56:33

by Jiachen Zhang

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

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 <[email protected]>
---
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



2023-06-18 15:57:51

by Jochen Friedrich

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

Tested-By: Jochen Friedrich <[email protected]>

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 <[email protected]>
> ---
> 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 <[email protected]>
> X-Patchwork-Id: 13271765
> Return-Path: <[email protected]>
> 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 <[email protected]>;
> Thu, 8 Jun 2023 08:47:37 +0000 (UTC)
> Received: ([email protected]) by vger.kernel.org via listexpand
> id S235560AbjFHIrf (ORCPT
> <rfc822;[email protected]>);
> 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;[email protected]>);
> 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 <[email protected]>;
> Thu, 8 Jun 2023 01:47:31 -0700 (PDT)
> Received: by mail-pf1-x432.google.com with SMTP id
> d2e1a72fcca58-651f2f38634so284228b3a.0
> for <[email protected]>;
> 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 <[email protected]>
> To: Miklos Szeredi <[email protected]>, [email protected],
> [email protected]
> Cc: Andrew Morton <[email protected]>, [email protected],
> Jiachen Zhang <[email protected]>
> Subject: [PATCH 2/2] fuse: remove an unnecessary if statement
> Date: Thu, 8 Jun 2023 16:46:09 +0800
> Message-Id: <[email protected]>
> X-Mailer: git-send-email 2.35.1
> In-Reply-To: <[email protected]>
> References: <[email protected]>
> MIME-Version: 1.0
> Precedence: bulk
> List-ID: <linux-fsdevel.vger.kernel.org>
> X-Mailing-List: [email protected]
>
> 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 <[email protected]>
> ---
> 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);
>