Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <05262a89-1c70-3cdc-abd9-32f6730d763f@ddn.com>
Date:   Tue, 3 May 2022 22:48:00 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH v4 1/3] FUSE: Implement atomic lookup + create
Content-Language: en-US
To:     Vivek Goyal <vgoyal@redhat.com>,
        Dharmendra Singh <dharamhans87@gmail.com>
Cc:     miklos@szeredi.hu, linux-fsdevel@vger.kernel.org,
        fuse-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        Dharmendra Singh <dsingh@ddn.com>
References: <20220502102521.22875-1-dharamhans87@gmail.com>
 <20220502102521.22875-2-dharamhans87@gmail.com> <YnGIUOP2BezDAb1k@redhat.com>
From:   Bernd Schubert <bschubert@ddn.com>
In-Reply-To: <YnGIUOP2BezDAb1k@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 2
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VHVuR3hER0V6RVZOZnNmQVhCT3Y1NFN0K2tXbUp2cTAyQlBxa1hWZUFXK294?=
 =?utf-8?B?a2Vtc2ZSeE96RTQ3dFpZUVA2ZHpuSXV5Tm5MZUN5QmdPa0xQaDFvdHljYUM5?=
 =?utf-8?B?OU1DU3JReTkwaVJxOVJDQ0VKWXJQUDB6dW5SbmdYelpIeEZmbEFQVEllZWZY?=
 =?utf-8?B?Y0lrb0ZOWFVSZldNaStjb3dLTm1iS0FvUGxxdGU0U1dTSk1XY0o3VS9OdjFn?=
 =?utf-8?B?Yk9Ca09RSHBNdFl4bldPbE8wamlMdytBTTlDOHZ3bk5LVE9DMTMxY2ZGMmlv?=
 =?utf-8?B?TVFLcWdoQklkVS8rUU5VUmtQK3JFdDRxOTZoZzNUK2dhTlFmalhsb1hmQ282?=
 =?utf-8?B?aEszTENtM1FOcDRBTVptTFJxRGY5ZFFpSmhhWGhnWHd2TUp1dnNINHJYSHo3?=
 =?utf-8?B?VEc2QXN6K1F4SU1Id2VSSDd2djdubmYzVjVCMmJDcUgrSCtmV3pCSVNQbGxa?=
 =?utf-8?B?WmxWenl3U2JrRE5GMzNYaXAwR1pEeG4yS01XQWVMa0pyQkF3ajE5b0lubUMx?=
 =?utf-8?B?bkMveHJIL0Q3M2t4ZVk4bTBtamxEaTVEWnYvRDVBYlMyYWxDVk1QblFqL3ht?=
 =?utf-8?B?SlUvNUJUQ3ZWOFhnYVk5bXlTOXRJTThBVDQwaHJpZy84ZnlIM2g4TGwrcFlT?=
 =?utf-8?B?S0RhRmFrMlRIMzcvRXQwZzZzd2I2aTZ3VjNhSGdId29rckxlMEpySjYwQ1dY?=
 =?utf-8?B?cWVENSthUXFxTkdiSE1CSUw5RVNsQjkvWGJOSHBqV1FWMlkrcmFSNXBMNVdv?=
 =?utf-8?B?RG92eCsyOEE0ZWl4MUtwN0tZZWsxOFZydUFXMDBjaW9lc1RWV1hndDlKUmpB?=
 =?utf-8?B?YjA0cWkxZENEdm1mTWxHQTNhTWVYK1V4Q2hpZU1RQjZPOGJiK3QvMXNCY0J4?=
 =?utf-8?B?OExPa2NSaDVnMEY1TWowYk95bmgrMUdkQXRaV3RkM24wbUg2bWVCbUJxVTMv?=
 =?utf-8?B?SHEvQlhYa0EwRWJxa2FYaElDQmtKRkVKMXlrMlZoSWVjRjBpclBDQ2pOZmdt?=
 =?utf-8?B?RXRlRGhnanJqNUxLZ0x3L1V3OTliRkwzSDhGd1NTMmJwb1hLUVdTYlZLRHh2?=
 =?utf-8?B?UGdhL0d1c2ZCT3Q0RHc5TjcrR2M1YXB4YzBzKzZzMVhsZ0xrV3pFaVo2czla?=
 =?utf-8?B?VzdlZGFWdUtBaWovR09xWmttRDUrMGFYdE5zVEoxcjhoS0lMaFdoRXdhaDkr?=
 =?utf-8?B?MHZUay9wRE1FME9UZC8yM0JmWHFKc29LbmRKOU9GVUs1Mm4wc2gwS2JHSW9P?=
 =?utf-8?B?RlNsNXB5NDBUai9zL3A3UFZUMDcyWm1MZitHTG45L0l0QzRhbDQ3Y2RLZnV5?=
 =?utf-8?B?c2NYdEE3enZQckdSM0w1MWF0cDZXRzdmTDJqRWxvblRlSEwzcFVuVFBhaGpn?=
 =?utf-8?B?Z3UrM1VsdVRBa2t5QlRJZkZQVFpLOGhvUS9JbWE3cVcvemdJRStoblk3d3M5?=
 =?utf-8?B?bUl2Zm1NQ2ZXcXg1SlVKNmRjZjh3dHR5ZEhKK1UyL3JwMCs0QnlPRzZ1UVMx?=
 =?utf-8?B?WDQyVWlaV2VGRlRMZDB0R0tEYU5mRHE0M1ZwOVEwd0JuZngwNDIrK3FGMFJX?=
 =?utf-8?B?bUtNNXpZdENQeEdMNzZJMTY0ZHN3T3luS0pDakpHT0VTWStPTlF4RUJobmFk?=
 =?utf-8?B?Y1czZ1FwL1o5aUtuMXVxWHZNcFhpVWVDSE41NzBKanJKOFQwM0c4YlZ4TnpQ?=
 =?utf-8?B?SDlQbWtQeDFEakpDTHBLL3BrTngyTjB2OGphL3orWVBkckxjZEd0MHk0S2VX?=
 =?utf-8?B?OTZaYjBmditQTG5SQmhITVBFN1F1cHpNTmVzdzE3U2RhQk1zdmFGSU0rUmFt?=
 =?utf-8?B?bkpUeCtBZG8zTC9jdE4xeWxISjFUaTNxT0hXVkI4WGVLSUkyeXZFY2x3SGsy?=
 =?utf-8?B?RFVJdjVKb2xYeU16eHRNS2dvcDBNdHJkTnBYaXQxcndMRUxzTFk3aHVodTVG?=
 =?utf-8?B?b0xISS83a1FkSEZzTmtFV2ltVGdTWFZxMExzeXZzcG5VN24zdlZrWUQrZ0RN?=
 =?utf-8?B?SEUwVDBXUm5WV00wSFNyMGJMS0lwOGlNTXpHR1NJS1cvV3RGOVVtcThsQ0t4?=
 =?utf-8?B?cGJEb2xEZ1BvQUVBbEt0TU9QMi9jc2JQRUwxbHczSkxZNW5GbEtudElUeU9S?=
 =?utf-8?B?UVduSXpqK09SbXVDZmVucndHR0dYaW0vcUZmNjY0RXFxOVA3UDB2L2U0dDAy?=
 =?utf-8?B?VDExdzVOczBGMEJwUlNHOGU4RHliejFSTUdIck9PM1RTZHpFdzRkejBicUFN?=
 =?utf-8?B?aWthamRsRHVLb0F4dlZzeGY0bVpFYm9kaSs2YlIrMHRKSEQrYzA0V1k1emNm?=
 =?utf-8?B?TnVCa3hLeTB5QVZ0WlRHd3RnQ080NVI3RWRXZ05VcURHTkdPTjRKd2ZSZVZZ?=
 =?utf-8?Q?sqNb/LsaGP0bhzf7gbYHJ4BPHRjJ8MQlNGi+hvsvZnxHg?=
X-MS-Exchange-AntiSpam-MessageData-1: KtsByil27NX4ng==
X-OriginatorOrg: ddn.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f189e487-66ea-4394-bc33-08da2d463cff
X-MS-Exchange-CrossTenant-AuthSource: DM5PR1901MB2037.namprd19.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 May 2022 20:48:12.2418
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 753b6e26-6fd3-43e6-8248-3f1735d59bb4
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: J6cghAEAVyRWhLhcHh9XduZmaeMTh/9leBpJrXmEu3TQ0Ahw3VIZxRLwuJHo4W2FnM0IIYZwvMIfZ7F0od73XA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR19MB6003
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Vivek,

On 5/3/22 21:53, Vivek Goyal wrote:
> Reading the existing code a little bit more and trying to understand
> existing semantics. And that will help me unerstand what new is being
> done.
> 
> So current fuse_atomic_open() does following.
> 
> A. Looks up dentry (if d_in_lookup() is set).
> B. If dentry is positive or O_CREAT is not set, return.
> C. If server supports atomic create + open, use that to create file and
>     open it as well.
> D. If server does not support atomic create + open, just create file
>     using "mknod" and return. VFS will take care of opening the file.
> 
> Now with this patch, new flow is.
> 
> A. Look up dentry if d_in_lookup() is set as well as either file is not
>     being created or fc->no_atomic_create is set. This basiclally means
>     skip lookup if atomic_create is supported and file is being created.
> 
> B. Remains same. if dentry is positive or O_CREATE is not set, return.
> 
> C. If server supports new atomic_create(), use that.
> 
> D. If not, if server supports atomic create + open, use that
> 
> E. If not, fall back to mknod and do not open file.
> 
> So to me this new functionality is basically atomic "lookup + create +
> open"?
> 
> Or may be not. I see we check "fc->no_create" and fallback to mknod.
> 
>          if (fc->no_create)
>                  goto mknod;
> 
> So fc->no_create is representing both old atomic "create + open" as well
> as new "lookup + create + open" ?
> 
> It might be obvious to you, but it is not to me. So will be great if
> you shed some light on this.

we are going to reply more in detail tomorrow, it gets rather late for 
me as well. Anyway yes, goal is basically to avoid lookups as much 
possible.
AFAIK, lookup-intents had been first introduced years ago by Lustre 
developers - I guess to reduce network and server load - same reason for 
us. Later Miklos had introduced atomic_open, which makes code 
using/avoiding lookup much easier to read.
I guess unoticed that time, fuse was not fully using all possibilities 
of atomic-open - we can see quite some lookup/revalidate traffic for our 
file system.


I guess the commit message and introduction letter should be updated 
with your A/B/C/D/E scheme. A) changes a bit in patch 2/3, which extents 
it to normal file open, and patch 3/3 adds in revalidate.


Hope it helps,
Bernd