On Thu, Jul 2, 2020 at 2:17 PM Pavel Machek <pavel@xxxxxxx> 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?
Linus
