[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c2a0af77-2714-202b-43d9-7100daeb80b1@oracle.com>
Date: Sat, 4 Jul 2020 18:49:20 -0700
From: Junxiao Bi <junxiao.bi@...cle.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Pavel Machek <pavel@...x.de>
Cc: Sasha Levin <sashal@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
stable <stable@...r.kernel.org>,
Joseph Qi <joseph.qi@...ux.alibaba.com>,
Changwei Ge <gechangwei@...e.cn>, Gang He <ghe@...e.com>,
Joel Becker <jlbec@...lplan.org>,
Jun Piao <piaojun@...wei.com>, Mark Fasheh <mark@...heh.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 4.19 114/131] ocfs2: avoid inode removal while nfsd is
accessing it
On 7/2/20 3:24 PM, Linus Torvalds wrote:
> On Thu, Jul 2, 2020 at 2:17 PM Pavel Machek <pavel@...x.de> wrote:
>>
>>> commit 4cd9973f9ff69e37dd0ba2bd6e6423f8179c329a upstream.
>>>
>>> Patch series "ocfs2: fix nfsd over ocfs2 issues", v2.
>> This causes locking imbalance:
> This sems to be true upstream too.
>
>> When ocfs2_nfs_sync_lock() returns error, caller can not know if the
>> lock was taken or not.
> Right you are.
>
> And your patch looks sane:
>
>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
>> index c141b06811a6..8149fb6f1f0d 100644
>> --- a/fs/ocfs2/dlmglue.c
>> +++ b/fs/ocfs2/dlmglue.c
>> @@ -2867,9 +2867,15 @@ int ocfs2_nfs_sync_lock(struct ocfs2_super *osb, int ex)
>>
>> status = ocfs2_cluster_lock(osb, lockres, ex ? LKM_EXMODE : LKM_PRMODE,
>> 0, 0);
>> - if (status < 0)
>> + if (status < 0) {
>> mlog(ML_ERROR, "lock on nfs sync lock failed %d\n", status);
>>
>> + if (ex)
>> + up_write(&osb->nfs_sync_rwlock);
>> + else
>> + up_read(&osb->nfs_sync_rwlock);
>> + }
>> +
>> return status;
>> }
> although the whole thing looks messy.
>
> If the issue is a lifetime thing (like that commit says), the proper
> model isn't a lock, but a refcount.
>
> Oh well. Junxiao?
There is a block number embedded in nfs file handle, to verify it's an
inode, need acquire this nfs_sync_lock global lock to avoid any inode
removed from local node and other nodes in the cluster, before this
verify done, seemed no way to use a refcount.
Thanks,
Junxiao.
>
> Linus
Powered by blists - more mailing lists