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: <8003098d-6b17-5cdf-866d-06fefdf1ca31@ddn.com>
Date:   Wed, 4 May 2022 17:46:27 +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 Hans <dharamhans87@gmail.com>
Cc:     Miklos Szeredi <miklos@szeredi.hu>, linux-fsdevel@vger.kernel.org,
        fuse-devel <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>
 <CACUYsyGoX+o19u41cZyF92eDBO-9rFN_EEWBvWBGrEMuNn29Mw@mail.gmail.com>
 <YnKR9CFYPXT1bM1F@redhat.com>
From:   Bernd Schubert <bschubert@ddn.com>
In-Reply-To: <YnKR9CFYPXT1bM1F@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?M0FOQkNnMjIwTTlhQXY2N2ZzYTRUQ3F1djB5bm5hWkdTYlh4Z3gzcmFhbyt3?=
 =?utf-8?B?a0Q3dUFGMXZJRFZvdFdMR2pwb0gzZnpjdVlra0d0RUNCZ0hkNnVXdy92ODJ6?=
 =?utf-8?B?VG8vcW4vaXRkbmVkSG9VRGdXQk1UT3c0dHAwcGVKZk1OQzArUE1WNjBYaVR5?=
 =?utf-8?B?aUZ1R1FTTllvWk9QV3dNTG5qSnFuUWo1R3g4SzBYaS93emRVMmhpSDhDT2R2?=
 =?utf-8?B?RjgvMDExTm1sK3JYYy9YZGpLNEovaUVLd3RDZkQycFZnTmNaeFZvTkxJSVR3?=
 =?utf-8?B?NUpTWjYreHlKSkhPL21FdENsY1dKcE5KN29iMmtqSFNLQWxEd0l0M3hZeWlT?=
 =?utf-8?B?ZUphbTkySHd3MVZNVi9QZ2taY2dvOGZwRXY2RlhpMTl3SG5JUFUrT3VNc1U3?=
 =?utf-8?B?TlVvKzZyeXFFLy9MbUNiMCtTODhPT2xlZDhBRDhMNnliNnVvYTUwWmluaVlq?=
 =?utf-8?B?MkFJaUQzdnk4UVNUcDhGazlDVmdSUDMwTndGYWhPdk93akNOQ25lM0k0VWh2?=
 =?utf-8?B?YUxpSENmSUJBUXN2clpyT3hJSDNPUDVPSFVUdllqOVNnUThZY3RnVHlsbktG?=
 =?utf-8?B?UTY3TTVsMkdmb243TE1uODhGU3k0TTcyZHFiZncxZEVwS2JPN2ovQzVud1U3?=
 =?utf-8?B?MXRPZDZqb0syMVFMZ1JNRHpkZ0dWc3hFc2ZOMjduN3ovVHFZMnpmaHZxVlBG?=
 =?utf-8?B?YWcwclV6cWo3Sy9vamw1aXI3ejFlUUcxbHZzSkMyeGxYSkdKZ204ODFmS0Fs?=
 =?utf-8?B?WEFSd2IxWlREU1d4ckY1WXN3K1FBcGc1VU96RVdqZDhhanhRMkdUVnp2NmE1?=
 =?utf-8?B?S0JEZnhjSmhJRjRnaXZXUm1WZlNXY0xjUGlERlV2MVlQNmg5aCt3bXNlVVZY?=
 =?utf-8?B?Y2VqRTcvZXg2b05obkRLMGwwMVo0cGZ3VXBVRzk1cXpiZVhjdGtCUUZ4bkRQ?=
 =?utf-8?B?UFA5MDhOZG9IOURqMkZBMHlrUVMxc3BqZHdDTERVbC9kZ0tRWDYxek90U3BN?=
 =?utf-8?B?SlRYa0E2aXB5ZlJhQU9QZkNzWTYvYmhEZEFqaFJEZmlhcERBVS9NWTBaeHN0?=
 =?utf-8?B?N3ZhQUpXZGg2NE10dTFId1lCVUs3cWFVRm40amR2N3B2eFdVbkFvM3lKV1Fl?=
 =?utf-8?B?RXozTk8yeUhiN2pLR0hPT2ZERUd3NExUYUg3MHVqcnNBOFJkckhlZTRsTFJz?=
 =?utf-8?B?eHhkTjVCbVJsU0FkMlBXZFN5MStPaUFqcFJ5NzEvcFgyM2hPdVIySC9MRFVj?=
 =?utf-8?B?VHZmbE9nTzY0aTFNdXVwU1JTL0Fna3I4NVNUUjk1QUFSRW9raUZ1bU5rcnVE?=
 =?utf-8?B?dyt4aDN2Rjc0Sy9mb3BaUVI1NzdJb2hTS3loUCtuT1pPM2ZKUjEzTm8ydzhM?=
 =?utf-8?B?ckxiWkRNd3NCV21wNGJyZ0hVeWxDNmEwYjN3NUFqMk9hUE4zNTZYNVQzTjJm?=
 =?utf-8?B?bjRhemRVU0pyNlg0VmsyWWlVdVUzS3kzdUtsSkJnMTVodFJ1cWlDYzhhUWpr?=
 =?utf-8?B?UHpvWW9SMGlmQU1rSDJEdXBXazBPZm5raGl4cW10Tm80QmR1QkpZYVVRTEM2?=
 =?utf-8?B?MGpQK2NvUFdlVWFPcVlEb21DMWpVald1eldHY2JBQTYxQVRzTzdZdjZISTlY?=
 =?utf-8?B?Y25qdFZVeXB5anlIUDVpSVF6dXhwTWRjSHdpYXJvcHJtOFJsVGREVFBGMFY1?=
 =?utf-8?B?WVpMVW1uQUdURUF3U2lDWnNRS0oxZmpMeXUrRyt5OGpoM1A2OW9wZmlmVWFE?=
 =?utf-8?B?dVl2T0JVY1JzR091dS9iM0doVGVXbGpLc2IyRHlMY0dSQUxTbHZKYU9pTEJk?=
 =?utf-8?B?UHBPUFcyOE92dTJLOFBFditNQWlXVUlmWm1kdTdjeU1XZTI2Rkd4T1Z5VzZ6?=
 =?utf-8?B?SXlZVjJpYU9naHVZS2VNUGd0YjFZanNyenArN2IzWnFyVFozeXFuSktzOW9j?=
 =?utf-8?B?U1BsbVNrdlNJQldIenh3d0hmSlRtbjJnaDhwWmp0NzU1S0hZWCthZUZ0d1pI?=
 =?utf-8?B?TFJNZVlhRy9DcFhXRUFFQ3lyaVlSZUJ4MjlhRUlNN3ljVFJMQVAwK0h6dHkv?=
 =?utf-8?B?NFdueFpDUkxVV1lsdzdCb014RkdOZWkyaklWM3BMTVJ0YmFxZVp0TW44NUtX?=
 =?utf-8?B?MUFFMmxzVnhWazROK0pYSWdLMVNQSGM5azljMGJJZkc4YWdKRzZQUnhRTTdE?=
 =?utf-8?B?b3JEcWIzRndDTHptaXF6OHJBc3RkaFozSlZnS2oyZVpjV2pVelF3QlNlUTFK?=
 =?utf-8?B?dHp1MTQ4SEdxemZrQ0lwckVYR3g1RDZ3eklDNEJZUXp1NWR6S1czQW5RaUND?=
 =?utf-8?B?a0x6ZWNmYzh6UG56cGN1bS9seDBGZ2RNcUJDUEROUVVPcmxrU3JZaUxvRGJX?=
 =?utf-8?Q?6P1FgDVjN6aTvnWdNTcZhk3Sz+tUqyrpq36aW1T0DufIS?=
X-MS-Exchange-AntiSpam-MessageData-1: RpJLJ4otC8Rg7w==
X-OriginatorOrg: ddn.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2242c03d-a16a-4b11-8f54-08da2de543a2
X-MS-Exchange-CrossTenant-AuthSource: DM5PR1901MB2037.namprd19.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2022 15:46:33.3396
 (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: 0TDZ3vXmnon5q9H3WWBetTa/whHJK8W72Wivg/xbW+K1UXYOAYZJT+FFmrOmkS5RoPRLVhZ2UETReOQ2BmB1rg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR19MB0057
X-Spam-Status: No, score=-4.6 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


On 5/4/22 16:47, Vivek Goyal wrote:

> Ok, naming is little confusing. I think we will have to put it in
> commit message and where you define FUSE_ATOMIC_CREATE that what's
> the difference between FUSE_CREATE and FUSE_ATOMIC_CREATE. This is
> ATOMIC w.r.t what?
> 
> May be atomic here means that "lookup + create + open" is a single operation.
> But then even FUSE_CREATE is atomic because "creat + open" is a single
> operation.
> 
> In fact FUSE_CREATE does lookup anyway and returns all the information
> in fuse_entry_out.
> 
> IIUC, only difference between FUSE_CREATE and FUSE_ATOMIC_CREATE is that
> later also carries information in reply whether file was actually created
> or not (FOPEN_FILE_CREATED). This will be set if file did not exist
> already and it was created indeed. Is that right?
> 
> I see FOPEN_FILE_CREATED is being used to avoid calling
> fuse_dir_changed(). That sounds like a separate optimization and probably
> should be in a separate patch.
> 
> IOW, I think this patch should be broken in to multiple pieces. First
> piece seems to be avoiding lookup() and given the way it is implemented,
> looks like we can avoid lookup() even by using existing FUSE_CREATE
> command. We don't necessarily need FUSE_ATOMIC_CREATE. Is that right?

The initial non-published patches had that, but I had actually asked not 
to go that route, because I'm scared that some user space file system 
implementations might get broken. Right now there is always a lookup 
before fuse_create_open() and when the resulting dentry is positive 
fuse_create_open/FUSE_CREATE is bypassed. I.e. user space 
implementations didn't need to handle existing files. Out of the sudden 
user space implementations might need to handle it and some of them 
might get broken with that kernel update. I guess even a single broken 
user space implementation would count as regression.
So I had asked to change the patch to require a user space flag.

-- Bernd