Messages in this thread | | | Subject | Re: [PATCH 4.19 114/131] ocfs2: avoid inode removal while nfsd is accessing it | From | Junxiao Bi <> | Date | Sat, 4 Jul 2020 18:49:20 -0700 |
| |
On 7/2/20 3:24 PM, Linus Torvalds wrote:
> On Thu, Jul 2, 2020 at 2:17 PM Pavel Machek <pavel@denx.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
| |