linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yafang Shao <laoar.shao@gmail.com>
To: torvalds@linux-foundation.org
Cc: brauner@kernel.org, jack@suse.cz, laoar.shao@gmail.com,
	linux-fsdevel@vger.kernel.org, longman@redhat.com,
	viro@zeniv.linux.org.uk, walters@verbum.org,
	wangkai86@huawei.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Matthew Wilcox <willy@infradead.org>
Subject: [PATCH] vfs: Delete the associated dentry when deleting a file
Date: Wed, 15 May 2024 17:17:27 +0800	[thread overview]
Message-ID: <20240515091727.22034-1-laoar.shao@gmail.com> (raw)
In-Reply-To: <CAHk-=whHsCLoBsCdv2TiaQB+2TUR+wm2EPkaPHxF=g9Ofki7AQ@mail.gmail.com>

Our applications, built on Elasticsearch[0], frequently create and delete
files. These applications operate within containers, some with a memory
limit exceeding 100GB. Over prolonged periods, the accumulation of negative
dentries within these containers can amount to tens of gigabytes.

Upon container exit, directories are deleted. However, due to the numerous
associated dentries, this process can be time-consuming. Our users have
expressed frustration with this prolonged exit duration, which constitutes
our first issue.

Simultaneously, other processes may attempt to access the parent directory
of the Elasticsearch directories. Since the task responsible for deleting
the dentries holds the inode lock, processes attempting directory lookup
experience significant delays. This issue, our second problem, is easily
demonstrated:

  - Task 1 generates negative dentries:
  $ pwd
  ~/test
  $ mkdir es && cd es/ && ./create_and_delete_files.sh

  [ After generating tens of GB dentries ]

  $ cd ~/test && rm -rf es

  [ It will take a long duration to finish ]

  - Task 2 attempts to lookup the 'test/' directory
  $ pwd
  ~/test
  $ ls

  The 'ls' command in Task 2 experiences prolonged execution as Task 1
  is deleting the dentries.

We've devised a solution to address both issues by deleting associated
dentry when removing a file. Interestingly, we've noted that a similar
patch was proposed years ago[1], although it was rejected citing the
absence of tangible issues caused by negative dentries. Given our current
challenges, we're resubmitting the proposal. All relevant stakeholders from
previous discussions have been included for reference.

Some alternative solutions are also under discussion[2][3], such as
shrinking child dentries outside of the parent inode lock or even
asynchronously shrinking child dentries. However, given the straightforward
nature of the current solution, I believe this approach is still necessary.

[0]. https://github.com/elastic/elasticsearch
[1]. https://patchwork.kernel.org/project/linux-fsdevel/patch/1502099673-31620-1-git-send-email-wangkai86@huawei.com
[2]. https://lore.kernel.org/linux-fsdevel/20240511200240.6354-2-torvalds@linux-foundation.org/
[3]. https://lore.kernel.org/linux-fsdevel/CAHk-=wjEMf8Du4UFzxuToGDnF3yLaMcrYeyNAaH1NJWa6fwcNQ@mail.gmail.com/

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Waiman Long <longman@redhat.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Wangkai <wangkai86@huawei.com>
Cc: Colin Walters <walters@verbum.org>
---
 fs/dcache.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 71a8e943a0fa..2ffdb98e9166 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2360,19 +2360,17 @@ EXPORT_SYMBOL(d_hash_and_lookup);
  * - unhash this dentry and free it.
  *
  * Usually, we want to just turn this into
- * a negative dentry, but if anybody else is
- * currently using the dentry or the inode
- * we can't do that and we fall back on removing
- * it from the hash queues and waiting for
- * it to be deleted later when it has no users
+ * a negative dentry, but certain workloads can
+ * generate a large number of negative dentries.
+ * Therefore, it would be better to simply
+ * unhash it.
  */
  
 /**
  * d_delete - delete a dentry
  * @dentry: The dentry to delete
  *
- * Turn the dentry into a negative dentry if possible, otherwise
- * remove it from the hash queues so it can be deleted later
+ * Remove the dentry from the hash queues so it can be deleted later.
  */
  
 void d_delete(struct dentry * dentry)
@@ -2381,14 +2379,14 @@ void d_delete(struct dentry * dentry)
 
 	spin_lock(&inode->i_lock);
 	spin_lock(&dentry->d_lock);
+	__d_drop(dentry);
+
 	/*
 	 * Are we the only user?
 	 */
 	if (dentry->d_lockref.count == 1) {
-		dentry->d_flags &= ~DCACHE_CANT_MOUNT;
 		dentry_unlink_inode(dentry);
 	} else {
-		__d_drop(dentry);
 		spin_unlock(&dentry->d_lock);
 		spin_unlock(&inode->i_lock);
 	}
-- 
2.30.1 (Apple Git-130)


  reply	other threads:[~2024-05-15  9:17 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-11  2:27 [RFC PATCH] fs: dcache: Delete the associated dentry when deleting a file Yafang Shao
2024-05-11  2:53 ` Linus Torvalds
2024-05-11  3:35   ` Yafang Shao
2024-05-11  4:54     ` Waiman Long
2024-05-11 15:58       ` Matthew Wilcox
2024-05-11 16:07         ` Linus Torvalds
2024-05-11 16:13           ` Linus Torvalds
2024-05-11 18:05             ` Linus Torvalds
2024-05-11 18:26               ` [PATCH] vfs: move dentry shrinking outside the inode lock in 'rmdir()' Linus Torvalds
2024-05-11 18:42                 ` Linus Torvalds
2024-05-11 19:28                   ` Al Viro
2024-05-11 19:55                     ` Linus Torvalds
2024-05-11 20:31                       ` Al Viro
2024-05-11 21:17                         ` Al Viro
2024-05-12 15:45                     ` James Bottomley
2024-05-12 16:16                       ` Al Viro
2024-05-12 19:59                         ` Linus Torvalds
2024-05-12 20:29                           ` Linus Torvalds
2024-05-13  5:31                           ` Al Viro
2024-05-13 15:58                             ` Linus Torvalds
2024-05-13 16:33                               ` Al Viro
2024-05-13 16:44                                 ` Linus Torvalds
2024-05-23  7:18                                 ` Dave Chinner
2024-05-11 20:02                   ` [PATCH v2] " Linus Torvalds
2024-05-12  3:06                     ` Yafang Shao
2024-05-12  3:30                       ` Al Viro
2024-05-12  3:36                         ` Yafang Shao
2024-05-11 19:24                 ` [PATCH] " Al Viro
2024-05-15  2:18     ` [RFC PATCH] fs: dcache: Delete the associated dentry when deleting a file Yafang Shao
2024-05-15  2:36       ` Linus Torvalds
2024-05-15  9:17         ` Yafang Shao [this message]
2024-05-15 16:05           ` [PATCH] vfs: " Linus Torvalds
2024-05-16 13:44             ` Oliver Sang
2024-05-22  8:51             ` Oliver Sang
2024-05-23  2:21               ` Yafang Shao
2024-05-22  8:11           ` kernel test robot
2024-05-22 16:00             ` Linus Torvalds
2024-05-22 17:13               ` Matthew Wilcox
2024-05-22 18:11                 ` Linus Torvalds
2024-05-11  3:36   ` [RFC PATCH] fs: dcache: " Al Viro
2024-05-11  3:51     ` Yafang Shao
2024-05-11  5:18     ` Al Viro
2024-05-11  5:32     ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240515091727.22034-1-laoar.shao@gmail.com \
    --to=laoar.shao@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=walters@verbum.org \
    --cc=wangkai86@huawei.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).