[RFC PATCH v2 1/2] fscrypt: remove d_lock on reading DCACHE_ENCRYPTED_WITH_KEY

[RFC PATCH v2 1/2] fscrypt: remove d_lock on reading DCACHE_ENCRYPTED_WITH_KEY

[Gao Xiang]

It seems there is no need to take d_lock when accessing
dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY in
fscrypt_d_revalidate. It only needs to be serialized for updating.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---

Change log v2:
 - Fix negative dentry (NULL d_inode) issue as Eric pointed out;
 - Fix to use '__lockref_is_dead()' as Eric pointed out before;

[original message]
Hi,
 At glance, I have no idea why fscrypt_d_revalidate disables
RCU-lookup. Therefore I made patches to raise a question and
do some test (eg. fsstress w/ and w/o drop_caches) and it
seems to work.

Thanks,
Gao Xiang

 fs/crypto/crypto.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 0f46cf550907..b38c574f70ac 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -329,9 +329,8 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 		return 0;
 	}
 
-	spin_lock(&dentry->d_lock);
-	cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY;
-	spin_unlock(&dentry->d_lock);
+	cached_with_key = READ_ONCE(dentry->d_flags) &
+		DCACHE_ENCRYPTED_WITH_KEY;
 	dir_has_key = (d_inode(dir)->i_crypt_info != NULL);
 	dput(dir);
 
-- 
2.14.4

[RFC PATCH v2 2/2] fscrypt: enable RCU-walk path for .d_revalidate

[Gao Xiang]

This patch attempts to enable RCU-walk for fscrypt.
It looks harmless at glance and could have better
performance than do ref-walk only.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---

p.s.
  In my opinion, it is better to retry for the case of
  READ_ONCE(dentry->d_parent) != dir rather than just
  return 0; and then do real lookup for ref-walk path...
  It behaves much like a seqlock and I tend to avoid
  taking d_lock as well..
  Please kindly correct me if I am wrong... Thanks in advance.

Thanks,
Gao Xiang

 fs/crypto/crypto.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index b38c574f70ac..2cc26fe9c43d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -319,20 +319,29 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 {
 	struct dentry *dir;
 	int dir_has_key, cached_with_key;
+	struct inode *dir_inode;
 
-	if (flags & LOOKUP_RCU)
-		return -ECHILD;
+repeat:
+	rcu_read_lock();
+	dir = READ_ONCE(dentry->d_parent);
 
-	dir = dget_parent(dentry);
-	if (!IS_ENCRYPTED(d_inode(dir))) {
-		dput(dir);
+	dir_inode = d_inode_rcu(dir);
+	if (!dir_inode || !IS_ENCRYPTED(dir_inode)) {
+		rcu_read_unlock();
 		return 0;
 	}
+	dir_has_key = (dir_inode->i_crypt_info != NULL);
+
+	/* original dir becomes invalid after sampling all? */
+	if (unlikely(__lockref_is_dead(&dir->d_lockref) ||
+		     READ_ONCE(dentry->d_parent) != dir)) {
+		rcu_read_unlock();
+		goto repeat;
+	}
+	rcu_read_unlock();
 
 	cached_with_key = READ_ONCE(dentry->d_flags) &
 		DCACHE_ENCRYPTED_WITH_KEY;
-	dir_has_key = (d_inode(dir)->i_crypt_info != NULL);
-	dput(dir);
 
 	/*
 	 * If the dentry was cached without the key, and it is a
-- 
2.14.4