Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <46e78dab-72fd-99f4-97c5-99733bf34c6c@scram.de>
Date:   Sun, 18 Jun 2023 17:04:55 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH 1/2] fuse: support unlock remote OFD locks on file release
To:     Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>,
        Miklos Szeredi <miklos@szeredi.hu>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Andrew Morton <akpm@osdl.org>, me@jcix.top, jan.peschke@quobyte.com
References: <20230608084609.14245-1-zhangjiachen.jaycee@bytedance.com>
 <20230608084609.14245-2-zhangjiachen.jaycee@bytedance.com>
From:   Jochen Friedrich <jochen@scram.de>
In-Reply-To: <20230608084609.14245-2-zhangjiachen.jaycee@bytedance.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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);
>