Received: by 2002:a6b:500f:0:0:0:0:0 with SMTP id e15csp511121iob; Wed, 4 May 2022 01:39:23 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzfynxp332w46OGbduJ40f8BsbJrGPlO26re5srYXZDTyAqeUVxOaCiTL4Cgnz4dDMt1wN3 X-Received: by 2002:a63:fb4d:0:b0:3c2:6049:6e22 with SMTP id w13-20020a63fb4d000000b003c260496e22mr8043882pgj.2.1651653563308; Wed, 04 May 2022 01:39:23 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1651653563; cv=none; d=google.com; s=arc-20160816; b=L6Dw3AzQA+yI2bB8SEGKbCEz2iwsVKc/cQLsok9ikPYdggDIY5TUnaadVU4jNzX0lH ccXlEojhhp8OPHa1yPO/LjHKgDeTrchBQM3IeOIKFZbGJYGyG4oS3oWDeItaSlDyQ9iu IB8rwWh33bHRN2CpRc/CAtsF6e/SfJRXYJL9GFpGPlD9KU2zYQczG7P1gLWWMR066FPW 0onwD7rtyWnWh6B6O+zLfp6/VBO8F6w7U8hYZraM/m8DYYVRvrlMeMUUQqInrWg5C7Ux ZLUQtOu+hkjyee9hVOevg69tPgxFVXNFHc91oUTMiMu0svh4pbLHJNmYbzgVjOsnbiBn mdVQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=d3hGKvDjDhA3A5CPaQDCX7CfmyhvahBgWk3Z2qwaPMg=; b=SO1eQ6sbjgN1Kxoe7/IRhZKBJM36DD+gFJWuwmf+KVAR+Zg/h6Hfj0pAVycKZMfPKt fdf0ek/mU5C2seVfy1ndxzANjY24pVsSSRlV831r1Ysnak/73GHD4IaqJULfRD5en4Eo VDgLSoVk4DszqKC3icTgU9oeF8FZ3PTjpRRvaqj4iwNdPfuHJkSuftLcN3UZgSugQtVB XbMxC/pdiLSGS1hMuKgR79StqFLmiwq5LnmxA5bYYD1RnW9Ho6/z0F5h4yVERKJ2HoDZ lWO4pSMFlrDWYEuivLahylTSMfjJag9LWIvYJZkdnm8LfcodLd8KdE0twNT9WRJWYDE4 O4qw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=gehLCKNa; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y73-20020a638a4c000000b003a9f944b0cfsi18412853pgd.558.2022.05.04.01.39.06; Wed, 04 May 2022 01:39:23 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=gehLCKNa; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238408AbiECP2a (ORCPT + 99 others); Tue, 3 May 2022 11:28:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33276 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237485AbiECP22 (ORCPT ); Tue, 3 May 2022 11:28:28 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id E91C63BBED for ; Tue, 3 May 2022 08:24:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1651591495; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=d3hGKvDjDhA3A5CPaQDCX7CfmyhvahBgWk3Z2qwaPMg=; b=gehLCKNaXnXVs3Qm+5wF9MhvfCjI/49QD73gXuRpCo3eUY3tvXyWhF5b+YBYmq9TtO+Lkv iHy+XAW/jSlSrTFXCb6zKtzbk/SFaKXZTn5k1VIelKptQLpwOgvcZl1oYS33pQ2p7FbcwR x1m/u1W1jKV3CFp60O/GocbdLDTTFG4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-622-5WN2CR5QOB-ZMM7mWcgrSQ-1; Tue, 03 May 2022 11:24:45 -0400 X-MC-Unique: 5WN2CR5QOB-ZMM7mWcgrSQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4D32FA97710; Tue, 3 May 2022 12:43:01 +0000 (UTC) Received: from horse.redhat.com (unknown [10.18.25.210]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3977340D2820; Tue, 3 May 2022 12:43:01 +0000 (UTC) Received: by horse.redhat.com (Postfix, from userid 10451) id E979B22570F; Tue, 3 May 2022 08:43:00 -0400 (EDT) Date: Tue, 3 May 2022 08:43:00 -0400 From: Vivek Goyal To: Dharmendra Singh Cc: miklos@szeredi.hu, linux-fsdevel@vger.kernel.org, fuse-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, bschubert@ddn.com, Dharmendra Singh Subject: Re: [PATCH v4 1/3] FUSE: Implement atomic lookup + create Message-ID: References: <20220502102521.22875-1-dharamhans87@gmail.com> <20220502102521.22875-2-dharamhans87@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220502102521.22875-2-dharamhans87@gmail.com> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW, SPF_HELO_NONE,SPF_NONE,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: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 02, 2022 at 03:55:19PM +0530, Dharmendra Singh wrote: > From: Dharmendra Singh > > When we go for creating a file (O_CREAT), we trigger > a lookup to FUSE USER SPACE. It is very much likely > that file does not exist yet as O_CREAT is passed to > open(). This lookup can be avoided and can be performed > as part of create call into libfuse. > > This lookup + create in single call to libfuse and finally > to USER SPACE has been named as atomic create. Looking at it and trying to understand if lookup can be avoided. But before that this naming of calling it atomic create seems confusing. We already have fuse_create_open() which we are calling it as "Atomic create+open operation". Now looks like this is extension to this operation where we are doing "Atomic lookup + create + open". Do I understand it correctly? Thanks Vivek > It is expected > that USER SPACE create the file, open it and fills in the > attributes which are then used to make inode stand/revalidate > in the kernel cache. Also if file was newly created(does not > exist yet by this time) in USER SPACE then it should be indicated > in `struct fuse_file_info` by setting a bit which is again used by > libfuse to send some flags back to fuse kernel to indicate that > that file was newly created. These flags are used by kernel to > indicate changes in parent directory. > > Fuse kernel automatically detects if atomic create is implemented > by libfuse/USER SPACE or not. And depending upon the outcome of > this check all further creates are decided to be atomic or non-atomic > creates. > > If libfuse/USER SPACE has not implemented the atomic create operation > then by default behaviour remains same i.e we do not optimize lookup > calls which are triggered before create calls into libfuse. > > Signed-off-by: Dharmendra Singh > --- > fs/fuse/dir.c | 82 +++++++++++++++++++++++++++++++++++---- > fs/fuse/fuse_i.h | 3 ++ > include/uapi/linux/fuse.h | 3 ++ > 3 files changed, 81 insertions(+), 7 deletions(-) > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c > index 656e921f3506..cad3322a007f 100644 > --- a/fs/fuse/dir.c > +++ b/fs/fuse/dir.c > @@ -523,7 +523,7 @@ static int get_security_context(struct dentry *entry, umode_t mode, > */ > static int fuse_create_open(struct inode *dir, struct dentry *entry, > struct file *file, unsigned int flags, > - umode_t mode) > + umode_t mode, uint32_t opcode) > { > int err; > struct inode *inode; > @@ -535,8 +535,10 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry, > struct fuse_entry_out outentry; > struct fuse_inode *fi; > struct fuse_file *ff; > + struct dentry *res = NULL; > void *security_ctx = NULL; > u32 security_ctxlen; > + bool atomic_create = (opcode == FUSE_ATOMIC_CREATE ? true : false); > > /* Userspace expects S_IFREG in create mode */ > BUG_ON((mode & S_IFMT) != S_IFREG); > @@ -566,7 +568,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry, > inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID; > } > > - args.opcode = FUSE_CREATE; > + args.opcode = opcode; > args.nodeid = get_node_id(dir); > args.in_numargs = 2; > args.in_args[0].size = sizeof(inarg); > @@ -613,9 +615,44 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry, > goto out_err; > } > kfree(forget); > - d_instantiate(entry, inode); > + /* > + * In atomic create, we skipped lookup and it is very much likely that > + * dentry has DCACHE_PAR_LOOKUP flag set on it so call d_splice_alias(). > + * Note: Only REG file is allowed under create/atomic create. > + */ > + /* There is special case when at very first call where we check if > + * atomic create is implemented by USER SPACE/libfuse or not, we > + * skipped lookup. Now, in case where atomic create is not implemented > + * underlying, we fall back to FUSE_CREATE. here we are required to handle > + * DCACHE_PAR_LOOKUP flag. > + */ > + if (!atomic_create && !d_in_lookup(entry) && fm->fc->no_atomic_create) > + d_instantiate(entry, inode); > + else { > + res = d_splice_alias(inode, entry); > + if (res) { > + /* Close the file in user space, but do not unlink it, > + * if it was created - with network file systems other > + * clients might have already accessed it. > + */ > + if (IS_ERR(res)) { > + fi = get_fuse_inode(inode); > + fuse_sync_release(fi, ff, flags); > + fuse_queue_forget(fm->fc, forget, outentry.nodeid, 1); > + err = PTR_ERR(res); > + goto out_err; > + } > + /* res is expected to be NULL since its REG file */ > + WARN_ON(res); > + } > + } > fuse_change_entry_timeout(entry, &outentry); > - fuse_dir_changed(dir); > + /* > + * In case of atomic create, we want to indicate directory change > + * only if USER SPACE actually created the file. > + */ > + if (!atomic_create || (outopen.open_flags & FOPEN_FILE_CREATED)) > + fuse_dir_changed(dir); > err = finish_open(file, entry, generic_file_open); > if (err) { > fi = get_fuse_inode(inode); > @@ -634,6 +671,29 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry, > return err; > } > > +static int fuse_atomic_create(struct inode *dir, struct dentry *entry, > + struct file *file, unsigned int flags, > + umode_t mode) > +{ > + int err; > + struct fuse_conn *fc = get_fuse_conn(dir); > + > + if (fc->no_atomic_create) > + return -ENOSYS; > + > + err = fuse_create_open(dir, entry, file, flags, mode, > + FUSE_ATOMIC_CREATE); > + /* If atomic create is not implemented then indicate in fc so that next > + * request falls back to normal create instead of going into libufse and > + * returning with -ENOSYS. > + */ > + if (err == -ENOSYS) { > + if (!fc->no_atomic_create) > + fc->no_atomic_create = 1; > + } > + return err; > +} > + > static int fuse_mknod(struct user_namespace *, struct inode *, struct dentry *, > umode_t, dev_t); > static int fuse_atomic_open(struct inode *dir, struct dentry *entry, > @@ -643,11 +703,12 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry, > int err; > struct fuse_conn *fc = get_fuse_conn(dir); > struct dentry *res = NULL; > + bool create = flags & O_CREAT ? true : false; > > if (fuse_is_bad(dir)) > return -EIO; > > - if (d_in_lookup(entry)) { > + if ((!create || fc->no_atomic_create) && d_in_lookup(entry)) { > res = fuse_lookup(dir, entry, 0); > if (IS_ERR(res)) > return PTR_ERR(res); > @@ -656,7 +717,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry, > entry = res; > } > > - if (!(flags & O_CREAT) || d_really_is_positive(entry)) > + if (!create || d_really_is_positive(entry)) > goto no_open; > > /* Only creates */ > @@ -665,7 +726,13 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry, > if (fc->no_create) > goto mknod; > > - err = fuse_create_open(dir, entry, file, flags, mode); > + err = fuse_atomic_create(dir, entry, file, flags, mode); > + /* Libfuse/user space has not implemented atomic create, therefore > + * fall back to normal create. > + */ > + if (err == -ENOSYS) > + err = fuse_create_open(dir, entry, file, flags, mode, > + FUSE_CREATE); > if (err == -ENOSYS) { > fc->no_create = 1; > goto mknod; > @@ -683,6 +750,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry, > } > > /* > + > * Code shared between mknod, mkdir, symlink and link > */ > static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args, > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index e8e59fbdefeb..d577a591ab16 100644 > --- a/fs/fuse/fuse_i.h > +++ b/fs/fuse/fuse_i.h > @@ -669,6 +669,9 @@ struct fuse_conn { > /** Is open/release not implemented by fs? */ > unsigned no_open:1; > > + /** Is atomic create not implemented by fs? */ > + unsigned no_atomic_create:1; > + > /** Is opendir/releasedir not implemented by fs? */ > unsigned no_opendir:1; > > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h > index d6ccee961891..e4b56004b148 100644 > --- a/include/uapi/linux/fuse.h > +++ b/include/uapi/linux/fuse.h > @@ -301,6 +301,7 @@ struct fuse_file_lock { > * FOPEN_CACHE_DIR: allow caching this directory > * FOPEN_STREAM: the file is stream-like (no file position at all) > * FOPEN_NOFLUSH: don't flush data cache on close (unless FUSE_WRITEBACK_CACHE) > + * FOPEN_FILE_CREATED: the file was actually created > */ > #define FOPEN_DIRECT_IO (1 << 0) > #define FOPEN_KEEP_CACHE (1 << 1) > @@ -308,6 +309,7 @@ struct fuse_file_lock { > #define FOPEN_CACHE_DIR (1 << 3) > #define FOPEN_STREAM (1 << 4) > #define FOPEN_NOFLUSH (1 << 5) > +#define FOPEN_FILE_CREATED (1 << 6) > > /** > * INIT request/reply flags > @@ -537,6 +539,7 @@ enum fuse_opcode { > FUSE_SETUPMAPPING = 48, > FUSE_REMOVEMAPPING = 49, > FUSE_SYNCFS = 50, > + FUSE_ATOMIC_CREATE = 51, > > /* CUSE specific operations */ > CUSE_INIT = 4096, > -- > 2.17.1 >