Subject: Re: [PATCH] v9fs: handle async processing of F_SETLK with FL_SLEEP
 flag
To:     Dominique Martinet <asmadeus@codewreck.org>
Cc:     Eric Van Hensbergen <ericvh@gmail.com>,
        Latchesar Ionkov <lucho@ionkov.net>, kernel@openvz.org,
        v9fs-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        "J. Bruce Fields" <bfields@fieldses.org>
References: <YcXfm6U/6+Xmv7be@codewreck.org>
From:   Vasily Averin <vvs@virtuozzo.com>
Message-ID: <5ec8d1c6-e410-e297-0d3a-037c7de30b6b@virtuozzo.com>
Date:   Mon, 27 Dec 2021 10:42:48 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
In-Reply-To: <YcXfm6U/6+Xmv7be@codewreck.org>
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?eWlZMlorMXQxOEgydHN2ZFdNN1hGNTNyMXpDWlU3T2J5dHdpRWhNMkxGVmJD?=
 =?utf-8?B?elVDOEY1YXdmNmJESklCZGFlZ0M5YXJpT3ZMMW9wNGRYT1MrQ3pyNmNFZWU3?=
 =?utf-8?B?TDhpTW5OMXpoL01nKzViV3BNL0VvUzdDenZkZjEvM2x6dURPNi81ekxRNWVa?=
 =?utf-8?B?TTQ5amlYMzZXTUozL2xZbkVtTzRVdXJMYy9SaWFSRWJ5U3RiNGFudWh3NGla?=
 =?utf-8?B?L0dMeGNsVDF3LzVad2FQcEJxK1ArRU9TWmpzMDFDR2tERUNuTnVMSUVIWTAy?=
 =?utf-8?B?eWptcElXeGNqczRob1lFSktHSDVWL1Q2VVpmZEQ1K3gxYVVjWTUwRFh3Ky80?=
 =?utf-8?B?NUVIZFV0SkJHbGxSRHZGMDRRblFtV3VuMi9zVlAwWFRnaVNEenlPenFzUGJ1?=
 =?utf-8?B?bTVRdFhncWh6MkJPWThnU2U1ZFo2U1pOZjVCWm44dEh4Qm5uakpmYVFZMzE2?=
 =?utf-8?B?SGlYVndVYUh3UHU5cVk2MmxONjFHRWZ1YzVsL0pOM0RjVnpSaW1wMTNObVJp?=
 =?utf-8?B?TmU3U1JhU3ZGREwwRUNEV0pJL0xQQ2lCc0g4Q1VTUHpnTkFnRmVkZGxMY0xp?=
 =?utf-8?B?U0RZVjFrSmY4TE1tay9wSWhNblJZT2ViMVdBdnNtQlpQVzQ2YS9yNG5VTlVs?=
 =?utf-8?B?TXJWT2RpYUlzK29MV0lzSGd1TjNtbGVSSkc3cW5zT2JpYjFCQURRbm5pdWkv?=
 =?utf-8?B?SmJUQVhlYzVzTHI1UldCTUtUTWJub1puM2hFUng5Y3N1VFZQVHQvL2pLQWQ2?=
 =?utf-8?B?MEZWRmt3QkQxQzRLak1JTm84QzlkaXpPejFMSEh0eU1MelhnNnc1M0tBWkNX?=
 =?utf-8?B?UG9peXJlODh5UmluL2NxbmVkSXphdFJkMHh3WG9XV3VpL1pEdEhZREZyT1JZ?=
 =?utf-8?B?RnhGbXVWTEp6VTB4K0lQeFBuR0NRVWVuWTExWlFpWVNGRy82WmRPSTJVeExL?=
 =?utf-8?B?cXJ0VzhwRWNRYUoxYWNXQ3EraCtjVGhNMEN5cksxc0drODJzNTg1ZGcvanhm?=
 =?utf-8?B?R0NDZEk0TkJTL2UvVWt0MjA2Uys1UUQzcE40YWg2SXFLaVhVdmVMUGZ0S2RV?=
 =?utf-8?B?cnhZaWlsSDVYeVlNK3BGanVXYjFid3JZQWMrSU41L0V3Vzg5RkpuODVKbGlX?=
 =?utf-8?B?YzRCN2IvdGpZQ2RGL1hHemRrTjNDZ1kwSmxBL01Tdlg2WDVVWnE4eXdtS2Qx?=
 =?utf-8?B?b1VyeUszdUJya0U0ZlhSUHZrZHZaSVJvekJZaExWRk4vYWdZQ2RyNlJ5cHpC?=
 =?utf-8?B?KzBTL2ZGdU1CQVY1cVhmMytOR2FxMmRQMWJhSlFyenN4blBadC9UT3htYUZC?=
 =?utf-8?B?S3RyZEZ6UzZXZWppSzhWS2QxUzNOSktOT1hpVkgyT2tqYTEvS0kreFE1dHJ2?=
 =?utf-8?B?RGIzTlVleDhjRUhpWlBBOXc0Sk9kWjU3ZXIvQ0t0eHo4V0s0U3FOTE12ZkxM?=
 =?utf-8?B?T0ZOT1BxVmR2ZnVJTEJVMnJyQmk2ODROb2psT25PYy9DUm5NUm5Kb0JJS213?=
 =?utf-8?B?UW9NdXhtWFp4cDVwR3pVSDQweGpNSnFkcW02dWk5UHU4ZnBLQy90SDg5TGRk?=
 =?utf-8?B?MGl4UXhJUWVnRWVhVjB1OUh0NEZ1ZkM2QnVGRmtHM1czM3JwUGNVZGJldE9C?=
 =?utf-8?B?RXdEcm83RjVBRHQ4QkoyMHk3dG00dk9yZ0hGdzl3ckxKdWg0TW05M3JlMk1W?=
 =?utf-8?B?cWprT3pIVmZyRzFVMEZrZ2gzZG9KQk9CT01HNGg3RHZXK1BTSjdEUkJqNk5R?=
 =?utf-8?B?QXhCMCt1WnR5Q1FRTk5DUFBIZjNSUnlZaVgxc0hpbjVBUkNzYXZibndrN3lw?=
 =?utf-8?B?UEhtbWkrUWQrWXc4ZEhvb2huUVBwajZCU1QxUVRDd2MvRmM3YkFuSmNWbFlF?=
 =?utf-8?B?THdWZzllSFVOTTdNa2NFbEZHaXc1TlVISTA2WkgxN1J0Mlk4VmxFZXFxeXFm?=
 =?utf-8?B?enJQekZKUDBYQmhjM3hJNGl3eElQdjREQTRGS2Npb20vanNvYy84dnhyTHFu?=
 =?utf-8?B?czRCMlpyQ3FCSjVHZkt1dlBJWWRGMFREblRSR09FOVhxSC80MWROc1IyWFRY?=
 =?utf-8?B?aThOTVFYWEVtQmVESWhPOTFSeDhZcDFsWi9uckNKTFFSVjVFNkFJRndnU0NH?=
 =?utf-8?B?YkVoUEFoTnhHaUxIWldjSklRRHR4UUlwWCtGV24yTFdlUU5wR0kwMVViZTdM?=
 =?utf-8?Q?5vEzAyBFx3NVOeeCeV1crqM=3D?=
X-OriginatorOrg: virtuozzo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d965b5df-14fc-49ef-1366-08d9c90c7b3f
X-MS-Exchange-CrossTenant-AuthSource: AM9PR08MB6241.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Dec 2021 07:42:49.5575
 (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: Z0n0dQwCEo2cpHoW4FdspzAuBiazhFnRQMrHhxCOoq8+4PivfIXPpnebKKJBukZI/IRtzKp3sTXVjWUt0KwBeg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR08MB2836
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 24.12.2021 17:56, Dominique Martinet wrote:
> Vasily Averin wrote on Fri, Dec 24, 2021 at 01:18:54PM +0300:

>> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
>> index 612e297f3763..27ede4a4a6f4 100644
>> --- a/fs/9p/vfs_file.c
>> +++ b/fs/9p/vfs_file.c
>> @@ -142,10 +142,15 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl)
>>  	if ((fl->fl_flags & FL_POSIX) != FL_POSIX)
>>  		BUG();
>>  
>> -	res = locks_lock_file_wait(filp, fl);
>> -	if (res < 0)
>> -		goto out;
>> -
>> +	if ((fl->fl_flags & FL_SLEEP) && IS_SETLK(cmd)) {
>> +		res = posix_lock_file(filp, fl, NULL);
> 
> Should we also check fl->fl_flags & (FL_POSIX|FL_FLOCK) like
> locks_lock_file_wait does, to call either posix_lock_file or ... there
> doesn't seem to be an exported flock_lock_file equivalent in the other
> case, so back to wait variant there?
> (or rephrasing the question, what happens if the lock is a FL_FLOCK lock
> and we call posix_lock_file on it? Or are we guaranted that if FL_SLEEP
> is set we're about posix locks?)

SETLK with FL_SLEEP flag can be set by kernel export threads for posix locks only.

> Vasily Averin wrote on Fri, Dec 24, 2021 at 03:07:38PM +0300:
>  > On 24.12.2021 10:31, Dominique Martinet wrote:
>> It isn't necessary to make request asynchronous,
>> it's enough to avoid blocking locks.
> 
> Well, it depends on what you have in mind with blocking.
> A synchronous RPC to some server which might never reply if it doesn't
> feel like it (bug or whatever) is very much blocking in my opinion.

The main goal is to avoid deadlock of server threads.
It is acceptable to sleep or process such a request for a very long time,
however it is unacceptable to wait for another command to remove our lock,
because there can be no available working threads to process this command:
all of them can be busy by processing of blocking locks. 
Thank you,
	Vasily Averin