From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 D7597C4167B for ; Wed, 14 Dec 2022 02:04:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236427AbiLNCEv (ORCPT ); Tue, 13 Dec 2022 21:04:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60190 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236039AbiLNCEq (ORCPT ); Tue, 13 Dec 2022 21:04:46 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 74107637B for ; Tue, 13 Dec 2022 18:03:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670983438; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZxWgAtzssiZUci/PlwUY7k+wYk7RgEHn1aQq5Ig7ASE=; b=EqjexTW5hBV3c3qvtShzFrHj84rEnHZfrXgIKlGZGARxZxwew+A9TLJBYYFjUA9cIPm00T 3Zkvs+VLglPJLqHfR1SQ00q/RBrksZ2q1Uevr79gAXutDWJj4M+lGmUsf6XZ8qhM3zgk/0 w0MXJcSMm5pDvIZkpvTzihskIHWY6F0= Received: from mail-pf1-f199.google.com (mail-pf1-f199.google.com [209.85.210.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-600-2AK7WtD5MxyWTaQ8H1SbVg-1; Tue, 13 Dec 2022 21:03:57 -0500 X-MC-Unique: 2AK7WtD5MxyWTaQ8H1SbVg-1 Received: by mail-pf1-f199.google.com with SMTP id b13-20020a056a000a8d00b0057348c50123so3181925pfl.18 for ; Tue, 13 Dec 2022 18:03:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-language:content-transfer-encoding:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ZxWgAtzssiZUci/PlwUY7k+wYk7RgEHn1aQq5Ig7ASE=; b=lMZQY85zt7rGUDp+gVNrXSDBU8WictQ6IhAcK1iPqLe6DVpuO8ChrCt/jSzsK7Cg7u crdM/nUDZvaPjWrf/h/ee/kz75jrJ9YxZKFHQPK4IuIt5vvI+/PQsCDaF+3I5mHOy5wq 3Oko5V++wYJZtAn0UM+4LVLii6yAOATItYmFZRTLYI+q18C2adacCnYcuuhbyzTXQ5L3 NesPlhuZxVGg5nSN93vv2BKxz6PliZU3jNmHI8ug5a/f6F/jVwaJ7T9Ef687Es89S+uS osSnngt0Kb3O/h5TCz2Cl+k03MI+B8XUtvM43N3QZdibGw7w94MwTXh7t1X5KAkbl/OF 1deg== X-Gm-Message-State: ANoB5ply555MtCvViG3HVoMYkN68gAFTNffgDQNOIzI3Ck7CW97gJZir /YbqzjmdosaRYqT6AN5mnoSJm89BMpQyfRoKN/Wm8uKEJf8fM1KhospgnlIrViWjXulv2Di0c91 Cb/HD0/tvprRTnVyeT4kKiTmq X-Received: by 2002:a17:902:6905:b0:189:340c:20d2 with SMTP id j5-20020a170902690500b00189340c20d2mr24488966plk.23.1670983436073; Tue, 13 Dec 2022 18:03:56 -0800 (PST) X-Google-Smtp-Source: AA0mqf58GHtAnUtVx7eHJgWLx44Wxgv4SDCZhO6Z6Z6rNRrAmwxlPG98KL5qMp4Cs3IqOyWZ/lYpmg== X-Received: by 2002:a17:902:6905:b0:189:340c:20d2 with SMTP id j5-20020a170902690500b00189340c20d2mr24488949plk.23.1670983435793; Tue, 13 Dec 2022 18:03:55 -0800 (PST) Received: from [10.72.13.36] ([43.228.180.230]) by smtp.gmail.com with ESMTPSA id j7-20020a170902690700b0017ec1b1bf9fsm516185plk.217.2022.12.13.18.03.51 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Dec 2022 18:03:55 -0800 (PST) Subject: Re: [PATCH v4 2/2] ceph: add ceph specific member support for file_lock To: Ilya Dryomov Cc: jlayton@kernel.org, ceph-devel@vger.kernel.org, mchangir@redhat.com, lhenriques@suse.de, viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, stable@vger.kernel.org References: <20221213121103.213631-1-xiubli@redhat.com> <20221213121103.213631-3-xiubli@redhat.com> From: Xiubo Li Message-ID: Date: Wed, 14 Dec 2022 10:03:49 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14/12/2022 02:05, Ilya Dryomov wrote: > On Tue, Dec 13, 2022 at 1:11 PM wrote: >> From: Xiubo Li >> >> When ceph releasing the file_lock it will try to get the inode pointer >> from the fl->fl_file, which the memory could already be released by >> another thread in filp_close(). Because in VFS layer the fl->fl_file >> doesn't increase the file's reference counter. >> >> Will switch to use ceph dedicate lock info to track the inode. >> >> And in ceph_fl_release_lock() we should skip all the operations if >> the fl->fl_u.ceph_fl.fl_inode is not set, which should come from >> the request file_lock. And we will set fl->fl_u.ceph_fl.fl_inode when >> inserting it to the inode lock list, which is when copying the lock. >> >> Cc: stable@vger.kernel.org >> Cc: Jeff Layton >> URL: https://tracker.ceph.com/issues/57986 >> Signed-off-by: Xiubo Li >> --- >> fs/ceph/locks.c | 20 ++++++++++++++++++-- >> include/linux/fs.h | 3 +++ >> 2 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c >> index b191426bf880..cf78608a3f9a 100644 >> --- a/fs/ceph/locks.c >> +++ b/fs/ceph/locks.c >> @@ -34,18 +34,34 @@ static void ceph_fl_copy_lock(struct file_lock *dst, struct file_lock *src) >> { >> struct inode *inode = file_inode(dst->fl_file); >> atomic_inc(&ceph_inode(inode)->i_filelock_ref); >> + dst->fl_u.ceph.fl_inode = igrab(inode); >> } >> >> +/* >> + * Do not use the 'fl->fl_file' in release function, which >> + * is possibly already released by another thread. >> + */ >> static void ceph_fl_release_lock(struct file_lock *fl) >> { >> - struct inode *inode = file_inode(fl->fl_file); >> - struct ceph_inode_info *ci = ceph_inode(inode); >> + struct inode *inode = fl->fl_u.ceph.fl_inode; >> + struct ceph_inode_info *ci; >> + >> + /* >> + * If inode is NULL it should be a request file_lock, >> + * nothing we can do. >> + */ >> + if (!inode) >> + return; >> + >> + ci = ceph_inode(inode); >> if (atomic_dec_and_test(&ci->i_filelock_ref)) { >> /* clear error when all locks are released */ >> spin_lock(&ci->i_ceph_lock); >> ci->i_ceph_flags &= ~CEPH_I_ERROR_FILELOCK; >> spin_unlock(&ci->i_ceph_lock); >> } >> + fl->fl_u.ceph.fl_inode = NULL; >> + iput(inode); >> } >> >> static const struct file_lock_operations ceph_fl_lock_ops = { >> diff --git a/include/linux/fs.h b/include/linux/fs.h >> index 7b52fdfb6da0..6106374f5257 100644 >> --- a/include/linux/fs.h >> +++ b/include/linux/fs.h >> @@ -1119,6 +1119,9 @@ struct file_lock { >> int state; /* state of grant or error if -ve */ >> unsigned int debug_id; >> } afs; >> + struct { >> + struct inode *fl_inode; > Hi Xiubo, > > Nit: I think it could be just "inode", without the prefix which is > already present in the union field name. Okay, I can fix this in the next version. Thanks. - Xiubo > Thanks, > > Ilya >