Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Dai Ngo <dai.ngo@oracle.com>
CC:     Bruce Fields <bfields@fieldses.org>,
        Jeff Layton <jlayton@redhat.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH RFC 1/3] fs/lock: add new callback, lm_expire_lock, to
 lock_manager_operations
Thread-Topic: [PATCH RFC 1/3] fs/lock: add new callback, lm_expire_lock, to
 lock_manager_operations
Thread-Index: AQHYFH7WfI678dNhb0CTIchhEp3zcqyCMl6A
Date:   Thu, 3 Feb 2022 18:41:40 +0000
Message-ID: <A40572BF-F202-4FB4-8384-4A410860AB0E@oracle.com>
References: <1643398773-29149-1-git-send-email-dai.ngo@oracle.com>
 <1643398773-29149-2-git-send-email-dai.ngo@oracle.com>
In-Reply-To: <1643398773-29149-2-git-send-email-dai.ngo@oracle.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?si/fd2B1fKOEUbyJDZ5VchniRmeTdzaZpY2fmXfPNtrD8Jn0pgOcU+EneEUf?=
 =?us-ascii?Q?n5dIs+I6iFR4tHqIE9MDpNZ7Nxx1pvoHaxhUAsSJztRvzXUC9tOUSZd6g0hu?=
 =?us-ascii?Q?AWM05a1DC4kmW7GOm7NY4HH/csKNSwnyjsDfxMfPUuQgPPQugL8VqUHdrfxE?=
 =?us-ascii?Q?DMiL0qo3CZAFcdxszdlu1EEM/TxJAiUqAVaUO0kbrN9ESbMlLGiMEApmji8m?=
 =?us-ascii?Q?+tbW04LMinAd+C/LOm0l6WuGRVCmsjoy8kPl4+H7U9xhgR0C+NsLRxtwcbOO?=
 =?us-ascii?Q?GcR+ftrwCFpzViRQB+FLmoRVy5Zro1VF/w+uOCf9abM9lEFSA8F23B4UhnEa?=
 =?us-ascii?Q?cEcFaCBsh0HL9+OR0ca2ZgUFG4j4mJtPT+K4XGgmX9GqRDW6sa9khTJlPtOT?=
 =?us-ascii?Q?iOnSCMFqrirL4uoL0oPMqkRafIebFh8Pcd/kYcYq9UL/VsNV4L3spNUbjmFS?=
 =?us-ascii?Q?HgLdkxR2Vk3keTVLhdiE5Qd6BgbX+6JvLwyJwq6itdgBaKyM8+9GGSZSH3gg?=
 =?us-ascii?Q?Q1IIbLm9oPSL19YMuyDmvHJOYK03aKvhnGaASIk9ClIqS2ML1aXDahcIIAQt?=
 =?us-ascii?Q?9UMIfFs9SHTGkzY+gZMkwctD/ZFBn/W0zEotA7tC/iXc43455RdAWUyXSQEX?=
 =?us-ascii?Q?VoDt7rUXaFtr/NDmJnvQ78+j4sy7oV0j4hS6GTii6n/SmRpqTPwx8EJWjnOj?=
 =?us-ascii?Q?L6uyvl11f5CNBqnucRV+X6/pnw6pOjs41kxKqtf6ygL0JJvmw3iDABn3f4jX?=
 =?us-ascii?Q?Cht7EgoJDv3WNs3Fr2Brf4IgNA4RUAz3FVrvwr+GwUDfXbramNBAU9oBHs1k?=
 =?us-ascii?Q?UQS51pZKCckvm9OfxOnA2MuJ+aoGrtbGVpH6JlpF/kJ3SxWcrToFLaBnKUJU?=
 =?us-ascii?Q?2t4sOxmVL8uFaANya1xftB49yzMhnweikOhnZl8Dm5zHsT6vSbYkpqXsIbeS?=
 =?us-ascii?Q?HEOvbxS2Ldg6YqpSB5jrT184I5U12aDE2wKOLy81ex0oYnRfTgK1CBHgGj3Y?=
 =?us-ascii?Q?bL2vTWyTGJgYtNmlcX2yYt307ga/y9jXIDOptOAlD7atmTB7/H3vaQ/E5V/Z?=
 =?us-ascii?Q?dpI2dmyNeW50MEx7Ys4JJyR9DMZszNIZCVcj/mTpB+hdfCimi4O0t5RNtPVN?=
 =?us-ascii?Q?HVDO8b1NFNDmndH43WIDMxi7Ne+e9mpDw4LbGdeSWEJelFrDiZdmS5fQCPMR?=
 =?us-ascii?Q?vtnCMlqarWnPpSHVdDtJbCN3KpMNdfWflpA586rj4Blx2h4k6/oVYmRnEPB+?=
 =?us-ascii?Q?vlYuodkZ0YBxF+BCGjF57gmrXRqJ97VQLEsAn+UtkbFPRAy4w8NzskEHkKVM?=
 =?us-ascii?Q?LKp4+uMCZ8IM8s5v7bLVqNoiXnywiJlvdb0qDGIuPTuzszBurEuuAPE3EILX?=
 =?us-ascii?Q?S2wIgZ+XPwDjjBVWLcGMCmS4Q9LPDXrNwedPLYR2Xe6TZfqoGIKFVFjfAvKo?=
 =?us-ascii?Q?u3fLnAXS1yrGnymhZN8faNpPwYE3ktNDgj6f3zY7GH+defSL7tJ5MUzPJCof?=
 =?us-ascii?Q?13/OMcR3qhftFqwplknqLSa68WuDkaxrJfahqMrY2WzoLcLiFjG84LPiICPA?=
 =?us-ascii?Q?LtC40Ax1GQeS1jYxTLlsDWFHPRIG14ahxJsId0O69D2HanvM5IbjWw0+LHb8?=
 =?us-ascii?Q?4+WbJhcBv/pPYoralRkSVDU=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <22DDF84BC8D3224D9DF4B9D3310C6626@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB4858.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fc28caae-05f6-4dee-3874-08d9e744d168
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Feb 2022 18:41:40.5603
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 4vbhRH8mGJTsGpr/df0n7YTXEk+b9NFUM2ApQivZESGaEnCWz/eUI1OiTZHHzf0hyzeLt6mPrcgbjp98p41vVA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR10MB2436
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10247 signatures=673430
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999
 adultscore=0 malwarescore=0 bulkscore=0 suspectscore=0 mlxscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2201110000 definitions=main-2202030113
X-Proofpoint-ORIG-GUID: 07Py_Pq6bsQCJfq3twPdFPgTEllXKFv9
X-Proofpoint-GUID: 07Py_Pq6bsQCJfq3twPdFPgTEllXKFv9
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jan 28, 2022, at 2:39 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>=20
> Add new callback, lm_expire_lock, to lock_manager_operations to allow
> the lock manager to take appropriate action to resolve the lock conflict
> if possible. The callback takes 1 argument, the file_lock of the blocker
> and returns true if the conflict was resolved else returns false. Note
> that the lock manager has to be able to resolve the conflict while
> the spinlock flc_lock is held.
>=20
> Lock manager, such as NFSv4 courteous server, uses this callback to
> resolve conflict by destroying lock owner, or the NFSv4 courtesy client
> (client that has expired but allowed to maintains its states) that owns
> the lock.

This change is nice and simple now. The only issue is that the
short and long patch descriptions need to be updated to replace
"lm_expire_lock" with "lm_lock_conflict".


> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
> Documentation/filesystems/locking.rst |  2 ++
> fs/locks.c                            | 14 ++++++++++----
> include/linux/fs.h                    |  1 +
> 3 files changed, 13 insertions(+), 4 deletions(-)
>=20
> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesy=
stems/locking.rst
> index d36fe79167b3..57ce0fbc8ab1 100644
> --- a/Documentation/filesystems/locking.rst
> +++ b/Documentation/filesystems/locking.rst
> @@ -439,6 +439,7 @@ prototypes::
> 	void (*lm_break)(struct file_lock *); /* break_lease callback */
> 	int (*lm_change)(struct file_lock **, int);
> 	bool (*lm_breaker_owns_lease)(struct file_lock *);
> +	bool (*lm_lock_conflict)(struct file_lock *);
>=20
> locking rules:
>=20
> @@ -450,6 +451,7 @@ lm_grant:		no		no			no
> lm_break:		yes		no			no
> lm_change		yes		no			no
> lm_breaker_owns_lease:	no		no			no
> +lm_lock_conflict:       no		no			no
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D	=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D	=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D	=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> buffer_head
> diff --git a/fs/locks.c b/fs/locks.c
> index 0fca9d680978..052b42cc7f25 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -853,10 +853,13 @@ posix_test_lock(struct file *filp, struct file_lock=
 *fl)
>=20
> 	spin_lock(&ctx->flc_lock);
> 	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
> -		if (posix_locks_conflict(fl, cfl)) {
> -			locks_copy_conflock(fl, cfl);
> -			goto out;
> -		}
> +		if (!posix_locks_conflict(fl, cfl))
> +			continue;
> +		if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_conflict &&
> +			!cfl->fl_lmops->lm_lock_conflict(cfl))
> +			continue;
> +		locks_copy_conflock(fl, cfl);
> +		goto out;
> 	}
> 	fl->fl_type =3D F_UNLCK;
> out:
> @@ -1059,6 +1062,9 @@ static int posix_lock_inode(struct inode *inode, st=
ruct file_lock *request,
> 		list_for_each_entry(fl, &ctx->flc_posix, fl_list) {
> 			if (!posix_locks_conflict(request, fl))
> 				continue;
> +			if (fl->fl_lmops && fl->fl_lmops->lm_lock_conflict &&
> +				!fl->fl_lmops->lm_lock_conflict(fl))
> +				continue;
> 			if (conflock)
> 				locks_copy_conflock(conflock, fl);
> 			error =3D -EAGAIN;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index bbf812ce89a8..21cb7afe2d63 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1068,6 +1068,7 @@ struct lock_manager_operations {
> 	int (*lm_change)(struct file_lock *, int, struct list_head *);
> 	void (*lm_setup)(struct file_lock *, void **);
> 	bool (*lm_breaker_owns_lease)(struct file_lock *);
> +	bool (*lm_lock_conflict)(struct file_lock *cfl);
> };
>=20
> struct lock_manager {
> --=20
> 2.9.5
>=20

--
Chuck Lever