From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>
CC:     Bruce Fields <bfields@fieldses.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Dai Ngo <dai.ngo@oracle.com>
Subject: Re: [PATCH RFC v6 1/2] fs/lock: add new callback, lm_expire_lock, to
 lock_manager_operations
Thread-Topic: [PATCH RFC v6 1/2] fs/lock: add new callback, lm_expire_lock, to
 lock_manager_operations
Thread-Index: AQHX6ssSX8kmzDHzL0ydpUvZWW9YQKwly7cA
Date:   Mon, 6 Dec 2021 18:39:51 +0000
Message-ID: <133AE467-0990-469D-A8A9-497C1C1BD09A@oracle.com>
References: <20211206175942.47326-1-dai.ngo@oracle.com>
 <20211206175942.47326-2-dai.ngo@oracle.com>
In-Reply-To: <20211206175942.47326-2-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?EZt8A1mTjNJv48pV1TAvQvCEE5tBif7x+BYqRPiLusYafsEOzjxPOOc4PcTR?=
 =?us-ascii?Q?tIU9Jhli2Ol4mA5ws2SLFOwRPGiYIiUVWSuG++xQptLKqx0aLounmjidBI3M?=
 =?us-ascii?Q?g+gLxLF1rftSvFLsYcV4QwZL2LicBECC/LA1QzlIQkTlw1wt0G2HJVxYyhD/?=
 =?us-ascii?Q?j5N9V5REmlTJ2jFRbGPbcFOGumZ46CHt+aWdEA7RIDI7TO9a0XQYVFCrc2hy?=
 =?us-ascii?Q?/3Oe0fBzJgyYtNbOjN8ZYXspwSjQ1KZRExN15REIReFfT0kV+qgy+ocR8Z8n?=
 =?us-ascii?Q?C2sOtOUH3PIkWzWmIrmuAp6V7JPahupvmpxciOzNoIRtKslkB8aYuE8kn31j?=
 =?us-ascii?Q?R4CVM033/JCwh8DxXQ5w3zBP7BaqqHNAUYRzktCr9KPgVqkYRBJXFXav5Uh4?=
 =?us-ascii?Q?WyHg+YO+uCQn3BTI2L3H4Xo7oFxHyuEYyW420bYX3iQcfLnJoSK93t6G/8Zq?=
 =?us-ascii?Q?4zIkL/snNCYrKxFP2N3tFqjwM85URt0gFRXiP9HVBGYIl4UCSr65fBIyphua?=
 =?us-ascii?Q?x9T/ejHOnU6uj8j77gNTMrcfshRvJS2HfNxi9e0v87/g0KFrfkoJZgbtldUT?=
 =?us-ascii?Q?+BiObyss2vVNrTrkh5WEB5LjYcM4G+hilLMj/PNU7/kc42g87a/pt6UZ/7Jr?=
 =?us-ascii?Q?NAqEr2xkIwgKCxbKIo60Wvf8ywjqwQZKKWPnwnBB4Braz3BJPdZ8ZRR/f5by?=
 =?us-ascii?Q?zaSjC5aPtXO8ff/+Fwe4TKxkvh6JUdrAEhTx93Xb/EpBvvSndarDWt/Z6fyk?=
 =?us-ascii?Q?5LyS3X443/VBqOh3FWXeao7eOkb/aOd1fYkRDtKt1UHgztwx4SXF5RevJjA8?=
 =?us-ascii?Q?Qko2+L4nau1qXsnQxGEGynyh6Cl6nuTZGZGw9LikOeUJc/53fbKQKvxTZ1JB?=
 =?us-ascii?Q?QTB3nSUHzSSGfgjjkwIwQNqb/gJQcVKRR4bc98FEx7xp52le339/I2svGi0Q?=
 =?us-ascii?Q?kCSR4SvlDTS3ZCTQ3iBidQDe1L0LKz/SaGZ5hUJCu99bfzajw98ZMkN4AcIc?=
 =?us-ascii?Q?dMBqtWFgFlC+6+YC8p3XTF9fCkUSbLEJAXCWZHuDXRwVBJysdHZphtxV4rnl?=
 =?us-ascii?Q?6aWz/0IxDcHOnyyUfRWyIoOI2M+UR2YBpy0Z1Shkd5bUa1S1sT/AzgiIEgdX?=
 =?us-ascii?Q?1L8ZLu8bJM9XkLIT8C3SCNrqhSnZRNnC00rVZxm5fZx3XOI709/aT8Hrgx7q?=
 =?us-ascii?Q?ePp2dYjR8bBg4brh9Kyl+DJ43F7txGpZuP1yFk4yG7VdqpiOJgk8jopDNSXv?=
 =?us-ascii?Q?ngc8kVW3KyYEnC/Y/wseB6Gut4znOuh9fdPcipBU2sBmcQuR7Kn2ZsBeGpiU?=
 =?us-ascii?Q?P4ZXPlN4NX5/FQn+FBgzleCxYfqjH5dH5tFiwEEmrHQ5mz20elvryFd5Q3Wz?=
 =?us-ascii?Q?vscy1RTm22ZyPMYldI36WI4x6gZGlPE46r7m8eOsUI8r0ChpWH77H/SSRu/F?=
 =?us-ascii?Q?g+cpzEE3TXvnkQy6eWP3Qvcy2+pCp4dpszYuA+HN+OItGsE0KnNGUJwlSdQN?=
 =?us-ascii?Q?wJjeP5Nk+nY7IHEYRe7END2DNGU84RsdHsS5HXj55nFfTxCW4RMQd9gxNyMt?=
 =?us-ascii?Q?D/8TVNSkz5BRJDaJAAqI2wHDqRLt1/YirdN9aPjgwjrq2Eu5gfedcgNxiA9g?=
 =?us-ascii?Q?QMqlFV7Q6zWiduvEUSwPCrY=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <914631708601C44688B5265C29B5E8B3@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: afd2b830-678f-4600-0e12-08d9b8e7ca36
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Dec 2021 18:39:51.8684
 (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: 9PKZVqOewDQ2Jchc/ABl7MUb7N6oZtFZr5lE4oZG8BfE4VDCwMPndSO22z0LouGTsPE51qsFxv4HrPmyCk5UPA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB5340
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10190 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0 phishscore=0
 malwarescore=0 spamscore=0 mlxlogscore=999 adultscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2112060111
X-Proofpoint-ORIG-GUID: HHiUM23jVUgNWxlHcGse3dr0iOB5I4gL
X-Proofpoint-GUID: HHiUM23jVUgNWxlHcGse3dr0iOB5I4gL
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Dec 6, 2021, at 12:59 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 2 arguments, file_lock of the blocker
> and a testonly flag:
>=20
> testonly =3D 1  check and return true if lock conflict can be resolved
>              else return false.
> testonly =3D 0  resolve the conflict if possible, return true if conflict
>              was resolved esle return false.
>=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.
>=20
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>

Al, Jeff, as co-maintainers of record for fs/locks.c, can you give
an Ack or Reviewed-by? I'd like to take this patch through the nfsd
tree for v5.17. Thanks for your time!


> ---
> fs/locks.c         | 28 +++++++++++++++++++++++++---
> include/linux/fs.h |  1 +
> 2 files changed, 26 insertions(+), 3 deletions(-)
>=20
> diff --git a/fs/locks.c b/fs/locks.c
> index 3d6fb4ae847b..0fef0a6322c7 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -954,6 +954,7 @@ posix_test_lock(struct file *filp, struct file_lock *=
fl)
> 	struct file_lock *cfl;
> 	struct file_lock_context *ctx;
> 	struct inode *inode =3D locks_inode(filp);
> +	bool ret;
>=20
> 	ctx =3D smp_load_acquire(&inode->i_flctx);
> 	if (!ctx || list_empty_careful(&ctx->flc_posix)) {
> @@ -962,11 +963,20 @@ posix_test_lock(struct file *filp, struct file_lock=
 *fl)
> 	}
>=20
> 	spin_lock(&ctx->flc_lock);
> +retry:
> 	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_expire_lock &&
> +				cfl->fl_lmops->lm_expire_lock(cfl, 1)) {
> +			spin_unlock(&ctx->flc_lock);
> +			ret =3D cfl->fl_lmops->lm_expire_lock(cfl, 0);
> +			spin_lock(&ctx->flc_lock);
> +			if (ret)
> +				goto retry;
> 		}
> +		locks_copy_conflock(fl, cfl);
> +		goto out;
> 	}
> 	fl->fl_type =3D F_UNLCK;
> out:
> @@ -1140,6 +1150,7 @@ static int posix_lock_inode(struct inode *inode, st=
ruct file_lock *request,
> 	int error;
> 	bool added =3D false;
> 	LIST_HEAD(dispose);
> +	bool ret;
>=20
> 	ctx =3D locks_get_lock_context(inode, request->fl_type);
> 	if (!ctx)
> @@ -1166,9 +1177,20 @@ static int posix_lock_inode(struct inode *inode, s=
truct file_lock *request,
> 	 * blocker's list of waiters and the global blocked_hash.
> 	 */
> 	if (request->fl_type !=3D F_UNLCK) {
> +retry:
> 		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_expire_lock &&
> +					fl->fl_lmops->lm_expire_lock(fl, 1)) {
> +				spin_unlock(&ctx->flc_lock);
> +				percpu_up_read(&file_rwsem);
> +				ret =3D fl->fl_lmops->lm_expire_lock(fl, 0);
> +				percpu_down_read(&file_rwsem);
> +				spin_lock(&ctx->flc_lock);
> +				if (ret)
> +					goto retry;
> +			}
> 			if (conflock)
> 				locks_copy_conflock(conflock, fl);
> 			error =3D -EAGAIN;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e7a633353fd2..1a76b6451398 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1071,6 +1071,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_expire_lock)(struct file_lock *fl, bool testonly);
> };
>=20
> struct lock_manager {
> --=20
> 2.9.5
>=20

--
Chuck Lever