From:   Vasily Averin <vvs@virtuozzo.com>
Subject: [PATCH] nfsd4: add refcount for nfsd4_blocked_lock
To:     Bruce Fields <bfields@redhat.com>,
        Chuck Lever <chuck.lever@oracle.com>
Cc:     Konstantin Khorenko <khorenko@virtuozzo.com>,
        "Denis V. Lunev" <den@virtuozzo.com>,
        Cyrill Gorcunov <gorcunov@virtuozzo.com>,
        Jeff Layton <jlayton@kernel.org>, kernel@openvz.org,
        linux-nfs@vger.kernel.org
References: <CAPL3RVGBN4SMCBbJcrdgpdZfsfb+AFiHiLhDOFmjkD0ua+XpNQ@mail.gmail.com>
Message-ID: <943cfe47-d48a-4a55-3738-e93370160508@virtuozzo.com>
Date:   Fri, 17 Dec 2021 09:49:39 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
In-Reply-To: <CAPL3RVGBN4SMCBbJcrdgpdZfsfb+AFiHiLhDOFmjkD0ua+XpNQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K1NmdGhDMlErZGlTbjZMdlFFREVIalVzN0lmTUlNQzIxSmFTK2ZzVzdETHFK?=
 =?utf-8?B?b1pNNnFKd1Q2VTBQWjVtOERiMGdPWHN3YzVUd3FvRkM3UGdmMWl2b0xFclBa?=
 =?utf-8?B?UzhQWE45K08yWkRyTEtnTDE5WGN4aXR4cE1xaFJkdTBUKzVMeTExbkwrRnE4?=
 =?utf-8?B?a2MxTFQreTRpazFQNUVaRFI1MnhEYzlONzEvMWtFMCtuWTFUeEE0VEpDYmor?=
 =?utf-8?B?ZlQybUNMajhmTmxVcjJNQnliTnNDcmlZakYyeDIxL1p5M20zV3E1azI0eTRu?=
 =?utf-8?B?dG9SYjY3K3pmcE1GWWd5eHRQeU4zbFRBdFhwZ2JjN1ZUamlMN0NNeXFTVWJ4?=
 =?utf-8?B?MkVFZW9EOU5mNVA1WnEwQStaWDZYTkw0YUdZWTZwMzQ4NkdISWtwTGRmd3Z1?=
 =?utf-8?B?UXVUMzBOZTBrOFJ0T0RUREtDUXY5UGxOSGJ0dStsYVloVGlmMFpnTU9rbTU5?=
 =?utf-8?B?c0xPNFlwYWh0VGoyaGkwbmJleUJzci9sZE5QR0lPR3dIQTlLTjZFUHNFS3FN?=
 =?utf-8?B?eEdsVERXaTlsVktPR2lBUjJyZkdIeDRCMDFJdnVhd0laMjZUYkpLamtsV0V0?=
 =?utf-8?B?Z1RCTHlhK1JxMTJzNkEzQ1lHdVdMQlVKdnZrUVpYOUxPMnRqSWM2MXgydGI3?=
 =?utf-8?B?Rlh6bWZCenpUVE9PNVloMGlieDJ5M2tnU0cvcGZXTlBFTjMxSGtTbUxRQWRm?=
 =?utf-8?B?a3dGVStGN2twaXBnUVZFRkcyeW92cXpGbzN4UzBDNEpFbWlkSkpaTnZhTlRv?=
 =?utf-8?B?SnZ3RUFIRVpDYWdXS2xRQURMZ1orOTFibGlEby9wSEFVbW15a3MxeEZKNlFE?=
 =?utf-8?B?SlN2WDR3NXc3N3M4U0JaUkNkTjMwMkFXaWx2NHVKSUkvVXlLQ3dqNTRIYXRI?=
 =?utf-8?B?RGNuaHpGOWNpckRLakNnU2c3TXExa0U2RS9Nc1pFUFhKcDhOYXMxZldvSVJa?=
 =?utf-8?B?NlcyR1R2RGpXSG9uY0hweVRyUWQwRnFhVEV6M1VValhjZkJ1WCt5bzcxa1NF?=
 =?utf-8?B?UzNoUFRxL2s4QkMzTUE0WWJrbzVhV0wyZ1ZBaHZNS25XMXN6UjR6a0Nldk8v?=
 =?utf-8?B?NmlDOE1HYU9GWmEvUmp1K3NwbmhmWjZlaldsR041OWgxSk14ejBSRWdYSmhH?=
 =?utf-8?B?a0M1VUhBUE1yRVg4ZmdjZnNxdk1CSDRYajgwTEhrQzJ6NzZRaFhRd0tzZTJw?=
 =?utf-8?B?SEpvcTZtYmpiRDdicDZOby9UR0ZqYnBOSnd6aldqY3A3OVFzRFlBOVhuYUF5?=
 =?utf-8?B?aWM3TTl5V2hVV0RqR1hEQjV6OHFiU2dWQ0lwam4zWlRISHZHd1ZnSlllMGF1?=
 =?utf-8?B?MytlWStXUlNybU5VbG8rVE1VZ3BKWDExNmtOTkRUd3NkdGkvenNCQisvTXZ2?=
 =?utf-8?B?Mzl2Z1VlTCtELzJhZEhSVElWOG5DR1owNFBnSGFOQ2FoTlpxME5PaEF3KzI0?=
 =?utf-8?B?bSswRHQvS1RsRTBhUVNuZHFVODI2eGhLTmE4V1ZRKzhmRnhlaXBSSC9PSnlD?=
 =?utf-8?B?dWxUNnJyMmtMTFlzSmpyckRZZjFVa3hldVhYWEF0a1cwT1BpbFZOSnA0TnUw?=
 =?utf-8?B?YWhTMXdOV05tNXBJQlJVS0FhbU9vc0t0QTJBOVdhZ29iUkd5czArRlJBc2JH?=
 =?utf-8?B?RXlEaWJYRjR4MzUwL3UyaHlVN3o0ejlUK1lJcFpESnYxRHhXd0J1TlpvOXox?=
 =?utf-8?B?dGVKbFRORHpQeXlJQXpGOG5iQTJldUpnc2orZjJRM0hMWm83VzRuS0ZiTFJm?=
 =?utf-8?B?MFpaVUgyZzRBck1uc0tQMFJCaHBFa1RlNThoYnRqK05wek50d1QwaE9NdXZG?=
 =?utf-8?B?KzI3L0oySHdBZXRRVkxPbnFLMWlMOUVFeXNWNEkrMlVPWExyWmJaR05ZU2Qx?=
 =?utf-8?B?MFcraVFyTnhpcnFrTnY2SVhkQ2RnbEVlRCtXZWhpRWFzTk4yVVRHUDBhaTFa?=
 =?utf-8?B?UGhZRzV0Vmd6c2hlNXRFbmhXYldwWVFmVmhHK25WNzFoaXBHY3BaTUxtbWFW?=
 =?utf-8?B?amxYd09wNFA2dVJueEpHeS9tbHVLUVlPTTkwdEllZFdnbWJCa1o0NXBwNFhG?=
 =?utf-8?B?WGNQdFdrdklyRG4zUmxBVGJURU1hZWdCQnQ0MmRxcmpDTXFCNy9HMnNyZFZ6?=
 =?utf-8?B?L25hb3piTHF3VDRRN2U3dk4rVjFiOXBuYkhpKzNoazVkN3J2MFBHT29ac1dM?=
 =?utf-8?Q?U3y54hwZpomLNSf2L96LhUk=3D?=
X-OriginatorOrg: virtuozzo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ba5d04e5-4a5f-41c8-f967-08d9c1296690
X-MS-Exchange-CrossTenant-AuthSource: DB9PR08MB6619.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Dec 2021 06:49:40.9188
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cz8SVZO99iMVczn5C4/Dg/sO5nK3TATjqDevY3xmwez73kBsY3UiEQFKYFnts/uy1i3K3rUCc9qA4Sb4iAiL0g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR08MB6108
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

nbl allocated in nfsd4_lock can be released by a several ways:
directly in nfsd4_lock(), via nfs4_laundromat(), via another nfs
command RELEASE_LOCKOWNER or via nfsd4_callback.
This structure should be refcounted to be used and released correctly
in all these cases.

Refcount is initialized to 1 during allocation and is incremented
when nbl is added into nbl_list/nbl_lru lists.

Usually nbl is linked into both lists together, so only one refcount
is used for both lists.

However nfsd4_lock() should keep in mind that nbl can be present
in one of lists only. This can happen if nbl was handled already
by nfs4_laundromat/nfsd4_callback/etc.

Refcount is decremented if vfs_lock_file() returns FILE_LOCK_DEFERRED,
because nbl can be handled already by nfs4_laundromat/nfsd4_callback/etc.

Refcount is not changed in find_blocked_lock() because of it reuses counter
released after removing nbl from lists.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 25 ++++++++++++++++++++++---
 fs/nfsd/state.h     |  1 +
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d75e1235c4f5..b74f36e9901c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -246,6 +246,7 @@ find_blocked_lock(struct nfs4_lockowner *lo, struct knfsd_fh *fh,
 	list_for_each_entry(cur, &lo->lo_blocked, nbl_list) {
 		if (fh_match(fh, &cur->nbl_fh)) {
 			list_del_init(&cur->nbl_list);
+			WARN_ON(list_empty(&cur->nbl_lru));
 			list_del_init(&cur->nbl_lru);
 			found = cur;
 			break;
@@ -271,6 +272,7 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh,
 			INIT_LIST_HEAD(&nbl->nbl_lru);
 			fh_copy_shallow(&nbl->nbl_fh, fh);
 			locks_init_lock(&nbl->nbl_lock);
+			kref_init(&nbl->nbl_kref);
 			nfsd4_init_cb(&nbl->nbl_cb, lo->lo_owner.so_client,
 					&nfsd4_cb_notify_lock_ops,
 					NFSPROC4_CLNT_CB_NOTIFY_LOCK);
@@ -279,12 +281,21 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh,
 	return nbl;
 }
 
+static void
+free_nbl(struct kref *kref)
+{
+	struct nfsd4_blocked_lock *nbl;
+
+	nbl = container_of(kref, struct nfsd4_blocked_lock, nbl_kref);
+	kfree(nbl);
+}
+
 static void
 free_blocked_lock(struct nfsd4_blocked_lock *nbl)
 {
 	locks_delete_block(&nbl->nbl_lock);
 	locks_release_private(&nbl->nbl_lock);
-	kfree(nbl);
+	kref_put(&nbl->nbl_kref, free_nbl);
 }
 
 static void
@@ -302,6 +313,7 @@ remove_blocked_locks(struct nfs4_lockowner *lo)
 					struct nfsd4_blocked_lock,
 					nbl_list);
 		list_del_init(&nbl->nbl_list);
+		WARN_ON(list_empty(&nbl->nbl_lru));
 		list_move(&nbl->nbl_lru, &reaplist);
 	}
 	spin_unlock(&nn->blocked_locks_lock);
@@ -6976,6 +6988,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		spin_lock(&nn->blocked_locks_lock);
 		list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked);
 		list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru);
+		kref_get(&nbl->nbl_kref);
 		spin_unlock(&nn->blocked_locks_lock);
 	}
 
@@ -6988,6 +7001,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 			nn->somebody_reclaimed = true;
 		break;
 	case FILE_LOCK_DEFERRED:
+		kref_put(&nbl->nbl_kref, free_nbl);
 		nbl = NULL;
 		fallthrough;
 	case -EAGAIN:		/* conflock holds conflicting lock */
@@ -7008,8 +7022,13 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		/* dequeue it if we queued it before */
 		if (fl_flags & FL_SLEEP) {
 			spin_lock(&nn->blocked_locks_lock);
-			list_del_init(&nbl->nbl_list);
-			list_del_init(&nbl->nbl_lru);
+			if (!list_empty(&nbl->nbl_list) &&
+			    !list_empty(&nbl->nbl_lru)) {
+				list_del_init(&nbl->nbl_list);
+				list_del_init(&nbl->nbl_lru);
+				kref_put(&nbl->nbl_kref, free_nbl);
+			}
+			/* nbl can use one of lists to be linked to reaplist */
 			spin_unlock(&nn->blocked_locks_lock);
 		}
 		free_blocked_lock(nbl);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index e73bdbb1634a..ab61dc102300 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -629,6 +629,7 @@ struct nfsd4_blocked_lock {
 	struct file_lock	nbl_lock;
 	struct knfsd_fh		nbl_fh;
 	struct nfsd4_callback	nbl_cb;
+	struct kref		nbl_kref;
 };
 
 struct nfsd4_compound_state;
-- 
2.25.1