Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <0f895b2a-ca22-612e-fc8f-5eced86633ba@oracle.com>
Date:   Thu, 3 Feb 2022 13:38:37 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.5.1
Subject: Re: [PATCH RFC 1/3] fs/lock: add new callback, lm_expire_lock, to
 lock_manager_operations
Content-Language: en-US
To:     Chuck Lever III <chuck.lever@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>
References: <1643398773-29149-1-git-send-email-dai.ngo@oracle.com>
 <1643398773-29149-2-git-send-email-dai.ngo@oracle.com>
 <A40572BF-F202-4FB4-8384-4A410860AB0E@oracle.com>
From:   dai.ngo@oracle.com
In-Reply-To: <A40572BF-F202-4FB4-8384-4A410860AB0E@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UEl6QUlWY0FJeWlISGdscEtoZTJPOUhkQjllTngyYWdMc2I0U1RZYm9HVE9W?=
 =?utf-8?B?d2MyNDdMZHlCTVc4Q3pHdHBqRGpkQjZYZ251UmJ2M3g1ZUhHWTE3SllUUFpu?=
 =?utf-8?B?dGNrNFVqODg4WTdRdEJ2S0ZpWVB4dzJLMjBZa21kcWtxUmRKVDFiWXFFRDUv?=
 =?utf-8?B?d0hDcGx4bjRjUjhHRmQxVG5MT1NBNmF5c0N1Yi9PUGxvdFYrRVd4aytjbTdZ?=
 =?utf-8?B?Y0VaSlI0WVR5anFxeHE5SDZxSmgzT2FkbitSQmk2cHdkS0NsaVY5dFFpRUFj?=
 =?utf-8?B?bzFPVHpnZ1Uwem8rcmI5WUxwOVJ0WldjbDEvVlFPS3lQdUhqQ0lDS0Z2Q1R1?=
 =?utf-8?B?NDQ4MlV1NHFlMkxITE9uaFBYYkdjOVdESzRJNXlJWGdmMDZRM3BscFQ4YjY1?=
 =?utf-8?B?NHpHYlc3OUp2VDJWRjZhL3l0WmlORzNjSGZ3OWZwaTdVOFZtd1J3cHVEdVJB?=
 =?utf-8?B?RzhwRm1iSmQrdm5yNG5NbkVLbXJHOEYranhsUkdOaGlRaXVlU0dvSTQxYS9t?=
 =?utf-8?B?SkNnSVhnU2dYUm4rMEwyb20rZ1VabzE2UmJqTFYxMzRka2Jla2YvU1JVQ3Zz?=
 =?utf-8?B?SkZIWElhTDJzaVoyMWZNZVk3VW9qZmNHdEVzVWR4TVpLU3E2MkxQZXhxYlFM?=
 =?utf-8?B?NEFJS0xLVS8xYW9MUTRtMndaOG9Eb0NWcUprZW9pVGVyVU1mS29aUWpCQTFu?=
 =?utf-8?B?Q1lWdDJrNUlJOHAzS0hHcWtTSU9FTFRSdllZWlZOcmRLMVRhT3ptZWQvV3Q2?=
 =?utf-8?B?Y1VYQWZMV2dmTThPVVlrZHZjZllxZTVHUms1b1RqR2FEOHB4N29RU2VhOExR?=
 =?utf-8?B?Q3RDV0V3UW9Rck1iL0FsenQ3RU1oMkNZNjRFU2dXVjBtU1J0VlhVTUdRVzQx?=
 =?utf-8?B?VlZJazYwa3VmYWw3eUtXb3hyQWFzdGFYRVJ3aUZROXFYMVVFL3NMR2RYN1Nv?=
 =?utf-8?B?QUpLTkpuYXN5d1BxYVF5VEFpYzlhVGcvcmpNS3B4MlI0MXk1RzlwcWVjdTJu?=
 =?utf-8?B?TUlPKzZ1Zm5MMFIyMUQ0K1l3Ny9ISkJsWFQ1WTFSa241dXB6VUJQaTl0VCtq?=
 =?utf-8?B?Y3ZxYytGcGpVRk9xVjdFaE5uT0tnejhwWXhyQS9QcFkzTm5uaVRXT0RCTVNG?=
 =?utf-8?B?b2paOEdBT0JMdXN0S1NMb2VKNEhRZDRiQUk5bTVML2M2MGQrcnpsajBZYW03?=
 =?utf-8?B?VW9ML2J6cHVLN0FvS2VyWlBHN2JYWWFyOXFacjNXWExoMno0QlFqVVBtWG1R?=
 =?utf-8?B?c201NlYyS3NFVDZQRHNFdGltMFJ1dlFKb0NmeVQzd0h5NGlKV2thZEJaR3V4?=
 =?utf-8?B?bTBsb3BmK0VidHdMdUZvUTJzWHhXYjV1UDgxK3V2VHNnT0ExdG5LdUx4d3h0?=
 =?utf-8?B?ejN3N2QxUTdlTHhFVGdRRXhaQmZLVmZvYkRqazR3a1VyUVRpd2duVXY2MGI4?=
 =?utf-8?B?Qk1YbDRYUXg1QmJ3QkNZcGc2ZTljSlZWSmZDOUdFUXBvTzFOa2hiaXc1NWds?=
 =?utf-8?B?M3B3TkZIdEhteW5Nb25nU000My95WVU4VnhWUi83YjNNWmpnKzZtRE9Id1pZ?=
 =?utf-8?B?bnE3MHFRL1F6ejR2SnBtbHpaVVVRK3RIT1pvYmJ3Z0hiamRBajVBODNGT0hX?=
 =?utf-8?B?cm5rQ2dERmtIbW1QQWtQKzlVWnNTcnlRa1lyRDBxbEpQRVdJVS8zR3FXT1lo?=
 =?utf-8?B?ZWtiUURWZnZEWVFlNVZyRDNDVGxBTTdNb055dEJSWFNaR05SOE5OdmNQWmx5?=
 =?utf-8?B?YTIrVW9rSGpzTk9NWm15U2dVMjB6N1lGd2J1ZUpTRDV2bE1SeEtJeThGa0Vn?=
 =?utf-8?B?NldySXI4V0JNSm5qVHJrbDFwTWtoZkNtRWQ4MTREbUxJR0FsQS9nZUNibDND?=
 =?utf-8?B?aWUwWFNwQzZiLzhDanNFcWUyeldUam5UMEllQmt1cWd0V0JlNjhzUG9uNkRC?=
 =?utf-8?B?RUtlb0hlUlMweHJ5bFRCV0NrOHY3QjNBR0YyYUFNWm1hbXRCYzFBTXV2ZklD?=
 =?utf-8?B?WXoyRDFzdmUvMEovQk5XS2ZnZjh3MjVOSTBZRkxMTGxSRU1vRjNoM2NtS09I?=
 =?utf-8?B?WmcxbVdDQzkxWGtYc1hXN0J6Z0p3c1lpVzdTTDl1NGJrb2NheWpETTNpeXBN?=
 =?utf-8?B?ckFac2c5eis5UjJ1VHcvdUhySnp1a1VQZFNWSUtQdWN1Q09ucU5XOUI3SnV1?=
 =?utf-8?Q?jnznAXy6mybedatVT9bV+8k=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2baf55fc-80f3-4084-b528-08d9e75d8ad7
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4257.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Feb 2022 21:38:39.8877
 (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: MUswxK39VFKGbGTdeuIh/m6hjYbjJgMptQrgN4goXwT3ipkbTMYmJ5DWUv1ruBIUoNayurfP2kOhMKsLozm4ng==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR10MB3022
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-2202030129
X-Proofpoint-ORIG-GUID: vbU4_on4PGL_ZyNElsJ_68Hvgcfl1596
X-Proofpoint-GUID: vbU4_on4PGL_ZyNElsJ_68Hvgcfl1596
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

On 2/3/22 10:41 AM, Chuck Lever III wrote:
>
>> On Jan 28, 2022, at 2:39 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>
>> 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.
>>
>> 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".

Fix in v11.

Thanks,
-Dai

>
>
>> 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(-)
>>
>> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/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 *);
>>
>> locking rules:
>>
>> @@ -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
>> ======================	=============	=================	=========
>>
>> 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)
>>
>> 	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 = F_UNLCK;
>> out:
>> @@ -1059,6 +1062,9 @@ static int posix_lock_inode(struct inode *inode, struct 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 = -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);
>> };
>>
>> struct lock_manager {
>> -- 
>> 2.9.5
>>
> --
> Chuck Lever
>
>
>