[PATCH v2 0/2] fuse: add no forget support

[PATCH v2 0/2] fuse: add no forget support

[yangyun]

FUSE_FORGET requests are not used in some cases (e.g., juicefs) but have 
an impact on the system. So add no forget support.

---

v1: 
https://lore.kernel.org/linux-fsdevel/20240726083752.302301-1-yangyun50@huawei.com/

Changes from v1->v2:
- Still use fuse_queue_forget in patch 1 (Miklos)
- Simplify function name in patch 2 (Josef)

yangyun (2):
  fuse: move fuse_forget_link allocation inside fuse_queue_forget()
  fuse: add support for no forget requests

 fs/fuse/dev.c             | 14 +++++++--
 fs/fuse/dir.c             | 63 +++++++++------------------------------
 fs/fuse/fuse_i.h          | 34 ++++++++++++++-------
 fs/fuse/inode.c           | 43 ++++++--------------------
 fs/fuse/readdir.c         | 37 +++++------------------
 include/uapi/linux/fuse.h |  3 ++
 6 files changed, 70 insertions(+), 124 deletions(-)

-- 
2.33.0

[PATCH v2 1/2] fuse: move fuse_forget_link allocation inside fuse_queue_forget()

[yangyun]

The `struct fuse_forget_link` is allocated outside `fuse_queue_forget()`
before this patch. This requires the allocation in advance. In some
cases, this struct is not needed but allocated, which contributes to
memory usage and performance degradation. Besides, this messes up the
code to some extent. So move the `fuse_forget_link` allocation inside
fuse_queue_forget with __GFP_NOFAIL.

`fuse_force_forget()` is used by `readdirplus` before this patch for
the reason that we do not know how many 'fuse_forget_link' structures
will be allocated in advance when error happens. After this patch, this
function is not needed any more and can be removed. By this way, all
FUSE_FORGET requests are sent by using `fuse_queue_forget()` function as
e.g. virtiofs handles them differently from regular requests.

Signed-off-by: yangyun <yangyun50@huawei.com>
---
 fs/fuse/dev.c     | 10 ++++++--
 fs/fuse/dir.c     | 59 +++++++++++------------------------------------
 fs/fuse/fuse_i.h  | 11 +--------
 fs/fuse/inode.c   | 33 ++++----------------------
 fs/fuse/readdir.c | 29 +++++------------------
 5 files changed, 32 insertions(+), 110 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 9eb191b5c4de..4b106db2f97f 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -75,6 +75,12 @@ static void __fuse_put_request(struct fuse_req *req)
 	refcount_dec(&req->count);
 }
 
+/* Use __GFP_NOFAIL to force the allocation success */
+static struct fuse_forget_link *fuse_alloc_forget(void)
+{
+	return kzalloc(sizeof(struct fuse_forget_link), GFP_KERNEL_ACCOUNT | __GFP_NOFAIL);
+}
+
 void fuse_set_initialized(struct fuse_conn *fc)
 {
 	/* Make sure stores before this are seen on another CPU */
@@ -233,10 +239,10 @@ __releases(fiq->lock)
 	fiq->ops->wake_pending_and_unlock(fiq);
 }
 
-void fuse_queue_forget(struct fuse_conn *fc, struct fuse_forget_link *forget,
-		       u64 nodeid, u64 nlookup)
+void fuse_queue_forget(struct fuse_conn *fc, u64 nodeid, u64 nlookup)
 {
 	struct fuse_iqueue *fiq = &fc->iq;
+	struct fuse_forget_link *forget = fuse_alloc_forget();
 
 	forget->forget_one.nodeid = nodeid;
 	forget->forget_one.nlookup = nlookup;
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 2b0d4781f394..f070e380fd7b 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -207,7 +207,6 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 		 (flags & (LOOKUP_EXCL | LOOKUP_REVAL | LOOKUP_RENAME_TARGET))) {
 		struct fuse_entry_out outarg;
 		FUSE_ARGS(args);
-		struct fuse_forget_link *forget;
 		u64 attr_version;
 
 		/* For negative dentries, always do a fresh lookup */
@@ -220,11 +219,6 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 
 		fm = get_fuse_mount(inode);
 
-		forget = fuse_alloc_forget();
-		ret = -ENOMEM;
-		if (!forget)
-			goto out;
-
 		attr_version = fuse_get_attr_version(fm->fc);
 
 		parent = dget_parent(entry);
@@ -239,15 +233,13 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 			fi = get_fuse_inode(inode);
 			if (outarg.nodeid != get_node_id(inode) ||
 			    (bool) IS_AUTOMOUNT(inode) != (bool) (outarg.attr.flags & FUSE_ATTR_SUBMOUNT)) {
-				fuse_queue_forget(fm->fc, forget,
-						  outarg.nodeid, 1);
+				fuse_queue_forget(fm->fc, outarg.nodeid, 1);
 				goto invalid;
 			}
 			spin_lock(&fi->lock);
 			fi->nlookup++;
 			spin_unlock(&fi->lock);
 		}
-		kfree(forget);
 		if (ret == -ENOMEM || ret == -EINTR)
 			goto out;
 		if (ret || fuse_invalid_attr(&outarg.attr) ||
@@ -365,7 +357,6 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 {
 	struct fuse_mount *fm = get_fuse_mount_super(sb);
 	FUSE_ARGS(args);
-	struct fuse_forget_link *forget;
 	u64 attr_version;
 	int err;
 
@@ -374,23 +365,17 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 	if (name->len > FUSE_NAME_MAX)
 		goto out;
 
-
-	forget = fuse_alloc_forget();
-	err = -ENOMEM;
-	if (!forget)
-		goto out;
-
 	attr_version = fuse_get_attr_version(fm->fc);
 
 	fuse_lookup_init(fm->fc, &args, nodeid, name, outarg);
 	err = fuse_simple_request(fm, &args);
 	/* Zero nodeid is same as -ENOENT, but with valid timeout */
 	if (err || !outarg->nodeid)
-		goto out_put_forget;
+		goto out;
 
 	err = -EIO;
 	if (fuse_invalid_attr(&outarg->attr))
-		goto out_put_forget;
+		goto out;
 	if (outarg->nodeid == FUSE_ROOT_ID && outarg->generation != 0) {
 		pr_warn_once("root generation should be zero\n");
 		outarg->generation = 0;
@@ -401,13 +386,11 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 			   attr_version);
 	err = -ENOMEM;
 	if (!*inode) {
-		fuse_queue_forget(fm->fc, forget, outarg->nodeid, 1);
+		fuse_queue_forget(fm->fc, outarg->nodeid, 1);
 		goto out;
 	}
 	err = 0;
 
- out_put_forget:
-	kfree(forget);
  out:
 	return err;
 }
@@ -617,7 +600,6 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	struct inode *inode;
 	struct fuse_mount *fm = get_fuse_mount(dir);
 	FUSE_ARGS(args);
-	struct fuse_forget_link *forget;
 	struct fuse_create_in inarg;
 	struct fuse_open_out *outopenp;
 	struct fuse_entry_out outentry;
@@ -628,15 +610,10 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	/* Userspace expects S_IFREG in create mode */
 	BUG_ON((mode & S_IFMT) != S_IFREG);
 
-	forget = fuse_alloc_forget();
-	err = -ENOMEM;
-	if (!forget)
-		goto out_err;
-
 	err = -ENOMEM;
 	ff = fuse_file_alloc(fm, true);
 	if (!ff)
-		goto out_put_forget_req;
+		goto out_err;
 
 	if (!fm->fc->dont_mask)
 		mode &= ~current_umask();
@@ -670,7 +647,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 
 	err = get_create_ext(&args, dir, entry, mode);
 	if (err)
-		goto out_put_forget_req;
+		goto out_err;
 
 	err = fuse_simple_request(fm, &args);
 	free_ext_value(&args);
@@ -690,11 +667,10 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	if (!inode) {
 		flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
 		fuse_sync_release(NULL, ff, flags);
-		fuse_queue_forget(fm->fc, forget, outentry.nodeid, 1);
+		fuse_queue_forget(fm->fc, outentry.nodeid, 1);
 		err = -ENOMEM;
 		goto out_err;
 	}
-	kfree(forget);
 	d_instantiate(entry, inode);
 	fuse_change_entry_timeout(entry, &outentry);
 	fuse_dir_changed(dir);
@@ -716,8 +692,6 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 
 out_free_ff:
 	fuse_file_free(ff);
-out_put_forget_req:
-	kfree(forget);
 out_err:
 	return err;
 }
@@ -782,15 +756,10 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	struct inode *inode;
 	struct dentry *d;
 	int err;
-	struct fuse_forget_link *forget;
 
 	if (fuse_is_bad(dir))
 		return -EIO;
 
-	forget = fuse_alloc_forget();
-	if (!forget)
-		return -ENOMEM;
-
 	memset(&outarg, 0, sizeof(outarg));
 	args->nodeid = get_node_id(dir);
 	args->out_numargs = 1;
@@ -800,28 +769,27 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	if (args->opcode != FUSE_LINK) {
 		err = get_create_ext(args, dir, entry, mode);
 		if (err)
-			goto out_put_forget_req;
+			goto out_err;
 	}
 
 	err = fuse_simple_request(fm, args);
 	free_ext_value(args);
 	if (err)
-		goto out_put_forget_req;
+		goto out_err;
 
 	err = -EIO;
 	if (invalid_nodeid(outarg.nodeid) || fuse_invalid_attr(&outarg.attr))
-		goto out_put_forget_req;
+		goto out_err;
 
 	if ((outarg.attr.mode ^ mode) & S_IFMT)
-		goto out_put_forget_req;
+		goto out_err;
 
 	inode = fuse_iget(dir->i_sb, outarg.nodeid, outarg.generation,
 			  &outarg.attr, ATTR_TIMEOUT(&outarg), 0);
 	if (!inode) {
-		fuse_queue_forget(fm->fc, forget, outarg.nodeid, 1);
+		fuse_queue_forget(fm->fc, outarg.nodeid, 1);
 		return -ENOMEM;
 	}
-	kfree(forget);
 
 	d_drop(entry);
 	d = d_splice_alias(inode, entry);
@@ -837,10 +805,9 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	fuse_dir_changed(dir);
 	return 0;
 
- out_put_forget_req:
+ out_err:
 	if (err == -EEXIST)
 		fuse_invalidate_entry(entry);
-	kfree(forget);
 	return err;
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f23919610313..90176133209d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -71,9 +71,6 @@ struct fuse_submount_lookup {
 	/** Unique ID, which identifies the inode between userspace
 	 * and kernel */
 	u64 nodeid;
-
-	/** The request used for sending the FORGET message */
-	struct fuse_forget_link *forget;
 };
 
 /** Container for data related to mapping to backing file */
@@ -98,9 +95,6 @@ struct fuse_inode {
 	/** Number of lookups on this inode */
 	u64 nlookup;
 
-	/** The request used for sending the FORGET message */
-	struct fuse_forget_link *forget;
-
 	/** Time in jiffies until the file attributes are valid */
 	u64 i_time;
 
@@ -1048,10 +1042,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 /**
  * Send FORGET command
  */
-void fuse_queue_forget(struct fuse_conn *fc, struct fuse_forget_link *forget,
-		       u64 nodeid, u64 nlookup);
-
-struct fuse_forget_link *fuse_alloc_forget(void);
+void fuse_queue_forget(struct fuse_conn *fc, u64 nodeid, u64 nlookup);
 
 struct fuse_forget_link *fuse_dequeue_forget(struct fuse_iqueue *fiq,
 					     unsigned int max,
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 99e44ea7d875..da3e5d4c032c 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -63,27 +63,13 @@ MODULE_PARM_DESC(max_user_congthresh,
 static struct file_system_type fuseblk_fs_type;
 #endif
 
-struct fuse_forget_link *fuse_alloc_forget(void)
-{
-	return kzalloc(sizeof(struct fuse_forget_link), GFP_KERNEL_ACCOUNT);
-}
-
 static struct fuse_submount_lookup *fuse_alloc_submount_lookup(void)
 {
 	struct fuse_submount_lookup *sl;
 
 	sl = kzalloc(sizeof(struct fuse_submount_lookup), GFP_KERNEL_ACCOUNT);
-	if (!sl)
-		return NULL;
-	sl->forget = fuse_alloc_forget();
-	if (!sl->forget)
-		goto out_free;
 
 	return sl;
-
-out_free:
-	kfree(sl);
-	return NULL;
 }
 
 static struct inode *fuse_alloc_inode(struct super_block *sb)
@@ -104,20 +90,15 @@ static struct inode *fuse_alloc_inode(struct super_block *sb)
 	fi->submount_lookup = NULL;
 	mutex_init(&fi->mutex);
 	spin_lock_init(&fi->lock);
-	fi->forget = fuse_alloc_forget();
-	if (!fi->forget)
-		goto out_free;
 
 	if (IS_ENABLED(CONFIG_FUSE_DAX) && !fuse_dax_inode_alloc(sb, fi))
-		goto out_free_forget;
+		goto out_free;
 
 	if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
 		fuse_inode_backing_set(fi, NULL);
 
 	return &fi->inode;
 
-out_free_forget:
-	kfree(fi->forget);
 out_free:
 	kmem_cache_free(fuse_inode_cachep, fi);
 	return NULL;
@@ -128,7 +109,6 @@ static void fuse_free_inode(struct inode *inode)
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
 	mutex_destroy(&fi->mutex);
-	kfree(fi->forget);
 #ifdef CONFIG_FUSE_DAX
 	kfree(fi->dax);
 #endif
@@ -144,8 +124,7 @@ static void fuse_cleanup_submount_lookup(struct fuse_conn *fc,
 	if (!refcount_dec_and_test(&sl->count))
 		return;
 
-	fuse_queue_forget(fc, sl->forget, sl->nodeid, 1);
-	sl->forget = NULL;
+	fuse_queue_forget(fc, sl->nodeid, 1);
 	kfree(sl);
 }
 
@@ -163,12 +142,8 @@ static void fuse_evict_inode(struct inode *inode)
 
 		if (FUSE_IS_DAX(inode))
 			fuse_dax_inode_cleanup(inode);
-		if (fi->nlookup) {
-			fuse_queue_forget(fc, fi->forget, fi->nodeid,
-					  fi->nlookup);
-			fi->forget = NULL;
-		}
-
+		if (fi->nlookup)
+			fuse_queue_forget(fc, fi->nodeid, fi->nlookup);
 		if (fi->submount_lookup) {
 			fuse_cleanup_submount_lookup(fc, fi->submount_lookup);
 			fi->submount_lookup = NULL;
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index 0377b6dc24c8..721fae563c84 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -262,27 +262,6 @@ static int fuse_direntplus_link(struct file *file,
 	return 0;
 }
 
-static void fuse_force_forget(struct file *file, u64 nodeid)
-{
-	struct inode *inode = file_inode(file);
-	struct fuse_mount *fm = get_fuse_mount(inode);
-	struct fuse_forget_in inarg;
-	FUSE_ARGS(args);
-
-	memset(&inarg, 0, sizeof(inarg));
-	inarg.nlookup = 1;
-	args.opcode = FUSE_FORGET;
-	args.nodeid = nodeid;
-	args.in_numargs = 1;
-	args.in_args[0].size = sizeof(inarg);
-	args.in_args[0].value = &inarg;
-	args.force = true;
-	args.noreply = true;
-
-	fuse_simple_request(fm, &args);
-	/* ignore errors */
-}
-
 static int parse_dirplusfile(char *buf, size_t nbytes, struct file *file,
 			     struct dir_context *ctx, u64 attr_version)
 {
@@ -320,8 +299,12 @@ static int parse_dirplusfile(char *buf, size_t nbytes, struct file *file,
 		nbytes -= reclen;
 
 		ret = fuse_direntplus_link(file, direntplus, attr_version);
-		if (ret)
-			fuse_force_forget(file, direntplus->entry_out.nodeid);
+		if (ret) {
+			struct inode *inode = file_inode(file);
+			struct fuse_mount *fm = get_fuse_mount(inode);
+
+			fuse_queue_forget(fm->fc, direntplus->entry_out.nodeid, 1);
+		}
 	}
 
 	return 0;
-- 
2.33.0

[PATCH v2 2/2] fuse: add support for no forget requests

[yangyun]

FUSE_FORGET requests are not used if the fuse file system does not
implement the forget operation in userspace (e.g., fuse file system
does not cache any inodes).

However, the kernel is invisible to the userspace implementation and
always sends FUSE_FORGET requests, which can lead to performance
degradation because of useless contex switch and memory copy in some
cases (e.g., many inodes are evicted from icache which was described
in commit 07e77dca8a1f ("fuse: separate queue for FORGET requests")).

Just like 'no_interrupt' in 'struct fuse_conn', we add 'no_forget'.
But since FUSE_FORGET request does not have a reply from userspace,
we can not use ENOSYS to reflect the 'no_forget' assignment. So add
the FUSE_NO_FORGET_SUPPORT init flag.

Besides, if no_forget is enabled, 'nlookup' in 'struct fuse_inode'
does not used and its value change can be disabled which are protected
by spin_lock to reduce lock contention.

Signed-off-by: yangyun <yangyun50@huawei.com>
---
 fs/fuse/dev.c             |  6 +++++-
 fs/fuse/dir.c             |  4 +---
 fs/fuse/fuse_i.h          | 23 +++++++++++++++++++++++
 fs/fuse/inode.c           | 10 +++++-----
 fs/fuse/readdir.c         |  8 ++------
 include/uapi/linux/fuse.h |  3 +++
 6 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 4b106db2f97f..449c29ef1bce 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -242,8 +242,12 @@ __releases(fiq->lock)
 void fuse_queue_forget(struct fuse_conn *fc, u64 nodeid, u64 nlookup)
 {
 	struct fuse_iqueue *fiq = &fc->iq;
-	struct fuse_forget_link *forget = fuse_alloc_forget();
+	struct fuse_forget_link *forget;
 
+	if (fc->no_forget)
+		return;
+
+	forget = fuse_alloc_forget();
 	forget->forget_one.nodeid = nodeid;
 	forget->forget_one.nlookup = nlookup;
 
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index f070e380fd7b..2cbf96bb8022 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -236,9 +236,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 				fuse_queue_forget(fm->fc, outarg.nodeid, 1);
 				goto invalid;
 			}
-			spin_lock(&fi->lock);
-			fi->nlookup++;
-			spin_unlock(&fi->lock);
+			fuse_inc_nlookup(fm->fc, fi);
 		}
 		if (ret == -ENOMEM || ret == -EINTR)
 			goto out;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 90176133209d..570decd4b8d6 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -854,6 +854,9 @@ struct fuse_conn {
 	/** Passthrough support for read/write IO */
 	unsigned int passthrough:1;
 
+	/** Is forget not implemented by fs? */
+	unsigned int no_forget:1;
+
 	/** Maximum stack depth for passthrough backing files */
 	int max_stack_depth;
 
@@ -1023,6 +1026,26 @@ static inline void fuse_sync_bucket_dec(struct fuse_sync_bucket *bucket)
 	rcu_read_unlock();
 }
 
+static inline void fuse_inc_nlookup(struct fuse_conn *fc, struct fuse_inode *fi)
+{
+	if (fc->no_forget)
+		return;
+
+	spin_lock(&fi->lock);
+	fi->nlookup++;
+	spin_lock(&fi->lock);
+}
+
+static inline void fuse_dec_nlookup(struct fuse_conn *fc, struct fuse_inode *fi)
+{
+	if (fc->no_forget)
+		return;
+
+	spin_lock(&fi->lock);
+	fi->nlookup--;
+	spin_lock(&fi->lock);
+}
+
 /** Device operations */
 extern const struct file_operations fuse_dev_operations;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index da3e5d4c032c..57fe1bbc9a83 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -458,9 +458,7 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 		}
 	}
 	fi = get_fuse_inode(inode);
-	spin_lock(&fi->lock);
-	fi->nlookup++;
-	spin_unlock(&fi->lock);
+	fuse_inc_nlookup(fc, fi);
 done:
 	fuse_change_attributes(inode, attr, NULL, attr_valid, attr_version);
 
@@ -1306,6 +1304,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			}
 			if (flags & FUSE_NO_EXPORT_SUPPORT)
 				fm->sb->s_export_op = &fuse_export_fid_operations;
+			if (flags & FUSE_NO_FORGET_SUPPORT)
+				fc->no_forget = 1;
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
 			fc->no_lock = 1;
@@ -1353,7 +1353,7 @@ void fuse_send_init(struct fuse_mount *fm)
 		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
 		FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
 		FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP |
-		FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND;
+		FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_NO_FORGET_SUPPORT;
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		flags |= FUSE_MAP_ALIGNMENT;
@@ -1568,7 +1568,7 @@ static int fuse_fill_super_submount(struct super_block *sb,
 	 * that, though, so undo it here.
 	 */
 	fi = get_fuse_inode(root);
-	fi->nlookup--;
+	fuse_dec_nlookup(fm->fc, fi);
 
 	sb->s_d_op = &fuse_dentry_operations;
 	sb->s_root = d_make_root(root);
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index 721fae563c84..6cda9b34a0e1 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -218,9 +218,7 @@ static int fuse_direntplus_link(struct file *file,
 		}
 
 		fi = get_fuse_inode(inode);
-		spin_lock(&fi->lock);
-		fi->nlookup++;
-		spin_unlock(&fi->lock);
+		fuse_inc_nlookup(fc, fi);
 
 		forget_all_cached_acls(inode);
 		fuse_change_attributes(inode, &o->attr, NULL,
@@ -247,9 +245,7 @@ static int fuse_direntplus_link(struct file *file,
 			if (!IS_ERR(inode)) {
 				struct fuse_inode *fi = get_fuse_inode(inode);
 
-				spin_lock(&fi->lock);
-				fi->nlookup--;
-				spin_unlock(&fi->lock);
+				fuse_dec_nlookup(fc, fi);
 			}
 			return PTR_ERR(dentry);
 		}
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index d08b99d60f6f..bf660880bc7a 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -217,6 +217,7 @@
  *  - add backing_id to fuse_open_out, add FOPEN_PASSTHROUGH open flag
  *  - add FUSE_NO_EXPORT_SUPPORT init flag
  *  - add FUSE_NOTIFY_RESEND, add FUSE_HAS_RESEND init flag
+ *  - add FUSE_NO_FORGET_SUPPORT init flag
  */
 
 #ifndef _LINUX_FUSE_H
@@ -421,6 +422,7 @@ struct fuse_file_lock {
  * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support
  * FUSE_HAS_RESEND: kernel supports resending pending requests, and the high bit
  *		    of the request ID indicates resend requests
+ * FUSE_NO_FORGET_SUPPORT: disable forget requests
  */
 #define FUSE_ASYNC_READ		(1 << 0)
 #define FUSE_POSIX_LOCKS	(1 << 1)
@@ -463,6 +465,7 @@ struct fuse_file_lock {
 #define FUSE_PASSTHROUGH	(1ULL << 37)
 #define FUSE_NO_EXPORT_SUPPORT	(1ULL << 38)
 #define FUSE_HAS_RESEND		(1ULL << 39)
+#define FUSE_NO_FORGET_SUPPORT  (1ULL << 40)
 
 /* Obsolete alias for FUSE_DIRECT_IO_ALLOW_MMAP */
 #define FUSE_DIRECT_IO_RELAX	FUSE_DIRECT_IO_ALLOW_MMAP
-- 
2.33.0

Re: [PATCH v2 1/2] fuse: move fuse_forget_link allocation inside fuse_queue_forget()

[Miklos Szeredi]

On Sat, 24 Aug 2024 at 11:26, yangyun <yangyun50@huawei.com> wrote:
>
> The `struct fuse_forget_link` is allocated outside `fuse_queue_forget()`
> before this patch. This requires the allocation in advance. In some
> cases, this struct is not needed but allocated, which contributes to
> memory usage and performance degradation. Besides, this messes up the
> code to some extent. So move the `fuse_forget_link` allocation inside
> fuse_queue_forget with __GFP_NOFAIL.
>
> `fuse_force_forget()` is used by `readdirplus` before this patch for
> the reason that we do not know how many 'fuse_forget_link' structures
> will be allocated in advance when error happens. After this patch, this
> function is not needed any more and can be removed. By this way, all
> FUSE_FORGET requests are sent by using `fuse_queue_forget()` function as
> e.g. virtiofs handles them differently from regular requests.

The patch is nice and clean.  However, I'm a bit worried about the
inode eviction path, which can be triggered from memory reclaim.
Allocating a small structure shouldn't be an issue, yet I feel that
the old way of preallocating it on inode creation should be better.

What do you think?

Thanks,
Miklos

Re: [PATCH v2 1/2] fuse: move fuse_forget_link allocation inside fuse_queue_forget()

[yangyun]

On Mon, Aug 26, 2024 at 09:24:22PM +0200, Miklos Szeredi wrote:
> On Sat, 24 Aug 2024 at 11:26, yangyun <yangyun50@huawei.com> wrote:
> >
> > The `struct fuse_forget_link` is allocated outside `fuse_queue_forget()`
> > before this patch. This requires the allocation in advance. In some
> > cases, this struct is not needed but allocated, which contributes to
> > memory usage and performance degradation. Besides, this messes up the
> > code to some extent. So move the `fuse_forget_link` allocation inside
> > fuse_queue_forget with __GFP_NOFAIL.
> >
> > `fuse_force_forget()` is used by `readdirplus` before this patch for
> > the reason that we do not know how many 'fuse_forget_link' structures
> > will be allocated in advance when error happens. After this patch, this
> > function is not needed any more and can be removed. By this way, all
> > FUSE_FORGET requests are sent by using `fuse_queue_forget()` function as
> > e.g. virtiofs handles them differently from regular requests.
> 
> The patch is nice and clean.  However, I'm a bit worried about the
> inode eviction path, which can be triggered from memory reclaim.
> Allocating a small structure shouldn't be an issue, yet I feel that
> the old way of preallocating it on inode creation should be better.
> 
> What do you think?
You are right. Since flag __GFP_NOFAIL would block the system, especailly in 
the memeory reclaim situaion. The preallocating of this struct is necessary
because it will be exactly used in the inode eviction path. I'm sorry for not 
considering the risk of __GFP_NOFAIL seriously as a beginner.

Thanks for the reminder. Update soon.
> 
> Thanks,
> Miklos

Re: [PATCH v2 0/2] fuse: add no forget support

[Miklos Szeredi]

On Sat, 24 Aug 2024 at 11:26, yangyun <yangyun50@huawei.com> wrote:
>
> FUSE_FORGET requests are not used in some cases (e.g., juicefs) but have
> an impact on the system. So add no forget support.

Applied, thanks.

Miklos